Oxide

isError is false for committed event on error page after restoring a browsing session

Bug #1423531 reported by Olivier Tilloy
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Oxide
Fix Released
Medium
Olivier Tilloy
1.4
Won't Fix
Undecided
Unassigned
1.5
Fix Released
Medium
Olivier Tilloy
Oxide 1.5.5

Bug Description

Since the fix for bug #1398941, if I point a webview to http://invalid, I’m seeing the following sequence of load events:

   started, !isError
   failed, !isError
   committed, isError

This works as expected. However it doesn’t work correctly if I save this webview’s state and restore it. I’m then seeing the same sequence of events, but isError is false for the last committed event.

I’m attaching a simple test app that demonstrates the issue.

Related branches

lp:~osomon/oxide/restore-error-page-unit-test
Chris Coulson: Approve
Diff: 82 lines (+63/-0)
2 files modified
qt/tests/qmltests/api/tst_WebView_loading.qml (+23/-0)
qt/tests/qmltests/api/tst_WebView_save_restore_state.qml (+40/-0)
lp:~osomon/oxide/backport-42-patch-for-isError
Chris Coulson: Approve
Diff: 298 lines (+266/-0)
4 files modified
patches/bug1423531.patch (+202/-0)
patches/series (+1/-0)
qt/tests/qmltests/api/tst_WebView_loading.qml (+23/-0)
qt/tests/qmltests/api/tst_WebView_save_restore_state.qml (+40/-0)
lp:oxide/1.5
lp:~oxide-developers/oxide/packaging.utopic
lp:~oxide-developers/oxide/packaging.vivid
lp:~oxide-developers/oxide/packaging.trusty
Revision history for this message
Olivier Tilloy (osomon) wrote :
Revision history for this message
Olivier Tilloy (osomon) wrote :

I can reproduce the issue with 1.4.3 as well as with the latest trunk (revision 966).

Revision history for this message
Olivier Tilloy (osomon) wrote :

When reloading a page from a session restore, I’m seeing oxide::WebView::DidCommitProvisionalLoadForFrame() being called, and the page type of the last committed entry is PAGE_TYPE_NORMAL (whereas I would expect PAGE_TYPE_ERROR).

The page type of a navigation entry is not serialized, which I guess makes sense. Not sure how to ensure that the type is set to PAGE_TYPE_ERROR after the fact when the entry is restored though. There’s only one place in chromium where the page type of a navigation entry is set to PAGE_TYPE_ERROR, and that is in content::NavigationControllerImpl::RendererDidNavigateToNewPage(…), but this is not called when restoring a navigation session.

Revision history for this message
Chris Coulson (chrisccoulson) wrote :

Which RendererDidNavigate*() function does get called from NavigationControllerImpl::RendererDidNavigate()? It seems like this is probably a Chromium bug

Revision history for this message
Olivier Tilloy (osomon) wrote :

Just tested again with the latest trunk (revision 990), and the bug isn’t there anymore.

Somewhere between chromium 42.0.2305.0 and 42.0.2311.11 the issue was fixed.

Changed in oxide:
status: New → Invalid
Revision history for this message
Chris Coulson (chrisccoulson) wrote :

It's fixed by https://chromium.googlesource.com/chromium/src.git/+/7431bb2983db683dcc4556e1918556254c3d5589. As there's a reproducible case, would you mind adding the test case to trunk (tst_bug1423531.qml or something)?

Revision history for this message
Chris Coulson (chrisccoulson) wrote :

Would probably be good to test reloading the error page too...

Revision history for this message
Olivier Tilloy (osomon) wrote :

Looks like the fix could be https://chromium.googlesource.com/chromium/src/+/7431bb2983db683dcc4556e1918556254c3d5589.

Revision history for this message
Olivier Tilloy (osomon) wrote :

Ha, you beat me to it. Ok, reopening the bug to track the addition of a unit test.

Changed in oxide:
status: Invalid → Confirmed
importance: Undecided → Medium
assignee: nobody → Olivier Tilloy (osomon)
Olivier Tilloy (osomon)
Changed in oxide:
status: Confirmed → In Progress
Olivier Tilloy (osomon)
Changed in oxide:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Bug attachments

Remote bug watches

Bug watches keep track of this bug in other bug trackers.