Exempt the WebUI version of NTP from the instant process.

The WebUI NTP must be committed and rendered in a WebUI process.
Currently, that is not the case because it is considered as part
of the instant process (used for the current version of local NTP).
This CL changes the implementation to no longer put the WebUI
NTP page as part of the instant process, which ensures it commits
in its own WebUI process.

Bug: 1073956
Change-Id: Ie3011374981584fa68d04bd42a29eed3d20781f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2163433
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762489}

chrome/browser/search/ntp_navigation_browsertest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/strings/strcat.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/common/url_constants.h"
+#include "chrome/common/webui_url_constants.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/web_contents.h"
+#include "net/dns/mock_host_resolver.h"
+
+#include "chrome/browser/search/ntp_features.h"
+
+class NtpNavigationBrowserTest : public InProcessBrowserTest {
+ public:
+  NtpNavigationBrowserTest() = default;
+  ~NtpNavigationBrowserTest() override = default;
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    embedded_test_server()->ServeFilesFromSourceDirectory("content/test/data");
+    ASSERT_TRUE(embedded_test_server()->InitializeAndListen());
+  }
+  void SetUpOnMainThread() override {
+    host_resolver()->AddRule("*", "127.0.0.1");
+    embedded_test_server()->StartAcceptingConnections();
+  }
+};
+
+// Verify that the local NTP commits in a SiteInstance with the local NTP URL.
+IN_PROC_BROWSER_TEST_F(NtpNavigationBrowserTest, VerifyNtpSiteInstance) {
+  GURL ntp_url(chrome::kChromeUINewTabURL);
+  ui_test_utils::NavigateToURL(browser(), ntp_url);
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_EQ(ntp_url, web_contents->GetLastCommittedURL());
+
+  GURL local_ntp_url(base::StrCat({chrome::kChromeSearchScheme, "://",
+                                   chrome::kChromeSearchLocalNtpHost, "/"}));
+  ASSERT_EQ(local_ntp_url,
+            web_contents->GetMainFrame()->GetSiteInstance()->GetSiteURL());
+}
+
+// Subclass to allow running tests against the WebUI NTP implementation.
+class WebUiNtpNavigationBrowserTest : public NtpNavigationBrowserTest {
+ public:
+  WebUiNtpNavigationBrowserTest() {
+    feature_list_.InitAndEnableFeature(ntp_features::kWebUI);
+  }
+  ~WebUiNtpNavigationBrowserTest() override = default;
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+// Verify that the WebUI NTP commits in a SiteInstance with the WebUI URL.
+IN_PROC_BROWSER_TEST_F(WebUiNtpNavigationBrowserTest,
+                       VerifyWebUiNtpSiteInstance) {
+  GURL ntp_url(chrome::kChromeUINewTabURL);
+  ui_test_utils::NavigateToURL(browser(), ntp_url);
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_EQ(ntp_url, web_contents->GetLastCommittedURL());
+
+  GURL webui_ntp_url(chrome::kChromeUINewTabPageURL);
+  ASSERT_EQ(webui_ntp_url,
+            web_contents->GetMainFrame()->GetSiteInstance()->GetSiteURL());
+}

chrome/browser/search/search.cc

@@ -314,9 +314,19 @@ GURL GetNewTabPageURL(Profile* profile) {
 #if !defined(OS_ANDROID)
 
 bool ShouldAssignURLToInstantRenderer(const GURL& url, Profile* profile) {
-  return url.is_valid() && profile && IsInstantExtendedAPIEnabled() &&
-         (url.SchemeIs(chrome::kChromeSearchScheme) ||
-          IsNTPOrRelatedURLHelper(url, profile));
+  if (!url.is_valid() || !profile || !IsInstantExtendedAPIEnabled())
+    return false;
+
+  bool is_ntp_related_url = IsNTPOrRelatedURLHelper(url, profile);
+
+  // When the WebUI NTP feature is enabled, it should be running in a WebUI
+  // process instead of the instant process.
+  if (base::FeatureList::IsEnabled(ntp_features::kWebUI) &&
+      is_ntp_related_url && url.SchemeIs(content::kChromeUIScheme)) {
+    return false;
+  }
+
+  return is_ntp_related_url || url.SchemeIs(chrome::kChromeSearchScheme);
 }
 
 bool ShouldUseProcessPerSiteForInstantURL(const GURL& url, Profile* profile) {

chrome/browser/ui/search/ntp_user_data_logger.cc

@@ -372,7 +372,9 @@ NTPUserDataLogger::~NTPUserDataLogger() {}
 // static
 NTPUserDataLogger* NTPUserDataLogger::GetOrCreateFromWebContents(
     content::WebContents* content) {
-  DCHECK(search::IsInstantNTP(content));
+  DCHECK(search::IsInstantNTP(content) ||
+         content->GetMainFrame()->GetSiteInstance()->GetSiteURL() ==
+             GURL(chrome::kChromeUINewTabPageURL));
 
   // Calling CreateForWebContents when an instance is already attached has no
   // effect, so we can do this.

chrome/test/BUILD.gn

@@ -1142,6 +1142,7 @@ if (!is_android) {
       "../browser/safe_browsing/test_safe_browsing_database_helper.h",
       "../browser/safe_xml_parser_browsertest.cc",
       "../browser/search/ntp_custom_background_enabled_policy_handler_browsertest.cc",
+      "../browser/search/ntp_navigation_browsertest.cc",
       "../browser/search_engines/template_url_scraper_browsertest.cc",
       "../browser/secure_origin_whitelist_browsertest.cc",
       "../browser/serial/chrome_serial_browsertest.cc",