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

Bug #1423531 reported by Olivier Tilloy on 2015-02-19
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Oxide
Medium
Olivier Tilloy
1.4
Undecided
Unassigned
1.5
Medium
Olivier Tilloy

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

Olivier Tilloy (osomon) wrote :
Olivier Tilloy (osomon) wrote :

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

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.

Chris Coulson (chrisccoulson) wrote :

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

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
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)?

Chris Coulson (chrisccoulson) wrote :

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

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) on 2015-03-02
Changed in oxide:
status: Confirmed → In Progress
Olivier Tilloy (osomon) on 2015-03-04
Changed in oxide:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Bug attachments