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

Issue 808011 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

NTP-related browser tests failing in official builds.

Project Member Reported by h...@chromium.org, Feb 1 2018

Issue description

The set of failed tests look pretty similar between 32-bit and 64-bit:
https://ci.chromium.org/buildbot/chromium.clang/ToTWin64/863
https://ci.chromium.org/buildbot/chromium.clang/ToTWin/897

There are also browser_tests failures on the dbg bots, but the set of tests looks a little bit different.

Not sure if it's actually ToT-specific, but I'm trying to find that out.
 
Cc: blundell@chromium.org
Looks like it started here:

https://ci.chromium.org/buildbot/chromium.clang/ToTWin64/798
https://ci.chromium.org/buildbot/chromium.clang/ToTWin/830

The failures all look ntp-related, e.g.

../../chrome/browser/ui/browser_browsertest.cc(1158): error: Expected equality of these values:
  chrome::kChromeUINewTabURL
    Which is: 00000001461B5C70 pointing to "chrome://newtab/"
  tab_strip_model->GetActiveWebContents()->GetLastCommittedURL().spec()
    Which is: "chrome-search://local-ntp/local-ntp.html"


So maybe it's just an official build thing like you guessed off-bug.

blundell's https://chromium-review.googlesource.com/870313 mentions "ntp" -- blundell, any chance that that change changed the ntp url in official builds?
They fail on https://uberchromegw.corp.google.com/i/official.desktop.continuous/builders/win%20trunk/ so very likely an official-build-thing, not a clang-tot-thing -- so not a roll blocker.
Cc: treib@chromium.org cco3@chromium.org
Summary: NTP-related browser tests failing in official builds. (was: browser_tests failing on Clang's ToTWin and ToTWin64 builders)
 https://chromium-review.googlesource.com/868994 looks related too.


Here's a list of failing tess:

InputImeApiTest.SendKeyEventsOnNormalPage
StartupBrowserCreatorTest.ProfilesWithoutPagesNotLaunched
ContentScriptApiTests/ContentScriptApiTest.CannotScriptTheNewTabPage/0
ContentScriptApiTests/ContentScriptApiTest.CannotScriptTheNewTabPage/1
BrowserTest.ClearPendingOnFailUnlessNTP
ExtensionApiTabTest.TabsOnUpdated
BrowserTest.NavigateToDefaultNTPPageOnExtensionUnload
AllUrlsApiTest.WhitelistedExtension
ExtensionURLRewriteBrowserTest.NewTabPageURL
PolicyTest.HomepageLocation
BrowserNavigatorTest.NavigateFromNTPToOptionsInSameTab

(see above for an example failure)
It shouldn't be the change to NTP Snippets -- that's just refactoring in the code that fetches content suggestions and displays them on the NTP.

treib@ is the right person to be CC'd here AFAIK :).

Comment 5 by h...@chromium.org, Feb 1 2018

Looking at the commits going into
https://ci.chromium.org/buildbot/chromium.clang/ToTWin/830

There's one that mentions NTP. No idea if this is related.. I'm still trying to get the test to run at all on my machine.

---
Replace the NTP Interceptor with Throttle

This change replaces the NewTabPageInterceptorService with a
NewTabPageNavigationThrottle.  The throttle reduces complexity, but it
is also neutral regarding the network stack / network service, so it
fixes the NewTabPageInterceptorServiceTests that were broken with the
NetworkService feature enabled.  These tests are now renamed
NewTabPageNavigationThrottleTest.

BUG= 802926 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Iec0664df33e79d6bcc18f60aecc4017de7f65858
Reviewed-on: https://chromium-review.googlesource.com/868994
Commit-Queue: Conley Owens <cco3@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530370}
---

Comment 6 by treib@chromium.org, Feb 1 2018

blundell's CL only affects mobile, so that's not the culprit.

The Throttle does look related. What it does is redirect to the local NTP ("chrome-search://local-ntp/local-ntp.html", i.e. the one that shows up in the error messages) on various errors. Maybe the new Throttle is more eager than the old Interceptor was? No idea why that would only happen in official builds though.
Can we hand this over to you and cco3 then? :-)

Comment 8 by treib@chromium.org, Feb 1 2018

Owner: cco3@chromium.org
I'm officially not working on the NTP anymore :D

Comment 9 by h...@chromium.org, Feb 1 2018

Verified locally that reverting the NTP Throttle patch fixes at least the StartupBrowserCreatorTest.ProfilesWithoutPagesNotLaunched test.

Over to cco3.

Since the patch still reverts cleanly, I'd suggest that you revert it before starting to fix.

Comment 10 by h...@chromium.org, Feb 1 2018

Actually, I'll go ahead and revert, and maybe the builds will green up by the time MTV get into the office.

Comment 12 by h...@chromium.org, Feb 1 2018

Blocking: -803661
Components: -Build UI>Browser>NewTabPage
Labels: NetworkService
Labels: -NetworkService Proj-Servicification
Project Member

Comment 15 by bugdroid1@chromium.org, Feb 1 2018

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

commit 0f77672dd3f34d5ba2e96912d34d657deb8f7433
Author: Hans Wennborg <hans@chromium.org>
Date: Thu Feb 01 15:46:13 2018

Revert "Replace the NTP Interceptor with Throttle"

This reverts commit 2ba21bcd266043ec39744a312c2921496e32000e.

Reason for revert: This broke browser_tests in official chrome-branded builds. See for example:
https://uberchromegw.corp.google.com/i/official.desktop.continuous/builders/win%20trunk/builds/78737

Also see the bug.

Original change's description:
> Replace the NTP Interceptor with Throttle
> 
> This change replaces the NewTabPageInterceptorService with a
> NewTabPageNavigationThrottle.  The throttle reduces complexity, but it
> is also neutral regarding the network stack / network service, so it
> fixes the NewTabPageInterceptorServiceTests that were broken with the
> NetworkService feature enabled.  These tests are now renamed
> NewTabPageNavigationThrottleTest.
> 
> BUG= 802926 
> 
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
> Change-Id: Iec0664df33e79d6bcc18f60aecc4017de7f65858
> Reviewed-on: https://chromium-review.googlesource.com/868994
> Commit-Queue: Conley Owens <cco3@chromium.org>
> Reviewed-by: Marc Treib <treib@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Reviewed-by: Reilly Grant <reillyg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#530370}

TBR=cco3@chromium.org,reillyg@chromium.org,mmenke@chromium.org,treib@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  802926 ,  808011 
Change-Id: I6bedcaea58a7009cd0302469390645f2e496203e
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/897423
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Hans Wennborg <hans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533675}
[modify] https://crrev.com/0f77672dd3f34d5ba2e96912d34d657deb8f7433/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/0f77672dd3f34d5ba2e96912d34d657deb8f7433/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/0f77672dd3f34d5ba2e96912d34d657deb8f7433/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/0f77672dd3f34d5ba2e96912d34d657deb8f7433/chrome/browser/ui/search/new_tab_page_interceptor_browsertest.cc
[add] https://crrev.com/0f77672dd3f34d5ba2e96912d34d657deb8f7433/chrome/browser/ui/search/new_tab_page_interceptor_service.cc
[add] https://crrev.com/0f77672dd3f34d5ba2e96912d34d657deb8f7433/chrome/browser/ui/search/new_tab_page_interceptor_service.h
[add] https://crrev.com/0f77672dd3f34d5ba2e96912d34d657deb8f7433/chrome/browser/ui/search/new_tab_page_interceptor_service_factory.cc
[add] https://crrev.com/0f77672dd3f34d5ba2e96912d34d657deb8f7433/chrome/browser/ui/search/new_tab_page_interceptor_service_factory.h
[delete] https://crrev.com/b696eeaf6b786760b9a7e30b0957cd5d38fcdc34/chrome/browser/ui/search/new_tab_page_navigation_throttle.cc
[delete] https://crrev.com/b696eeaf6b786760b9a7e30b0957cd5d38fcdc34/chrome/browser/ui/search/new_tab_page_navigation_throttle.h
[delete] https://crrev.com/b696eeaf6b786760b9a7e30b0957cd5d38fcdc34/chrome/browser/ui/search/new_tab_page_navigation_throttle_browsertest.cc
[modify] https://crrev.com/0f77672dd3f34d5ba2e96912d34d657deb8f7433/chrome/test/BUILD.gn
[modify] https://crrev.com/0f77672dd3f34d5ba2e96912d34d657deb8f7433/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Comment 16 by cco3@chromium.org, Feb 1 2018

Working on a fix

Comment 17 by cco3@chromium.org, Feb 1 2018

Status: Started (was: Assigned)

Comment 18 by cco3@chromium.org, Feb 1 2018

hans@, You said that you verified reverting locally fixes it.  Does that mean you were able to reproduce locally?  The tests I'm looking at, it seems that it must have to do with the configuration on the trybots.
It repros in official builds. Set

is_official_build=true
is_chrome_branded=true

in your args.gn and add src-internal to your .gclient. Sync, rebuild, you should be able to repro.

Comment 20 by h...@chromium.org, Feb 2 2018

> hans@, You said that you verified reverting locally fixes it.  Does that mean you were able to reproduce locally?

Yes, it reproduced for me locally. You can copy the build config from e.g. https://uberchromegw.corp.google.com/i/official.desktop.continuous/builders/win%20trunk/builds/78737

Comment 21 by zea@chromium.org, Feb 6 2018

Labels: zine-triaged
Project Member

Comment 22 by bugdroid1@chromium.org, Feb 21 2018

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

commit 9019d6765f35c87f22e1144421f3aceaf5962417
Author: Conley Owens <cco3@chromium.org>
Date: Wed Feb 21 20:32:32 2018

Replace the NTP Interceptor with Throttle

This change replaces the NewTabPageInterceptorService with a
NewTabPageNavigationThrottle.  The throttle reduces complexity, but it
is also neutral regarding the network stack / network service, so it
fixes the NewTabPageInterceptorServiceTests that were broken with the
NetworkService feature enabled.  These tests are now renamed
NewTabPageNavigationThrottleTest.

This reverts commit 0f77672dd3f34d5ba2e96912d34d657deb8f7433.

Issues with the previous version of the commit have been fixed by modifying
the affected tests to handle redirection appropriately when
features::kUseGoogleLocalNtp is not enabled.

BUG= 802926 ,804055, 808011 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Iec3a11c6454633860728f76564e10406ec0a9cbd
Reviewed-on: https://chromium-review.googlesource.com/920461
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Conley Owens <cco3@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538197}
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/extensions/all_urls_apitest.cc
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/extensions/content_script_apitest.cc
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/extensions/extension_url_rewrite_browsertest.cc
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/policy/policy_browsertest.cc
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/ui/browser_browsertest.cc
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/ui/browser_navigator_browsertest.cc
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/ui/input_method/input_method_engine.cc
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/ui/search/local_ntp_test_utils.cc
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/ui/search/local_ntp_test_utils.h
[delete] https://crrev.com/6e6e5b30bf3009a4b5ef8bbd25ef1f3da362b3cc/chrome/browser/ui/search/new_tab_page_interceptor_browsertest.cc
[delete] https://crrev.com/6e6e5b30bf3009a4b5ef8bbd25ef1f3da362b3cc/chrome/browser/ui/search/new_tab_page_interceptor_service.cc
[delete] https://crrev.com/6e6e5b30bf3009a4b5ef8bbd25ef1f3da362b3cc/chrome/browser/ui/search/new_tab_page_interceptor_service.h
[delete] https://crrev.com/6e6e5b30bf3009a4b5ef8bbd25ef1f3da362b3cc/chrome/browser/ui/search/new_tab_page_interceptor_service_factory.cc
[delete] https://crrev.com/6e6e5b30bf3009a4b5ef8bbd25ef1f3da362b3cc/chrome/browser/ui/search/new_tab_page_interceptor_service_factory.h
[add] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/ui/search/new_tab_page_navigation_throttle.cc
[add] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/ui/search/new_tab_page_navigation_throttle.h
[add] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/ui/search/new_tab_page_navigation_throttle_browsertest.cc
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/test/BUILD.gn
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/test/data/extensions/api_test/content_scripts/ntp/background.js
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/test/data/extensions/api_test/tabs/basics/crud2.js
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/test/data/extensions/api_test/tabs/basics/move.js
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/chrome/test/data/extensions/api_test/tabs/on_updated/test.js
[modify] https://crrev.com/9019d6765f35c87f22e1144421f3aceaf5962417/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Project Member

Comment 23 by bugdroid1@chromium.org, Feb 22 2018

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

commit 01bea8cadcbae1df52c4b3aa71310a2311328c2d
Author: Nico Weber <thakis@chromium.org>
Date: Thu Feb 22 18:15:24 2018

Revert "Replace the NTP Interceptor with Throttle"

This reverts commit 9019d6765f35c87f22e1144421f3aceaf5962417.

Reason for revert: The test still seems to fail on various bots, see e.g. crbug.com/814545

Original change's description:
> Replace the NTP Interceptor with Throttle
> 
> This change replaces the NewTabPageInterceptorService with a
> NewTabPageNavigationThrottle.  The throttle reduces complexity, but it
> is also neutral regarding the network stack / network service, so it
> fixes the NewTabPageInterceptorServiceTests that were broken with the
> NetworkService feature enabled.  These tests are now renamed
> NewTabPageNavigationThrottleTest.
> 
> This reverts commit 0f77672dd3f34d5ba2e96912d34d657deb8f7433.
> 
> Issues with the previous version of the commit have been fixed by modifying
> the affected tests to handle redirection appropriately when
> features::kUseGoogleLocalNtp is not enabled.
> 
> BUG= 802926 ,804055, 808011 
> 
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
> Change-Id: Iec3a11c6454633860728f76564e10406ec0a9cbd
> Reviewed-on: https://chromium-review.googlesource.com/920461
> Reviewed-by: Marc Treib <treib@chromium.org>
> Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
> Reviewed-by: Tommy Martino <tmartino@chromium.org>
> Reviewed-by: Trent Apted <tapted@chromium.org>
> Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
> Reviewed-by: Reilly Grant <reillyg@chromium.org>
> Commit-Queue: Conley Owens <cco3@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#538197}

TBR=bartfab@chromium.org,bauerb@chromium.org,tapted@chromium.org,shuchen@chromium.org,cco3@chromium.org,reillyg@chromium.org,treib@chromium.org,emaxx@chromium.org,tmartino@chromium.org

Change-Id: I65df1f24e4cdcc5d3ee8845fa64b669153819ec3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  802926 , 804055,  808011 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/932081
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Conley Owens <cco3@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538485}
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/extensions/all_urls_apitest.cc
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/extensions/content_script_apitest.cc
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/extensions/extension_url_rewrite_browsertest.cc
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/policy/policy_browsertest.cc
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/ui/browser_browsertest.cc
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/ui/browser_navigator_browsertest.cc
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/ui/input_method/input_method_engine.cc
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/ui/search/local_ntp_test_utils.cc
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/ui/search/local_ntp_test_utils.h
[add] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/ui/search/new_tab_page_interceptor_browsertest.cc
[add] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/ui/search/new_tab_page_interceptor_service.cc
[add] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/ui/search/new_tab_page_interceptor_service.h
[add] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/ui/search/new_tab_page_interceptor_service_factory.cc
[add] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/ui/search/new_tab_page_interceptor_service_factory.h
[delete] https://crrev.com/d471df2beb31bb40c50a530f36c701f7d85ae36c/chrome/browser/ui/search/new_tab_page_navigation_throttle.cc
[delete] https://crrev.com/d471df2beb31bb40c50a530f36c701f7d85ae36c/chrome/browser/ui/search/new_tab_page_navigation_throttle.h
[delete] https://crrev.com/d471df2beb31bb40c50a530f36c701f7d85ae36c/chrome/browser/ui/search/new_tab_page_navigation_throttle_browsertest.cc
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/test/BUILD.gn
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/test/data/extensions/api_test/content_scripts/ntp/background.js
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/test/data/extensions/api_test/tabs/basics/crud2.js
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/test/data/extensions/api_test/tabs/basics/move.js
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/chrome/test/data/extensions/api_test/tabs/on_updated/test.js
[modify] https://crrev.com/01bea8cadcbae1df52c4b3aa71310a2311328c2d/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Project Member

Comment 24 by bugdroid1@chromium.org, Feb 26 2018

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

commit aafcf1279bd15fa897864f29a02153606bb6b755
Author: Conley Owens <cco3@chromium.org>
Date: Mon Feb 26 18:53:51 2018

Replace the NTP Interceptor with Throttle

This change replaces the NewTabPageInterceptorService with a
NewTabPageNavigationThrottle.  The throttle reduces complexity, but it
is also neutral regarding the network stack / network service, so it
fixes the NewTabPageInterceptorServiceTests that were broken with the
NetworkService feature enabled.  These tests are now renamed
NewTabPageNavigationThrottleTest.

We modify affected tests to handle the redirection appropriately, using
either local_ntp_test_utils::GetFinalNtpUrl or search::IsInstantNTP.

This reverts commit 01bea8cadcbae1df52c4b3aa71310a2311328c2d.

The previous commit triggered a new test failure in
ExtensionURLRewriteBrowserTest.NewTabPageURLOverride, in which case we
were wrongly anticipating a redirect to the local NTP.  This commit does
not alter the shared TestURLNotShown method, but only fixes the test
directly affected by our change.

BUG= 802926 ,804055, 808011 ,814545

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ia7735eee7beed3a49ecfcf9eeb8798539693c97a
Reviewed-on: https://chromium-review.googlesource.com/931988
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Conley Owens <cco3@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539214}
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/extensions/all_urls_apitest.cc
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/extensions/content_script_apitest.cc
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/extensions/extension_url_rewrite_browsertest.cc
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/policy/policy_browsertest.cc
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/ui/browser_browsertest.cc
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/ui/browser_navigator_browsertest.cc
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/ui/input_method/input_method_engine.cc
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/ui/search/local_ntp_test_utils.cc
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/ui/search/local_ntp_test_utils.h
[delete] https://crrev.com/6e67d9e429f3fc0c545d0b908929fcb55342d5bf/chrome/browser/ui/search/new_tab_page_interceptor_browsertest.cc
[delete] https://crrev.com/6e67d9e429f3fc0c545d0b908929fcb55342d5bf/chrome/browser/ui/search/new_tab_page_interceptor_service.cc
[delete] https://crrev.com/6e67d9e429f3fc0c545d0b908929fcb55342d5bf/chrome/browser/ui/search/new_tab_page_interceptor_service.h
[delete] https://crrev.com/6e67d9e429f3fc0c545d0b908929fcb55342d5bf/chrome/browser/ui/search/new_tab_page_interceptor_service_factory.cc
[delete] https://crrev.com/6e67d9e429f3fc0c545d0b908929fcb55342d5bf/chrome/browser/ui/search/new_tab_page_interceptor_service_factory.h
[add] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/ui/search/new_tab_page_navigation_throttle.cc
[add] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/ui/search/new_tab_page_navigation_throttle.h
[add] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/ui/search/new_tab_page_navigation_throttle_browsertest.cc
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/test/BUILD.gn
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/test/data/extensions/api_test/content_scripts/ntp/background.js
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/test/data/extensions/api_test/tabs/basics/crud2.js
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/test/data/extensions/api_test/tabs/basics/move.js
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/chrome/test/data/extensions/api_test/tabs/on_updated/test.js
[modify] https://crrev.com/aafcf1279bd15fa897864f29a02153606bb6b755/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Comment 25 by cco3@chromium.org, Mar 5 2018

Status: Fixed (was: Started)

Sign in to add a comment