WebUI NTP: link navigations from NTP should change order of most-visited

The most-visited are populated by history segments which are created for
URLs typed in the omnibox (ui::PAGE_TRANSITION_TYPED) and URLs that are
loaded from top Chrome bookmarks (ui::PAGE_TRANSITION_AUTO_BOOKMARK).

https://source.chromium.org/chromium/chromium/src/+/master:components/history/core/browser/history_backend.cc;l=423;drc=b5056af54caa3a56c0d748bd39d0803ae0312a9b

The segments keep a |visit_count|. The segment entries are used to query
the top sites from the last 90 days.

https://source.chromium.org/chromium/chromium/src/+/master:components/history/core/browser/top_sites_impl.cc;l=241;drc=df87046cb8ae4dbd62cda6e56d317016a6fa02c7
https://source.chromium.org/chromium/chromium/src/+/master:components/history/core/browser/history_backend.cc;l=464;drc=b5056af54caa3a56c0d748bd39d0803ae0312a9b
https://source.chromium.org/chromium/chromium/src/+/master:components/history/core/browser/visitsegment_database.cc;l=168;drc=df87046cb8ae4dbd62cda6e56d317016a6fa02c7

The visits to the most-visited on the Android NTP use
ui::PAGE_TRANSITION_AUTO_BOOKMARK which means if a user taps on a
most-visited tile, the segment's |visit_count| will be incremented.

crbug.com/620296 was created so the remote and local NTP would work the
same as the Android NTP (most-visited tile clicks will increment the
associated segment's |visit_count|. The way this was accomplished was by
altering the link navigations when the current site URL has the origins
chrome-search://local-ntp or chrome-search://remote-ntp. The transition
was changed from ui::PAGE_TRANSITION_LINK to
ui::PAGE_TRANSITION_AUTO_BOOKMARK so the navigation would be considered
as starting a segment.

https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/chrome_content_browser_client.cc;l=1828;drc=c3de1340119b5e6e76cc34248f3310ea05dfaf6f

chrome://new-tab-page is replacing
chrome-search://local-ntp/local-ntp.html. If adding the transition
override for link navigations from the local NTP is still justified, this
CL does the same for chrome://new-tab-page.

Bug: 1089877
Change-Id: Ib6bea5d04ff95a4c20aaf5d8e90dd857719d83a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2247101Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Commit-Queue: Esmael Elmoslimany <aee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#783029}

chrome/browser/chrome_content_browser_client_unittest.cc

@@ -59,6 +59,7 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "chrome/test/base/search_test_utils.h"
+#include "ui/base/page_transition_types.h"
 #else
 #include "base/system/sys_info.h"
 #endif
@@ -291,6 +292,84 @@ TEST_F(ChromeContentBrowserClientWindowTest, OpenURL) {
   EXPECT_EQ(previous_count + 2, browser()->tab_strip_model()->count());
 }
 
+// TODO(crbug.com/566091): Remove the need for ShouldStayInParentProcessForNTP()
+//    and associated test.
+TEST_F(ChromeContentBrowserClientWindowTest, ShouldStayInParentProcessForNTP) {
+  ChromeContentBrowserClient client;
+  scoped_refptr<content::SiteInstance> site_instance =
+      content::SiteInstance::CreateForURL(
+          browser()->profile(),
+          GURL("chrome-search://local-ntp/local-ntp.html"));
+  EXPECT_TRUE(client.ShouldStayInParentProcessForNTP(
+      GURL("chrome-search://local-ntp/local-ntp.html"), site_instance.get()));
+
+  site_instance = content::SiteInstance::CreateForURL(
+      browser()->profile(), GURL("chrome://new-tab-page"));
+  // chrome://new-tab-page is an NTP replacing local-ntp and supports OOPIFs.
+  // ShouldStayInParentProcessForNTP() should only return true for NTPs hosted
+  // under the chrome-search: scheme.
+  EXPECT_FALSE(client.ShouldStayInParentProcessForNTP(
+      GURL("chrome://new-tab-page"), site_instance.get()));
+}
+
+TEST_F(ChromeContentBrowserClientWindowTest, OverrideNavigationParams) {
+  ChromeContentBrowserClient client;
+  ui::PageTransition transition;
+  bool is_renderer_initiated;
+  content::Referrer referrer = content::Referrer();
+  base::Optional<url::Origin> initiator_origin = base::nullopt;
+
+  scoped_refptr<content::SiteInstance> site_instance =
+      content::SiteInstance::CreateForURL(
+          browser()->profile(),
+          GURL("chrome-search://local-ntp/local-ntp.html"));
+  transition = ui::PAGE_TRANSITION_LINK;
+  is_renderer_initiated = true;
+  // The origin is a placeholder to test that |initiator_origin| is set to
+  // base::nullopt and is not meant to represent what would happen in practice.
+  initiator_origin = url::Origin::Create(GURL("https://www.example.com"));
+  client.OverrideNavigationParams(site_instance.get(), &transition,
+                                  &is_renderer_initiated, &referrer,
+                                  &initiator_origin);
+  EXPECT_TRUE(ui::PageTransitionCoreTypeIs(ui::PAGE_TRANSITION_AUTO_BOOKMARK,
+                                           transition));
+  EXPECT_FALSE(is_renderer_initiated);
+  EXPECT_EQ(base::nullopt, initiator_origin);
+
+  site_instance = content::SiteInstance::CreateForURL(
+      browser()->profile(), GURL("chrome://new-tab-page"));
+  transition = ui::PAGE_TRANSITION_LINK;
+  is_renderer_initiated = true;
+  initiator_origin = url::Origin::Create(GURL("https://www.example.com"));
+  client.OverrideNavigationParams(site_instance.get(), &transition,
+                                  &is_renderer_initiated, &referrer,
+                                  &initiator_origin);
+  EXPECT_TRUE(ui::PageTransitionCoreTypeIs(ui::PAGE_TRANSITION_AUTO_BOOKMARK,
+                                           transition));
+  EXPECT_FALSE(is_renderer_initiated);
+  EXPECT_EQ(base::nullopt, initiator_origin);
+
+  // No change for transitions that are not PAGE_TRANSITION_LINK.
+  site_instance = content::SiteInstance::CreateForURL(
+      browser()->profile(), GURL("chrome://new-tab-page"));
+  transition = ui::PAGE_TRANSITION_TYPED;
+  client.OverrideNavigationParams(site_instance.get(), &transition,
+                                  &is_renderer_initiated, &referrer,
+                                  &initiator_origin);
+  EXPECT_TRUE(
+      ui::PageTransitionCoreTypeIs(ui::PAGE_TRANSITION_TYPED, transition));
+
+  // No change for transitions on a non-NTP page.
+  site_instance = content::SiteInstance::CreateForURL(
+      browser()->profile(), GURL("https://www.example.com"));
+  transition = ui::PAGE_TRANSITION_LINK;
+  client.OverrideNavigationParams(site_instance.get(), &transition,
+                                  &is_renderer_initiated, &referrer,
+                                  &initiator_origin);
+  EXPECT_TRUE(
+      ui::PageTransitionCoreTypeIs(ui::PAGE_TRANSITION_LINK, transition));
+}
+
 #endif  // !defined(OS_ANDROID)
 
 // NOTE: Any updates to the expectations in these tests should also be done in

chrome/browser/search/search.cc

@@ -35,6 +35,7 @@
 #if !defined(OS_ANDROID)
 #include "chrome/browser/search/instant_service.h"
 #include "chrome/browser/search/instant_service_factory.h"
+#include "chrome/browser/ui/webui/new_tab_page/new_tab_page_ui.h"
 #endif
 
 namespace search {
@@ -262,9 +263,16 @@ bool IsNTPOrRelatedURL(const GURL& url, Profile* profile) {
 }
 
 bool IsNTPURL(const GURL& url) {
-  return url.SchemeIs(chrome::kChromeSearchScheme) &&
-         (url.host_piece() == chrome::kChromeSearchRemoteNtpHost ||
-          url.host_piece() == chrome::kChromeSearchLocalNtpHost);
+  if (url.SchemeIs(chrome::kChromeSearchScheme) &&
+      (url.host_piece() == chrome::kChromeSearchRemoteNtpHost ||
+       url.host_piece() == chrome::kChromeSearchLocalNtpHost)) {
+    return true;
+  }
+#if defined(OS_ANDROID)
+  return false;
+#else
+  return NewTabPageUI::IsNewTabPageOrigin(url);
+#endif
 }
 
 bool IsInstantNTP(content::WebContents* contents) {

chrome/browser/search/search_unittest.cc

@@ -408,6 +408,9 @@ TEST_F(SearchTest, IsNTPURL) {
       {"chrome-search://local-ntp/local-ntp.html?bar=abc", true,
        "Local NTP URL with params"},
       {"chrome-search://remote-ntp", true, "Remote NTP URL"},
+      {"chrome://new-tab-page", true, "WebUI NTP"},
+      {"chrome://new-tab-page/path?params", true,
+       "WebUI NTP with path and params"},
       {"invalid-scheme://local-ntp", false, "Invalid Local NTP URL"},
       {"invalid-scheme://remote-ntp", false, "Invalid Remote NTP URL"},
       {"chrome-search://most-visited/", false, "Most visited URL"},

content/browser/frame_host/navigation_request.cc

@@ -1144,7 +1144,9 @@ NavigationRequest::NavigationRequest(
 
   // Let the NTP override the navigation params and pretend that this is a
   // browser-initiated, bookmark-like navigation.
-  if (!browser_initiated_ && source_site_instance_) {
+  // TODO(crbug.com/1099431): determine why some link navigations on chrome://
+  //     pages have |browser_initiated_| set to true and others set to false.
+  if (source_site_instance_) {
     bool is_renderer_initiated = !browser_initiated_;
     Referrer referrer(*common_params_->referrer);
     GetContentClient()->browser()->OverrideNavigationParams(