Oxide

Merge lp:~osomon/oxide/fix-crash-popup-menu into lp:~oxide-developers/oxide/oxide.trunk

  1. fix-crash-popup-menu
  2. Merge into oxide.trunk
Proposed by Olivier Tilloy
Status: Merged
Merged at revision: 1058
Proposed branch: lp:~osomon/oxide/fix-crash-popup-menu
Merge into: lp:~oxide-developers/oxide/oxide.trunk
Diff against target: 94 lines (+61/-1)
3 files modified
qt/tests/qmltests/crash/tst_bug1450243.html (+10/-0)
qt/tests/qmltests/crash/tst_bug1450243.qml (+42/-0)
shared/browser/oxide_web_popup_menu.cc (+9/-1)
To merge this branch: bzr merge lp:~osomon/oxide/fix-crash-popup-menu
Reviewer Review Type Date Requested Status
Chris Coulson Approve
Review via email: mp+257938@code.launchpad.net

Commit message

Fix a crash when a popup menu is dismissed twice (by calling accept() and then cancel() on it).

To post a comment you must log in.
Revision history for this message
Chris Coulson (chrisccoulson) wrote :

Thanks - I've just left a small comment inline

review: Approve
lp:~osomon/oxide/fix-crash-popup-menu updated
1059. By Olivier Tilloy

Cosmetics.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'qt/tests/qmltests/crash/tst_bug1450243.html'
2--- qt/tests/qmltests/crash/tst_bug1450243.html 1970-01-01 00:00:00 +0000
3+++ qt/tests/qmltests/crash/tst_bug1450243.html 2015-04-30 20:03:04 +0000
4@@ -0,0 +1,10 @@
5+<html>
6+<body>
7+ <select id="test">
8+ <option value="test1" selected>Test 1</option>
9+ <option value="test2">Test 2</option>
10+ <option value="test3">Test 3</option>
11+ <option value="test4">Test 4</option>
12+ </select>
13+</body>
14+</html>
15
16=== added file 'qt/tests/qmltests/crash/tst_bug1450243.qml'
17--- qt/tests/qmltests/crash/tst_bug1450243.qml 1970-01-01 00:00:00 +0000
18+++ qt/tests/qmltests/crash/tst_bug1450243.qml 2015-04-30 20:03:04 +0000
19@@ -0,0 +1,42 @@
20+import QtQuick 2.0
21+import QtTest 1.0
22+import com.canonical.Oxide 1.0
23+import com.canonical.Oxide.Testing 1.0
24+
25+TestWebView {
26+ id: webView
27+ focus: true
28+ width: 200
29+ height: 200
30+
31+ property bool popupMenuDestroyed: false
32+
33+ popupMenu: Item {
34+ Component.onCompleted: {
35+ model.items.select(3)
36+ model.accept()
37+ }
38+ onVisibleChanged: {
39+ if (!visible) {
40+ model.cancel()
41+ }
42+ }
43+ Component.onDestruction: popupMenuDestroyed = true
44+ }
45+
46+ TestCase {
47+ name: "bug1450243"
48+ when: windowShown
49+
50+ function test_bug1450243() {
51+ webView.url = "http://testsuite/tst_bug1450243.html";
52+ verify(webView.waitForLoadSucceeded(),
53+ "Timed out waiting for successful load");
54+
55+ var r = webView.getTestApi().getBoundingClientRectForSelector("#test");
56+ mouseClick(webView, r.x + r.width / 2, r.y + r.height / 2, Qt.LeftButton);
57+
58+ tryCompare(webView, "popupMenuDestroyed", true);
59+ }
60+ }
61+}
62
63=== modified file 'shared/browser/oxide_web_popup_menu.cc'
64--- shared/browser/oxide_web_popup_menu.cc 2015-04-08 10:39:29 +0000
65+++ shared/browser/oxide_web_popup_menu.cc 2015-04-30 20:03:04 +0000
66@@ -47,19 +47,27 @@
67 }
68
69 void WebPopupMenu::Close() {
70+ render_frame_host_ = nullptr;
71 weak_ptr_factory_.InvalidateWeakPtrs();
72 Hide();
73- render_frame_host_ = nullptr;
74 content::BrowserThread::DeleteSoon(
75 content::BrowserThread::UI, FROM_HERE, this);
76 }
77
78 void WebPopupMenu::SelectItems(const std::vector<int>& selected_indices) {
79+ if (!render_frame_host_) {
80+ return;
81+ }
82+
83 render_frame_host_->DidSelectPopupMenuItems(selected_indices);
84 Close();
85 }
86
87 void WebPopupMenu::Cancel() {
88+ if (!render_frame_host_) {
89+ return;
90+ }
91+
92 render_frame_host_->DidCancelPopupMenu();
93 Close();
94 }

Subscribers

People subscribed via source and target branches