Merge lp:~joergberroth/unav/WebViewCrashHandling into lp:unav

Proposed by JkB on 2016-06-06
Status: Needs review
Proposed branch: lp:~joergberroth/unav/WebViewCrashHandling
Merge into: lp:unav
Diff against target: 417 lines (+174/-35)
9 files modified
qml/Favorites.qml (+2/-0)
qml/Location.qml (+4/-0)
qml/Main.qml (+32/-19)
qml/PoiDetailsPage.qml (+2/-0)
qml/PoiListPage.qml (+2/-0)
qml/SearchPage.qml (+1/-3)
qml/components/NotificationBar.qml (+30/-2)
qml/components/POIQuickAccessGridView.qml (+9/-11)
qml/components/WebProcessMonitor.qml (+92/-0)
To merge this branch: bzr merge lp:~joergberroth/unav/WebViewCrashHandling
Reviewer Review Type Date Requested Status
costales 2016-06-06 Pending
Review via email: mp+296529@code.launchpad.net

This proposal supersedes a proposal from 2016-05-27.

Commit message

* added WebProcessMonitor.qml from webbrowser-app to manage webview kills
  - notification that map crashed.
  - try to reload webview if it was killed.
  - if a route was active, it will be reloaded again.
* adjusted NotificationBar.qml and some code clean up (NotificationBarTimer handling)

Description of the change

* added WebProcessMonitor.qml from webbrowser-app to manage webview kills
  - notification that map crashed.
  - try to reload webview if it was killed.
  - if a route was active, it will be reloaded again.
* adjusted NotificationBar.qml and some code clean up (NotificationBarTimer handling)

To post a comment you must log in.
costales (costales) wrote : Posted in a previous version of this proposal

It didn't work for me.
I used uNav in the morning.
After ~10 hours, I opened and I saw the map and the red popup.
uNav was freezed ~5 seconds and then, the system killed it.

review: Needs Fixing
costales (costales) wrote :

Hi Joerg. I'm doing tests and this is working :))

Please, one question. Why this?

=== modified file 'nav/index.html'
--- nav/index.html 2016-06-08 17:14:51 +0000
+++ nav/index.html 2016-06-13 18:14:57 +0000
@@ -593,6 +593,7 @@
    nav.set_route_status('yes');
    ui.update();
    qml_set_center_onpos(2);
+ qml_set_route_status();
   }
  </script>

Thanks |o/

JkB (joergberroth) wrote :

Hey,
This line is needed to always set the current end coords on qml side.
This enables to set the last active route after an oxide crash again
automtically.

Am Montag, 13. Juni 2016 20:20:31 CEST schrieb costales
<email address hidden>:
> Hi Joerg. I'm doing tests and this is working :))
>
> Please, one question. Why this?
>
> === modified file 'nav/index.html'
> --- nav/index.html 2016-06-08 17:14:51 +0000
> +++ nav/index.html 2016-06-13 18:14:57 +0000
> @@ -593,6 +593,7 @@
> nav.set_route_status('yes');
> ui.update();
> qml_set_center_onpos(2);
> + qml_set_route_status();
> }
> </script>
>
> Thanks |o/

--
Versandt, mit Dekko von meinem Ubuntu-Gerät

costales (costales) wrote :

From my knowledge and tests more than 1 call between qml + html
created weird conflicts.

Could be that set in the initialization call on the new .qml?

Best regards!

JkB (joergberroth) wrote :

?
So far i have not experieced any issues with that.
It is important, that qml side is updated with the route details ad soon as
a route starts. Maybe you find a better place for the command....

Am Montag, 13. Juni 2016 21:57:23 CEST schrieb costales
<email address hidden>:
>>From my knowledge and tests more than 1 call between qml + html
> created weird conflicts.
>
> Could be that set in the initialization call on the new .qml?
>
> Best regards!
>

--
Versandt, mit Dekko von meinem Ubuntu-Gerät

77. By JkB on 2016-06-14

*merge with costales branch

78. By JkB on 2016-06-14

* modified notification text

Unmerged revisions

78. By JkB on 2016-06-14

* modified notification text

77. By JkB on 2016-06-14

*merge with costales branch

76. By JkB on 2016-06-05

*changed error messages text

75. By JkB on 2016-06-03

*removed timer and improved popup handling

74. By JkB on 2016-06-03

*merge with trunk

73. By JkB on 2016-06-01

*removed typo in id
*added close() signal

72. By JkB on 2016-05-29

*ensure that endLat/endLng will always be set in Main.qml if route_state changes to "yes"

71. By JkB on 2016-05-29

*add another notification
*wait 500ms before recalculating route
*modified a notifiacation in WebProcessMonitor.qml

70. By JkB on 2016-05-27

* small format fix

69. By JkB on 2016-05-27

* added WebProcessMonitor.qml from webbrowser-app to manage webview kills
  - notification that map crashed.
  - try to reload webview if it was killed.
  - if a route was active, it will be reloaded again.
* adjusted NotificationBar.qml and some code clean up (NotificationBarTimer handling)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'qml/Favorites.qml'
2--- qml/Favorites.qml 2016-05-05 19:21:33 +0000
3+++ qml/Favorites.qml 2016-06-14 19:54:18 +0000
4@@ -114,6 +114,8 @@
5 mainPageStack.removePages(mainPageStack.primaryPage)
6 mainPageStack.center_onpos = 2;
7 mainPageStack.routeState = 'yes';
8+ mainPageStack.endLat = model.lat;
9+ mainPageStack.endLng = model.lng;
10 mainPageStack.executeJavaScript("calc2coord("+ model.lat + "," + model.lng + ");");
11 }
12 },
13
14=== modified file 'qml/Location.qml'
15--- qml/Location.qml 2016-06-03 13:52:53 +0000
16+++ qml/Location.qml 2016-06-14 19:54:18 +0000
17@@ -256,6 +256,8 @@
18 mainPageStack.removePages(mainPageStack.primaryPage)
19 mainPageStack.center_onpos = 2;
20 mainPageStack.routeState = 'yes'
21+ mainPageStack.endLat = model.lat;
22+ mainPageStack.endLng = model.lng;
23 mainPageStack.executeJavaScript("calc2coord(" + model.lat + "," + model.lng + ");")
24 }
25 },
26@@ -341,6 +343,8 @@
27 mainPageStack.removePages(mainPageStack.primaryPage)
28 mainPageStack.center_onpos = 2;
29 mainPageStack.routeState = 'yes';
30+ mainPageStack.endLat = model.lat;
31+ mainPageStack.endLng = model.lng;
32 mainPageStack.executeJavaScript("calc2coord("+ model.lat + "," + model.lng + ");");
33 }
34 },
35
36=== modified file 'qml/Main.qml'
37--- qml/Main.qml 2016-06-08 17:16:09 +0000
38+++ qml/Main.qml 2016-06-14 19:54:18 +0000
39@@ -150,6 +150,7 @@
40 property int center_onpos: 0 // 0 GPS off, 1 GPS on + not center, 2 GPS on + center
41 property bool favPopup: false
42 property bool onLoadingExecuted: false
43+ property bool forcedReload: false
44
45 property string usContext: "messaging://"
46 // Workaround: as long as map keeps a webcontainer this function handles js events to the webview.
47@@ -368,19 +369,18 @@
48
49 case "http://show_notification/":
50 if (params[1] === "speed_camera_error") {
51- notifiactionBar.text = i18n.tr("Error getting speed cameras!");
52+ notificationBar.text = i18n.tr("Error getting speed cameras!");
53 }
54- switch (params[0]) {
55+ switch (params[0]) {
56 case "info":
57- notifiactionBar.info();
58+ notificationBar.info();
59 break;
60 case "warning":
61- notifiactionBar.warning();
62+ notificationBar.warning();
63 break;
64 case "critical":
65- notifiactionBar.critical();
66+ notificationBar.critical();
67 }
68- notificationBarTimer.start();
69 }
70 request.action = Oxide.NavigationRequest.ActionReject;
71 }
72@@ -424,12 +424,31 @@
73 goThereActionPopover.show();
74 }
75 }
76+
77+ if (mainPageStack.forcedReload === true && mainPageStack.routeState === "yes") {
78+ mainPageStack.forcedReload = false
79+ notificationBar.close()
80+
81+ if ( mainPageStack.endLat !== "null" && mainPageStack.endLng !== "null") {
82+ mainPageStack.center_onpos = 2;
83+ mainPageStack.executeJavaScript("calc2coord(" + mainPageStack.endLat + "," + mainPageStack.endLng + ");");
84+ notificationBar.text = i18n.tr("Active route lost. Recalculating it…");
85+ notificationBar.warning();
86+ } else {
87+ notificationBar.text = i18n.tr("Sorry, route info could not be retrieved. Please, re-set the route.");
88+ notificationBar.warning();
89+ }
90+ }
91 }
92 }
93
94 onGeolocationPermissionRequested: { request.allow() }
95 }
96
97+ WebProcessMonitor{
98+ webview: webview
99+ }
100+
101 Connections {
102 target: UriHandler
103 onOpened: {
104@@ -522,17 +541,8 @@
105 }
106 }
107
108- Timer {
109- id: notificationBarTimer
110- interval: 8000
111- repeat: false
112- onTriggered: {
113- notifiactionBar.visible = false
114- }
115- }
116-
117 NotificationBar {
118- id: notifiactionBar
119+ id: notificationBar
120 }
121
122 XmlListModel {
123@@ -648,9 +658,8 @@
124 Column {
125 height: routePageGrid.height
126 spacing: 0
127- Component.onCompleted: {
128- mainPageStack.executeJavaScript("qml_set_route_status();")
129- }
130+
131+ Component.onCompleted: mainPageStack.executeJavaScript("qml_set_route_status()")
132
133 Row {
134 id: gridRow
135@@ -731,6 +740,8 @@
136 goThereActionPopover.hide();
137 mainPageStack.center_onpos = 2;
138 mainPageStack.routeState = 'yes';
139+ mainPageStack.endLat = mainPageStack.clickedLat;
140+ mainPageStack.endLng = mainPageStack.clickedLng;
141 mainPageStack.executeJavaScript("calc2coord(" + mainPageStack.clickedLat + ", " + mainPageStack.clickedLng + ");");
142 }
143 }
144@@ -830,6 +841,8 @@
145 goThereActionPopover.hide();
146 mainPageStack.center_onpos = 2;
147 mainPageStack.routeState = 'yes';
148+ mainPageStack.endLat = mainPageStack.clickedLat;
149+ mainPageStack.endLng = mainPageStack.clickedLng;
150 mainPageStack.executeJavaScript("calc2coord(" + mainPageStack.clickedLat + ", " + mainPageStack.clickedLng + ");");
151 }
152 }
153
154=== modified file 'qml/PoiDetailsPage.qml'
155--- qml/PoiDetailsPage.qml 2016-06-03 13:50:14 +0000
156+++ qml/PoiDetailsPage.qml 2016-06-14 19:54:18 +0000
157@@ -199,6 +199,8 @@
158 }
159 mainPageStack.center_onpos = 2;
160 mainPageStack.routeState = 'yes'
161+ mainPageStack.endLat = mainPageStack.clickedLat;
162+ mainPageStack.endLng = mainPageStack.clickedLng;
163 mainPageStack.executeJavaScript("calc2coord("+ mainPageStack.clickedLat + "," + mainPageStack.clickedLng + ");");
164 } else if (model.mode === "SAVE") {
165 var incubator = mainPageStack.addPageToCurrentColumn(poiDetailsPage, Qt.resolvedUrl("SearchPage.qml"), {favLat: mainPageStack.clickedLat, favLng: mainPageStack.clickedLng, favName: poiName});
166
167=== modified file 'qml/PoiListPage.qml'
168--- qml/PoiListPage.qml 2016-05-05 19:21:33 +0000
169+++ qml/PoiListPage.qml 2016-06-14 19:54:18 +0000
170@@ -273,6 +273,8 @@
171 mainPageStack.removePages(mainPageStack.primaryPage)
172 mainPageStack.center_onpos = 2;
173 mainPageStack.routeState = 'yes'
174+ mainPageStack.endLat = model.lat;
175+ mainPageStack.endLng = model.lng;
176 mainPageStack.executeJavaScript("calc2coord("+ model.lat + "," + model.lng + ");");
177 }
178 },
179
180=== modified file 'qml/SearchPage.qml'
181--- qml/SearchPage.qml 2016-05-05 19:21:33 +0000
182+++ qml/SearchPage.qml 2016-06-14 19:54:18 +0000
183@@ -27,9 +27,7 @@
184 property string favLng
185 property string favName
186
187- Component.onCompleted: {
188- mainPageStack.executeJavaScript("qml_set_route_status();")
189- }
190+ Component.onCompleted: mainPageStack.executeJavaScript("qml_set_route_status()")
191
192 Component.onDestruction: {
193 // Hide 2nd column when returning to the map to avoid an empty white column
194
195=== modified file 'qml/components/NotificationBar.qml'
196--- qml/components/NotificationBar.qml 2016-05-22 17:20:29 +0000
197+++ qml/components/NotificationBar.qml 2016-06-14 19:54:18 +0000
198@@ -25,30 +25,41 @@
199 signal info()
200 signal warning()
201 signal critical()
202+ signal close()
203+
204+ onClose: {
205+ notificationBar.visible = false
206+ }
207
208 onInfo: {
209+ close()
210 notificationLabel.color = UbuntuColors.slate
211 notificationRectangle.color = "#FFFFFF"
212 notificationRectangle.visible = true;
213 notificationIcon.name = ""
214 notificationLabel.fontSize = "medium"
215 notificationRectangle.height = notificationLabel.height + units.gu(1)
216+ notificationBarTimer.restart()
217 }
218 onWarning: {
219+ close()
220 notificationLabel.color = "#FFFFFF"
221 notificationRectangle.color = UbuntuColors.orange;
222 notificationRectangle.visible = true;
223 notificationLabel.fontSize = "large"
224 notificationIcon.name = "dialog-warning-symbolic"
225 notificationRectangle.height = notificationLabel.height + units.gu(4)
226+ notificationBarTimer.restart()
227 }
228 onCritical: {
229+ close()
230 notificationLabel.color = "#FFFFFF"
231 notificationRectangle.color = UbuntuColors.red
232 notificationRectangle.visible = true;
233 notificationLabel.fontSize = "large"
234 notificationIcon.name = "dialog-error-symbolic"
235 notificationRectangle.height = notificationLabel.height + units.gu(4)
236+ notificationBarTimer.restart()
237 }
238
239 width: parent.width
240@@ -58,6 +69,14 @@
241 top: goThereActionPopover.isShown ? goThereActionPopover.bottom : navigationPage.header.bottom
242 }
243
244+ MouseArea {
245+ anchors.fill: parent
246+ onClicked: {
247+ notificationBarTimer.stop()
248+ notificationBar.visible = false
249+ }
250+ }
251+
252 Icon {
253 id: notificationIcon
254 width: units.gu(3.5)
255@@ -80,9 +99,18 @@
256 right: parent.right
257 verticalCenter: parent.verticalCenter
258 }
259- horizontalAlignment: Text.AlignHCenter
260- maximumLineCount: 3
261+ horizontalAlignment: notificationIcon.visible ? Text.AlignLeft : Text.AlignHCenter
262+ maximumLineCount: 6
263 wrapMode: Text.WordWrap
264 elide: Text.ElideRight
265 }
266+
267+ Timer {
268+ id: notificationBarTimer
269+ interval: 8000
270+ repeat: false
271+ onTriggered: {
272+ notificationBar.close()
273+ }
274+ }
275 }
276
277=== modified file 'qml/components/POIQuickAccessGridView.qml'
278--- qml/components/POIQuickAccessGridView.qml 2016-05-22 18:08:53 +0000
279+++ qml/components/POIQuickAccessGridView.qml 2016-06-14 19:54:18 +0000
280@@ -87,17 +87,15 @@
281 if (status === XmlListModel.Error) {
282 console.log(errorString());
283 source = "";
284- notificationBarTimer.start();
285- notifiactionBar.text = i18n.tr("Error getting results. Please, check your data connection.");
286- notifiactionBar.critical();
287+ notificationBar.text = i18n.tr("Error getting results. Please, check your data connection.");
288+ notificationBar.critical();
289 }
290 if (status === XmlListModel.Ready && count === 0) {
291- notificationBarTimer.start();
292- notifiactionBar.text = i18n.tr("Sorry, no results found nearby.");
293- notifiactionBar.info();
294+ notificationBar.text = i18n.tr("Sorry, no results found nearby.");
295+ notificationBar.info();
296 }
297 if (status === XmlListModel.Ready && count >> 0) {
298- notifiactionBar.visible = false;
299+ notificationBar.close()
300 mainPageStack.executeJavaScript("ui.markers_POI_set(" + JSON.stringify(poiXmlModel.allPOI()) + ")");
301 mainPageStack.center_onpos = 1;
302 goThereActionPopover.hide();
303@@ -170,13 +168,13 @@
304 mainPageStack.addPageToNextColumn(mainPageStack.primaryPage, Qt.resolvedUrl("../PoiQuickAccessPage.qml"))
305 } else {
306 if (mainPageStack.currentLat === 'null' || mainPageStack.currentLng === 'null') {
307- notifiactionBar.text = i18n.tr("Current position unknown. Try again after a position update.");
308- notifiactionBar.info();
309+ notificationBar.text = i18n.tr("Current position unknown. Try again after a position update.");
310+ notificationBar.info();
311 }
312 else {
313 poiXmlModel.clear();
314- notifiactionBar.text = i18n.tr("Searching…");
315- notifiactionBar.info();
316+ notificationBar.text = i18n.tr("Searching…");
317+ notificationBar.info();
318 poiXmlModel.distance = model.distance;
319 poiXmlModel.clause = model.clause;
320 poiXmlModel.search();
321
322=== added file 'qml/components/WebProcessMonitor.qml'
323--- qml/components/WebProcessMonitor.qml 1970-01-01 00:00:00 +0000
324+++ qml/components/WebProcessMonitor.qml 2016-06-14 19:54:18 +0000
325@@ -0,0 +1,92 @@
326+/*
327+ * Copyright 2015 Canonical Ltd.
328+ *
329+ * This file is part of webbrowser-app.
330+ *
331+ * webbrowser-app is free software; you can redistribute it and/or modify
332+ * it under the terms of the GNU General Public License as published by
333+ * the Free Software Foundation; version 3.
334+ *
335+ * webbrowser-app is distributed in the hope that it will be useful,
336+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
337+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
338+ * GNU General Public License for more details.
339+ *
340+ * You should have received a copy of the GNU General Public License
341+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
342+ */
343+
344+import QtQuick 2.4
345+import com.canonical.Oxide 1.9 as Oxide
346+
347+Item {
348+ id: monitor
349+
350+ visible: false
351+
352+ property var webview: null
353+
354+ readonly property bool killed: webview &&
355+ (webview.webProcessStatus === Oxide.WebView.WebProcessKilled)
356+ readonly property bool crashed: webview &&
357+ (webview.webProcessStatus === Oxide.WebView.WebProcessCrashed)
358+
359+ // When the renderer process is killed (most likely by the system’s
360+ // OOM killer), try to reload the page once, and if this results in
361+ // the process being killed again within one minute, then display
362+ // the sad tab.
363+
364+ readonly property int killedRetries: internal.killedRetries
365+
366+ QtObject {
367+ id: internal
368+ property int killedRetries: 0
369+ }
370+
371+ Connections {
372+ target: webview
373+ onWebProcessStatusChanged: {
374+ if (webview.webProcessStatus === Oxide.WebView.WebProcessKilled) {
375+ if (internal.killedRetries == 0) {
376+ // Do not attempt reloading right away, this would result in a crash
377+ delayedReload.restart()
378+ console.log("Webview was killed!")
379+ if (mainPageStack.routeState === "yes" ) {
380+ notificationBar.text = i18n.tr("Something went wrong.")
381+ + "\n" + i18n.tr("Reloading the map and the last active route…")
382+ } else {
383+ notificationBar.text = i18n.tr("Something went wrong.")
384+ + "\n" + i18n.tr("Reloading the map…")
385+ }
386+ notificationBar.critical()
387+ }
388+ }
389+ }
390+ }
391+
392+ Timer {
393+ id: delayedReload
394+ interval: 100
395+ onTriggered: {
396+ monitorTimer.restart()
397+ mainPageStack.onLoadingExecuted = false
398+ mainPageStack.forcedReload = true
399+ navigationPage.buttonsEnabled = false
400+ monitor.webview.reload()
401+ console.log("Trying to reload the webview…")
402+ internal.killedRetries++
403+ }
404+ }
405+
406+ Timer {
407+ id: monitorTimer
408+ interval: 60000 // 1 minute
409+ onTriggered: internal.killedRetries = 0
410+ }
411+
412+ onWebviewChanged: {
413+ internal.killedRetries = 0
414+ delayedReload.stop()
415+ monitorTimer.stop()
416+ }
417+}

Subscribers

People subscribed via source and target branches