New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 676235 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Get site isolation webkit tests to pass

Project Member Reported by ananta@chromium.org, Dec 21 2016

Issue description

The site-isolation webkit_tests
http/tests/intersection-observer/iframe-cross-origin.html fail with PlzNavigate.

This trace is the cause.
3637:3637:1220/102450.802932:6083714579:WARNING:render_frame_host_impl.cc(2116)]
OnDidStopLoading was called twice.

The FrameHostMsg_OnDidStartLoading notification is not sent for PlzNavigate with site-
per-process. We should set the is_loading_ flag when we receive the navigation request. 


 

Comment 1 by jam@chromium.org, Dec 21 2016

Are you sure that's the cause? That log warning happens often with and without plznavigate.

FrameHostMsg_DidStartLoading isn't sent from the renderer for plznavigate because the throbber is tracked differently with plznavigate.

Comment 2 by ananta@chromium.org, Dec 21 2016

Yeah. the trace is not related. 

Comment 3 by ananta@chromium.org, Dec 21 2016

Summary: Get site isolation webkit tests to pass (was: PlzNavigate: Get site isolation webkit tests to pass)
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3bdd8aea0439f2c6a50d24c3c805bc7e579d5a2c

commit 3bdd8aea0439f2c6a50d24c3c805bc7e579d5a2c
Author: ananta <ananta@chromium.org>
Date: Thu Dec 22 17:11:55 2016

Ensure that duplicate navigations are tagged as reload in the browser.
This includes the cases where we are navigating to a URL which was
committed previously or to a URL for which we haven't received a commit.

scottmg's patch for reference is here https://codereview.chromium.org/2381503003. This was reverted due to failures on Android.

The failure was seen in cases like this. Navigation to url1->url2->url1.
If the first navigation is not committed yet, then we would treat the last
navigation to url1 as a reload. This causes inbox on Android to hang.

The proposed fix is to use the last pending navigation if it exists or
the last committed navigation to compare whether the current navigation is
a reload. This fixes the issues with Android

Changes in this patch are as below:
1. Add members in the NavigationControllerImpl class to hold the last
   pending navigation entry, its index, and the last transient index.
   The last transient index is used to avoid treating navigations as
   reloads if we have an interstitial being displayed.

2. Add a flag in the NavigationEntryImpl class to indicate whether there
   was a SSL error in the navigation. This was added to fix browser test
   failures. We avoid treating navigations as reloads when the last
    pending navigation has a SSL error.

3. We compare the url of the last pending or committed entry with the
   current navigation entry to see if it is a reload. There are a number
   of cases to consider here. Please look at the code.

4. Added a unittest NavigationControllerTest.MultipleNavigationsAndReload
   which tests various combinations of pending and committed navigations
   to ensure that the navigation is tagged correctly as reload or not.

The non plznavigate case does not have the bug when the EnsureSamePageNavigationUpdatesFrameNavigationEntry test because Blink tells us the correct navigation history, i.e. whether a navigation is a
reload, etc.

R=nasko@chromium.org,clamy@chromium.org
BUG=630103,  475027 , 536102,  664319 , 676235
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2580753002
Cr-Commit-Position: refs/heads/master@{#440443}

[modify] https://crrev.com/3bdd8aea0439f2c6a50d24c3c805bc7e579d5a2c/content/browser/frame_host/navigation_controller_impl.cc
[modify] https://crrev.com/3bdd8aea0439f2c6a50d24c3c805bc7e579d5a2c/content/browser/frame_host/navigation_controller_impl.h
[modify] https://crrev.com/3bdd8aea0439f2c6a50d24c3c805bc7e579d5a2c/content/browser/frame_host/navigation_controller_impl_unittest.cc
[modify] https://crrev.com/3bdd8aea0439f2c6a50d24c3c805bc7e579d5a2c/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/3bdd8aea0439f2c6a50d24c3c805bc7e579d5a2c/content/browser/frame_host/navigation_entry_impl.h
[modify] https://crrev.com/3bdd8aea0439f2c6a50d24c3c805bc7e579d5a2c/content/browser/ssl/ssl_manager.cc

Comment 5 by nasko@chromium.org, Jan 18 2018

Components: Internals>Sandbox>SiteIsolation
Owner: ----
Status: Untriaged (was: Assigned)
Unassigning all bugs for ananta@chromium.org

Sign in to add a comment