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

Issue 699387 link

Starred by 3 users
Status: Fixed
Owner:
cblume@chromium.org
Closed: Mar 2017
Cc:
jam@chromium.org
█ aelias@chromium.org
█ ramyan@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Proj-Servicification

Blocking:
issue 699394



Sign in to add a comment

PlzNavigate fails new tab page tests

Project Member Reported by cblume@chromium.org, Mar 8 2017 
The new tab page tests
org.chromium.chrome.browser.ntp.NewTabPageTest#testUrlFocusAnimationsEnabledOnFailedLoad
org.chromium.chrome.browser.toolbar.ToolbarTest#testNTPNavigatesToErrorPageOnDisconnectedNetwork
are failing on Chromium for Android with PlzNavigate.

To repro,
$ git cl patch 2385413004   // this forces PlzNavigate without requiring a switch
$ ./out/Debug/bin/run_chrome_public_test_apk_incremental -f "*UrlFocusAnimationsEnabledOnFailedLoad*"
// or "*NTPNavigatesToErrorPageOnDisconnectedNetwork*"

Comment 1 by cblume@chromium.org, Mar 8 2017

Blocking: 699394

Comment 2 by wychen@chromium.org, Mar 23 2017 

https://codereview.chromium.org/2760463005/ fixed the following test:
ToolbarTest#testNTPNavigatesToErrorPageOnDisconnectedNetwork

Comment 3 by jam@chromium.org, Mar 24 2017 

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

commit cc2df16e35d1016a66a1b457b4070cc50a54e045
Author: jam <jam@chromium.org>
Date: Thu Mar 23 23:54:01 2017

Fix handling of external protocols with PlzNavigate.

This fixes
UrlOverridingTest#testNavigationFromXHRCallbackAndLongTimeout
ToolbarTest#testNTPNavigatesToErrorPageOnDisconnectedNetwork
externalnav.UrlOverridingTest#testNavigationFromTimer

Once is_external_protocol was set correctly, the second bug that happened was because with PlzNavigate we were making requests for chrome-native schemes. This caused NavigationHandleImpl::WillStartRequest to be called, which added navigation throttles and that ended up calling the Android navigation interceptor. It's not called without PlzNavigate, which led to these tests failures.

BUG= 659089 

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

[modify] https://crrev.com/cc2df16e35d1016a66a1b457b4070cc50a54e045/chrome/common/chrome_content_client.cc
[modify] https://crrev.com/cc2df16e35d1016a66a1b457b4070cc50a54e045/chrome/renderer/chrome_render_thread_observer.cc
[modify] https://crrev.com/cc2df16e35d1016a66a1b457b4070cc50a54e045/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/cc2df16e35d1016a66a1b457b4070cc50a54e045/content/common/navigation_params.cc
[modify] https://crrev.com/cc2df16e35d1016a66a1b457b4070cc50a54e045/content/common/url_schemes.cc
[modify] https://crrev.com/cc2df16e35d1016a66a1b457b4070cc50a54e045/content/public/common/content_client.h
[modify] https://crrev.com/cc2df16e35d1016a66a1b457b4070cc50a54e045/third_party/WebKit/Source/platform/weborigin/SchemeRegistry.cpp
[modify] https://crrev.com/cc2df16e35d1016a66a1b457b4070cc50a54e045/third_party/WebKit/Source/platform/weborigin/SchemeRegistry.h
[modify] https://crrev.com/cc2df16e35d1016a66a1b457b4070cc50a54e045/third_party/WebKit/Source/web/WebSecurityPolicy.cpp
[modify] https://crrev.com/cc2df16e35d1016a66a1b457b4070cc50a54e045/third_party/WebKit/public/web/WebSecurityPolicy.h
[modify] https://crrev.com/cc2df16e35d1016a66a1b457b4070cc50a54e045/url/url_util.cc
[modify] https://crrev.com/cc2df16e35d1016a66a1b457b4070cc50a54e045/url/url_util.h
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 24 2017 

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

commit adbac71e30fdf997eb5b543da155b08542f88c71
Author: John Abd-El-Malek <jam@chromium.org>
Date: Fri Mar 24 18:03:15 2017

Ensure that the PlzNavigate code path removes pending NavigationEntrys if the navigation is cancelled.

For the old code paths, this is done by NavigatorImpl::DidFailProvisionalLoadWithError. So copy that call.

This fixes
NewTabPageTest#testUrlFocusAnimationsEnabledOnFailedLoad

BUG= 699387 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
R=cblume@chromium.org, creis@chromium.org

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

[modify] https://crrev.com/adbac71e30fdf997eb5b543da155b08542f88c71/content/browser/frame_host/frame_tree_node.cc
[modify] https://crrev.com/adbac71e30fdf997eb5b543da155b08542f88c71/content/browser/frame_host/navigation_controller_impl_browsertest.cc

Comment 5 by cblume@chromium.org, Mar 24 2017

Status: Fixed (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 24 2017 

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

commit cd0b7b2029d532fd338ee0e5e7839987895971ab
Author: jam <jam@chromium.org>
Date: Fri Mar 24 22:13:05 2017

Ensure that when a new navigation cancels an existing one, the old navigation's DidFinishNavigation callback has an net::ERR_ABORTED error code.

This fixes
UrlOverridingTest#testNavigationWithFallbackURL

BUG= 699387 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/cd0b7b2029d532fd338ee0e5e7839987895971ab/content/browser/frame_host/frame_tree_node.cc
[modify] https://crrev.com/cd0b7b2029d532fd338ee0e5e7839987895971ab/content/browser/frame_host/navigation_handle_impl_browsertest.cc

Comment 7 by laforge@google.com, Nov 7 2017

Components: Internals>Network>Service

Comment 8 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
►

Sign in to add a comment