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

Issue 810070 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

gn arg is_chrome_branded changes the default New Tab Page URL

Project Member Reported by cco3@chromium.org, Feb 7 2018

Issue description

is_chrome_branded = false
search::GetNewTabPageURL(profile) returns "chrome-search://local-ntp/local-ntp.html"

is_chrome_branded = true
search::GetNewTabPageURL(profile) returns "https://www.google.com/_/chrome/newtab?ie=UTF-8"

This isn't expected.  In both circumstances, the NTP should be https://www.google.com/_/chrome/newtab
 

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

This behavior was seen in the PolicyTest.HomepageLocation browsertest

Comment 2 by cco3@chromium.org, Feb 7 2018

base::FeatureList::IsEnabled(features::kUseGoogleLocalNtp) is set when is_chrome_branded is false, but not when it is true.

Comment 3 by cco3@chromium.org, Feb 7 2018

Cc: cco3@chromium.org

Comment 4 by cco3@chromium.org, Feb 7 2018

The NTPUseGoogleLocalNtp field trial is getting triggered when is_chrome_branded = false, but not when is_chrome_branded = true.
https://cs.chromium.org/chromium/src/testing/variations/fieldtrial_testing_config.json?l=2169

I don't understand how the testing field trials work...so not exactly sure what's going on here.

Comment 5 by cco3@chromium.org, Feb 7 2018

Ah, from the README:
"""
Note: This configuration applies specifically to Chromium developer builds. Chrome branded / official builds do not use these definitions.
"""

Any suggestions on what to do from here Marc?  Should we get rid of the NTPUseGoogleLocalNtp or turn it off by default?

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

Ah, nice finding! Right, the field trial testing config will only affect non-Chrome-branded builds. So this much is working as expected.

I don't think we should turn off the feature because of this - all tests should work with it on and off, and so far they did. I still don't quite understand what exactly the NavigationThrottle CL changed that made the tests fail.

Comment 7 by cco3@chromium.org, Feb 9 2018

Status: WontFix (was: Untriaged)

Sign in to add a comment