The page in the screenshot is actually just a vanilla google.com homepage.
Here's what probably happened: The actual NTP (from https://www.google.com/_/chrome/newtab) first checks if the embeddedSearch API (https://www.chromium.org/embeddedsearch) is available. If it isn't, it forwards to a Google homepage instead. So the question is why that API isn't there. We make the API available only to "Instant" processes, so my guess is that isolating google.com messes with that.

Note that this is going to become obsolete soon(ish), since we're planning to get rid of the remote NTP. It'll be a few versions until we can actually remove the code though.

Marking this as available until the issue is either fixed or obsolete.

Thanks!  treib@, is there a bug we can follow for when the remote NTP isn't used anymore?  We're just curious about the timeframe, since it affects some of our plans for field trials.

The tracking bug is bug 583289. We're hoping to have the local NTP feature-complete for M63, though it'll be a while longer until all the remote NTP code can actually be removed.

treib@: Just wanted to check in on the timeline for issue 583289 and turning off the remote NTP.  I'm curious if we should look into a way to get the Instant API working in the meantime.  Thanks!

We're aiming for M65 with the local NTP launch, though the remote NTP will probably stay around for at least another version or two until we can fully shut it off.

Just to make sure: Is this specific to Google, or might we apply similar isolation to any default search engine? Because even after the local NTP launch, third-party remote NTPs will still exist and will presumably have the same problem.

Ah, we should fix this if Remote NTPs are sticking around.  We want users to be able to specific --isolate-origins= for any site, including their default search engine.

Alex, what are your thoughts on how we can get the isolated origin logic not to prevent the "Instant" API (comment 1) from working?  I imagine it's breaking for similar reasons that isolated origins take precedence over hosted apps.

I think this is because we completely ignore effective URL resolution for isolated origins.

Normally, navigating to https://www.google.com/_/chrome/newtab would resolve to an effective URL of chrome-search://remote-ntp/_/chrome/newtab?ie=UTF-8, which would get us a SiteInstance with the site URL  of "chrome-search://remote-ntp".  Later, in ChromeContentBrowserClient::SiteInstanceGotProcess(), we'd check that site URL with ShouldAssignURLToInstantRenderer() and then call AddInstantProcess() on the corresponding process, which according to #1 is what gives the NTP the embeddedSearch API.

With --isolate-origins=https://google.com, the effective URL for https://www.google.com/_/chrome/newtab is the same URL, and the SiteInstance's site URL is "https://google.com/".  When SiteInstanceGotProcess() calls ShouldAssignURLToInstantRenderer() on "https://google.com/", that returns false, and we don't call AddInstantProcess().

We originally skipped isolated origins in SiteInstanceImpl::GetEffectiveURL, because isolated origins needed to take precedence over hosted apps.  I think for this, we should let the NTP effective URLs still take precedence over any matching isolated origins.  So, we could add a ContentBrowserClient method that we'd consult from SiteInstanceImpl::GetEffectiveURL, on whether the particular effective URL should be used regardless of a matching isolated origin, or alternatively, pass a bool is_isolated_origin into ChromeContentBrowserClient::GetEffectiveURL, which would allow making this decision there.

I've just verified this fixes the NTP for me locally.  I should have a CL for this as soon as I write a test.

treib@: any hints for writing a browsertest that exercises the remote NTP?  I see local_ntp_browsertest.cc, but not sure whether there's an equivalent for the remote NTP.  Ideally, I'd pass a command-line flag at setup time to activate the same isolated origin used by the NTP URL (e.g., google.com), and then load the NTP and verify that IsInstantProcess() is true.  In my initial attempt at a browsertest, when I try to load search::GetNewTabPageURL(), I always end up loading the local NTP, so I'm probably not setting something up correctly.

I think what happens is: You try to load google.com/... which fails because you're in a test, and so NewTabPageInterceptor forwards you to the local NTP instead.

The way to test remote NTPs goes like this:
- Start an EmbeddedTestServer.
- Setup a custom search engine with an ntp_url pointing to the test server.
- Open chrome::kChromeUINewTabURL (which will get translated to the real URL).

You can look at InstantThemeTest for an example.

#10: thanks for the guidance!  Based on that, I was able to construct a test that works.  WIP CL at https://crrev.com/c/783655.

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

commit bb99a337d09d8c23eef26d18195af265770db6c8
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Wed Nov 22 04:49:57 2017

Prevent isolated origins from interfering with NTP.

Previously, when an isolated origin matched the remote NTP URL, it
would not allow the SiteInstance for the NTP to be resolved to the
effective chrome-search:// site URL.  This resulting in NTP not
loading correctly, as the resulting process was not marked as an
Instant process in SiteInstanceGotProcess().

The effective URL logic was completely bypassed by isolated origins so
that they could take precedence over hosted apps.  This CL adjusts the
effective URL calculation so that it's only bypassed for extensions
cases, but not for the NTP.

Bug:  755595 
Change-Id: I4a2909e7d3267e2f94d114daa13c429f81c84150
Reviewed-on: https://chromium-review.googlesource.com/783655
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518533}
[modify] https://crrev.com/bb99a337d09d8c23eef26d18195af265770db6c8/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/bb99a337d09d8c23eef26d18195af265770db6c8/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/bb99a337d09d8c23eef26d18195af265770db6c8/chrome/browser/chrome_content_browser_client_browsertest.cc
[modify] https://crrev.com/bb99a337d09d8c23eef26d18195af265770db6c8/content/browser/renderer_host/render_process_host_unittest.cc
[modify] https://crrev.com/bb99a337d09d8c23eef26d18195af265770db6c8/content/browser/site_instance_impl.cc
[modify] https://crrev.com/bb99a337d09d8c23eef26d18195af265770db6c8/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/bb99a337d09d8c23eef26d18195af265770db6c8/content/public/browser/content_browser_client.h

Alex: Can you check whether this is fixed?  r518533 landed in 64.0.3276.0, but in 64.0.3278.0 (Windows Canary) I'm seeing mixed results.  The bookmark bar is visible above the remote NTP when running with --isolate-origins=https://google.com, but the most visited tiles briefly appear and then disappear.  Do you see that as well?

Charlie: I wasn't seeing that on my local Linux build; the NTP seems to be working properly there.  I can verify that I also saw the disappearing tiles on the latest Mac Canary the *first* time I opened a new tab after updating and restarting.  But all the subsequent new tabs seem to have the NTP working properly (including after restarting the browser, and also starting in a new profile).  Is that also what you're seeing?

Hmm, the issue isn't going away for me, at least on Windows in my existing profile.  You're right that a clean profile doesn't show the issue, on either Windows or Mac.

I've confirmed in both cases that I'm using the remote NTP, by disabling #use-google-local-ntp on about:flags and checking that document.origin on the NTP is https://www.google.com.

I wonder what's different in my existing profile...  (Maybe we can mark this fixed and follow up if it's an issue for others?)

I've asked a couple more people to try this, and avallee@ and lukasza@ both confirmed that this worked for them on the latest Mac canary.  I know Lukasz tried this on an old profile which had never been run with --isolate-origins=https://google.com (same is probably true for Alex as well?), so one theory is that what we saw in #14-16 might've been an issue only for profiles which had used the flag and the NTP prior to my fix (so this shouldn't affect anyone else).

Because it's working for others I'll go ahead and assume this is fixed, and let's file a followup bug if this issue reappears for anyone else. 

Requesting merge to M63, as this is a blocker for  issue 760761 , which needs to go into M63.  See also  https://crbug.com/788837#c4  for the motivation -- the intention is to merge both of these (complementary) fixes.

This bug requires manual review: We are only 5 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Approving merge to M63 branch 3239 per offline chat with  alexmos@ & creis@. Please see https://bugs.chromium.org/p/chromium/issues/detail?id=788837#c8 for more details.

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

commit c392366fded33de326a4ec40c2b7428c4864ee36
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Wed Nov 29 23:08:24 2017

Prevent isolated origins from interfering with NTP (Merge to M63)

Previously, when an isolated origin matched the remote NTP URL, it
would not allow the SiteInstance for the NTP to be resolved to the
effective chrome-search:// site URL.  This resulting in NTP not
loading correctly, as the resulting process was not marked as an
Instant process in SiteInstanceGotProcess().

The effective URL logic was completely bypassed by isolated origins so
that they could take precedence over hosted apps.  This CL adjusts the
effective URL calculation so that it's only bypassed for extensions
cases, but not for the NTP.

TBR=alexmos@chromium.org

(cherry picked from commit bb99a337d09d8c23eef26d18195af265770db6c8)

Bug:  755595 
Change-Id: I4a2909e7d3267e2f94d114daa13c429f81c84150
Reviewed-on: https://chromium-review.googlesource.com/783655
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#518533}
Reviewed-on: https://chromium-review.googlesource.com/798213
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#611}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/c392366fded33de326a4ec40c2b7428c4864ee36/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/c392366fded33de326a4ec40c2b7428c4864ee36/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/c392366fded33de326a4ec40c2b7428c4864ee36/chrome/browser/chrome_content_browser_client_browsertest.cc
[modify] https://crrev.com/c392366fded33de326a4ec40c2b7428c4864ee36/content/browser/renderer_host/render_process_host_unittest.cc
[modify] https://crrev.com/c392366fded33de326a4ec40c2b7428c4864ee36/content/browser/site_instance_impl.cc
[modify] https://crrev.com/c392366fded33de326a4ec40c2b7428c4864ee36/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/c392366fded33de326a4ec40c2b7428c4864ee36/content/public/browser/content_browser_client.h

Rechecked this issue on Windows 10, Mac 10.12.6, Ubuntu 14.04 using chrome version 63.0.3239.82. Merge is working as intended. Screen shot attached. Tagging issue with TE-Verified labels for M63.

Thanks.!

Deleted: NTP.png 1.2 MB

	NTP.png 1.2 MB View Download