Merge lp:~costales/unav/0.64-webview-empty-then-reload into lp:unav

Proposed by costales on 2016-12-06
Status: Needs review
Proposed branch: lp:~costales/unav/0.64-webview-empty-then-reload
Merge into: lp:unav
Diff against target: 17 lines (+4/-2)
1 file modified
qml/Main.qml (+4/-2)
To merge this branch: bzr merge lp:~costales/unav/0.64-webview-empty-then-reload
Reviewer Review Type Date Requested Status
Olivier Tilloy 2016-12-06 Needs Information on 2016-12-06
Review via email: mp+312573@code.launchpad.net

Description of the change

Reload webview if there is an error calling a function in the webview.
All the times that I saw the webview empty, it generated the error http://paste.ubuntu.com/23521218/ then I'll thrush in reload it when there is an error.
Anyway, I have to test this a lot before and check it's working in all issues.

To post a comment you must log in.
JkB (joergberroth) wrote :

There was already a crash handler in one of my branches. Check that, too.
There wad quite nice stuff.....

Am Dienstag, 6. Dezember 2016 15:19:39 CET schrieb costales
<email address hidden>:
> The proposal to merge
> lp:~costales/unav/0.64-webview-empty-then-reload into lp:unav
> has been updated.
>
> Description changed to:
>
> Reload webview if there is an error calling a function in the webview.
> All the times that I saw the webview empty, it generated the
> error http://paste.ubuntu.com/23521218/ then I'll thrush in
> reload it when there is an error.
> Anyway, I have to test this a lot before and check it's working
> in all issues.
>
> For more details, see:
> https://code.launchpad.net/~costales/unav/0.64-webview-empty-then-reload/+merge/312573

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

costales (costales) wrote :

@Joerg: Yes :) I checked your branch and it has empty states yet (I don't know why) :(

With this branch I want to check if all empty states are fixed or not :) It will take me a few weeks to test that :S

A hug Joerg!

Olivier Tilloy (osomon) wrote :

In the log you linked to, I’m not seeing the console log ["Error " + code + ": " + explanation] anywhere, are you sure the renderer is killed in executeJavaScript(…) ?

Could you please detail under which circumstances the page becomes blank (in the bug report)?
Have you tried monitoring the value of webProcessStatus¹ on the webview? This would provide useful information as to what is really happening (is the renderer process crashing, or is it simply taken down by the system’s OOM killer).

It would probably be useful to keep the error code and explanation in the log too, to ease debugging.

¹ http://people.canonical.com/~chrisccoulson/oxide-documentation/qml-com-canonical-oxide-webview.html#webProcessStatus-prop

review: Needs Information
136. By costales on 2016-12-06

Added webview.webProcessStatus for testing

costales (costales) wrote :

Hi Olivier :)

> are you sure the renderer is killed in executeJavaScript(…) ?

I don't know where.

> circumstances the page becomes blank (in the bug report)?

I think when the app returns to foreground the system kills the rendered,
but I can't confirm that.
I saw this behavior in other apps (specially in previous OTAs), but in uNav
(I think) it is because it has an heavy use of CPU & RAM.

> Have you tried monitoring the value of webProcessStatus¹ on the webview?

No, I added this dump now (revision 136
<http://bazaar.launchpad.net/~costales/unav/0.64-webview-empty-then-reload/revision/136>,
If
you see it is wrong, please, ping me):

            function executeJavaScript(code) {
                  console.log('STATUS WEBVIEW: ' +
webview.webProcessStatus);
                  <.....>

> probably be useful to keep the error code and explanation

I restored them. Anyway, they are always like in my log in ubuntu pastebin.

I'll test it these weeks, with the new webProcessStatus and check if the
reload it's working in all cases.

Thanks Olivier!

costales (costales) wrote :

I get an empty renderer and this was the log:
    http://paste.ubuntu.com/23618857/

The dump of:
    console.log('STATUS WEBVIEW: ' + webview.webProcessStatus);
was:
    qml: STATUS WEBVIEW: 1
then the system killed it.

But the webview didn't reload either with this line:
    webview.reload();

Thanks in advance!

JkB (joergberroth) wrote :

Hi,

there is an old branch, which I already worked on:
https://code.launchpad.net/~joergberroth/unav/WebViewCrashHandling

It is based on your monitor and handles crashes in a way that the last
active route (if there was one) will be set again after the webview has
been reloaded.

Maybe one can use this as a base.

Best regards
Joerg

Am 21.02.2017 um 15:38 schrieb Olivier Tilloy:
> Have a look at how this is used in the browser app: https://bazaar.launchpad.net/~phablet-team/webbrowser-app/trunk/view/head:/src/app/WebProcessMonitor.qml
>

Unmerged revisions

136. By costales on 2016-12-06

Added webview.webProcessStatus for testing

135. By costales on 2016-12-06

If webview empty > reload it

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'qml/Main.qml'
2--- qml/Main.qml 2016-09-26 19:42:17 +0000
3+++ qml/Main.qml 2016-12-06 14:17:08 +0000
4@@ -172,10 +172,12 @@
5 function executeJavaScript(code) {
6 var req = webview.rootFrame.sendMessage(mainPageStack.usContext, "EXECUTE", {code: code});
7 //req.onreply = function (msg) {
8- //console.log(msg.str);
9+ //console.log(msg.str);
10 //}
11 req.onerror = function (code, explanation) {
12- console.log("Error " + code + ": " + explanation)
13+ mainPageStack.onLoadingExecuted = false;
14+ webview.reload();
15+ console.log("ERROR: Reloaded webview");
16 }
17 }
18

Subscribers

People subscribed via source and target branches