New issue
Advanced search Search tips

Issue 674303 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Proj-Servicification

Blocking:
issue 673806



Sign in to add a comment

org.chromium.chrome.browser.contextualsearch.ContextualSearchManagerTest_testPrefetchFailoverRequestMadeAfterOpen failing with PlzNavigate

Project Member Reported by jam@chromium.org, Dec 14 2016

Issue description

See this sample test run:
https://codereview.chromium.org/2574953002/#ps80001
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/199299

To run the test with PlzNavigate, add --enable-browser-side-navigation
 

Comment 1 by donnd@chromium.org, Jan 18 2017

This test checks that a low priority request that fails will be followed by a normal priority request when needed.  We call stop() on the failed request, but that asserts because there's not yet any valid request pending that can be stopped.  It appears that there's no valid request because we immediately simulate failure of the request without waiting for it to start. 

I think we should not be starting and stopping real search requests in our tests, rather the network calls should be stubbed out in the NetworkCommunicator.  This test failure seems like a compelling reason to vector stop() so it's not attempted in tests.

Comment 2 by donnd@chromium.org, Jan 18 2017

Components: UI>Browser>Mobile>TouchToSearch
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 19 2017

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

commit 51787340a7c9f2c1c84a522f7dc8ac0b4025440f
Author: donnd <donnd@chromium.org>
Date: Thu Jan 19 22:18:32 2017

[TTS] Fix a test that's failing with PlzNavigate enabled.

Fix ContextualSearchManagerTest_testPrefetchFailoverRequestMadeAfterOpen
failing with PlzNavigate.  When we simulate a navigation failure we
follow that with a call to stop navigation.  This causes
NavigationHandleImpl::GetPageTransition to fail an assert because
the navigation has not actually started yet, due to the way we
partially fake navigation in tests.

This CL vectors stopping the navigation through our NetworkCommunicator,
since network control actions should not run live in tests.

BUG= 674303 

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

[modify] https://crrev.com/51787340a7c9f2c1c84a522f7dc8ac0b4025440f/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
[modify] https://crrev.com/51787340a7c9f2c1c84a522f7dc8ac0b4025440f/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchNetworkCommunicator.java
[modify] https://crrev.com/51787340a7c9f2c1c84a522f7dc8ac0b4025440f/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchFakeServer.java

Comment 4 by donnd@chromium.org, Jan 24 2017

Status: Fixed (was: Assigned)

Comment 5 by jam@chromium.org, Jan 24 2017

Thank you!

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

Components: Internals>Network>Service

Comment 7 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