use brave:// for the virtual url and chrome:// for the internal u…

…rl to load fix brave/brave-browser#1616
brave · Jan 22, 2019 · 742129b · 742129b
1 parent 0e1ce21
commit 742129b
Show file tree

Hide file tree

Showing 14 changed files with 178 additions and 181 deletions.
diff --git a/browser/brave_content_browser_client.cc b/browser/brave_content_browser_client.cc
@@ -61,29 +61,52 @@ using extensions::ChromeContentBrowserClientExtensionsPart;
 
 namespace {
 
-bool HandleURLRewrite(GURL* url,
-                      content::BrowserContext* browser_context) {
-  if (url->SchemeIs(content::kChromeUIScheme) &&
-      (url->host() == chrome::kChromeUIWelcomeHost ||
-       url->host() == chrome::kChromeUIWelcomeWin10Host)) {
-    *url = GURL(kBraveUIWelcomeURL);
+bool HandleURLOverrideRewrite(GURL* url,
+                              content::BrowserContext* browser_context) {
+  // redirect sync-internals
+  if (url->host() == chrome::kChromeUISyncInternalsHost ||
+      url->host() == chrome::kChromeUISyncHost) {
+    GURL::Replacements replacements;
+    replacements.SetHostStr(chrome::kChromeUISyncHost);
+    *url = url->ReplaceComponents(replacements);
     return true;
   }
-  if (url->SchemeIs(content::kChromeUIScheme) &&
-      (url->host() == kBraveUISyncHost)) {
-    *url = GURL(kBraveUISyncURL);
+
+  // no special win10 welcome page
+  if (url->host() == chrome::kChromeUIWelcomeWin10Host ||
+      url->host() == chrome::kChromeUIWelcomeHost) {
+    *url = GURL(chrome::kChromeUIWelcomeURL);
     return true;
   }
 
   return false;
 }
 
-bool HandleURLReverseRewrite(GURL* url,
+
+bool HandleURLReverseOverrideRewrite(GURL* url,
                              content::BrowserContext* browser_context) {
-  if (url->spec() == kBraveUIWelcomeURL ||
-      url->spec() == kBraveUISyncURL) {
+  if (url->host() == chrome::kChromeUIWelcomeHost ||
+      url->host() == chrome::kChromeUISyncHost) {
+    GURL::Replacements replacements;
+    replacements.SetSchemeStr(kBraveUIScheme);
+    *url = url->ReplaceComponents(replacements);
     return true;
   }
+
+  return false;
+}
+
+bool HandleURLRewrite(GURL* url,
+                      content::BrowserContext* browser_context) {
+  if (url->SchemeIs(kBraveUIScheme)) {
+    GURL::Replacements replacements;
+    replacements.SetSchemeStr(content::kChromeUIScheme);
+    *url = url->ReplaceComponents(replacements);
+  }
+
+  if (HandleURLOverrideRewrite(url, browser_context))
+    return true;
+
   return false;
 }
 
@@ -110,8 +133,10 @@ void BraveContentBrowserClient::BrowserURLHandlerCreated(
   handler->AddHandlerPair(&webtorrent::HandleTorrentURLRewrite,
                           &webtorrent::HandleTorrentURLReverseRewrite);
   handler->AddHandlerPair(&HandleURLRewrite,
-                          &HandleURLReverseRewrite);
+                          &HandleURLReverseOverrideRewrite);
   ChromeContentBrowserClient::BrowserURLHandlerCreated(handler);
+  handler->AddHandlerPair(&HandleURLOverrideRewrite,
+                          &HandleURLReverseOverrideRewrite);
 }
 
 bool BraveContentBrowserClient::AllowAccessCookie(const GURL& url, const GURL& first_party,

diff --git a/browser/brave_content_browser_client_browsertest.cc b/browser/brave_content_browser_client_browsertest.cc
@@ -19,6 +19,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/common/buildflags.h"
 #include "chrome/common/chrome_content_client.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 "components/content_settings/core/browser/host_content_settings_map.h"
@@ -114,36 +115,127 @@ class BraveContentBrowserClientTest : public InProcessBrowserTest {
 };
 
 IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest, CanLoadChromeURL) {
-  GURL chrome_settings_url("chrome://about/");
-  std::vector<GURL> urls{chrome_settings_url, GURL("about:about/"),
-                         GURL("brave://about/")};
-  std::for_each(urls.begin(), urls.end(),
-                [this, &chrome_settings_url](const GURL& url) {
-                  content::WebContents* contents =
-                      browser()->tab_strip_model()->GetActiveWebContents();
-                  ui_test_utils::NavigateToURL(browser(), url);
-                  ASSERT_TRUE(WaitForLoadStop(contents));
-                  EXPECT_STREQ(contents->GetLastCommittedURL().spec().c_str(),
-                               chrome_settings_url.spec().c_str());
-                });
+  std::vector<std::string> pages {
+    chrome::kChromeUIWelcomeHost,
+  };
+
+  std::vector<std::string> schemes {
+    "about:",
+    "brave://",
+    "chrome://",
+  };
+
+  for (const std::string& page : pages) {
+    for (const std::string& scheme : schemes) {
+      content::WebContents* contents =
+          browser()->tab_strip_model()->GetActiveWebContents();
+      ui_test_utils::NavigateToURL(browser(), GURL(scheme + page + "/"));
+      ASSERT_TRUE(WaitForLoadStop(contents));
+
+      EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
+                       ->GetVirtualURL().spec().c_str(),
+                   ("brave://" + page + "/").c_str());
+      EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
+                       ->GetURL().spec().c_str(),
+                   ("chrome://" + page + "/").c_str());
+    }
+  }
 }
 
 IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest, CanLoadCustomBravePages) {
-  std::vector<GURL> urls {
-    GURL("chrome://adblock/"),
+  std::vector<std::string> pages {
+    "adblock",
 #if BUILDFLAG(BRAVE_REWARDS_ENABLED)
-        GURL("chrome://rewards/"),
+    "rewards",
 #endif
-        GURL("chrome://welcome/"), GURL("chrome://sync/")
+    chrome::kChromeUISyncHost,
+  };
+
+  std::vector<std::string> schemes {
+    "brave://",
+    "chrome://",
+  };
+
+  for (const std::string& page : pages) {
+    for (const std::string& scheme : schemes) {
+      content::WebContents* contents =
+          browser()->tab_strip_model()->GetActiveWebContents();
+      ui_test_utils::NavigateToURL(browser(), GURL(scheme + page + "/"));
+      ASSERT_TRUE(WaitForLoadStop(contents));
+
+      EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
+                       ->GetVirtualURL().spec().c_str(),
+                   ("brave://" + page + "/").c_str());
+      EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
+                       ->GetURL().spec().c_str(),
+                   ("chrome://" + page + "/").c_str());
+    }
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest, CanLoadAboutHost) {
+  std::vector<std::string> schemes {
+    "chrome://",
+    "brave://",
   };
-  std::for_each(urls.begin(), urls.end(), [this](const GURL& url) {
+
+  for (const std::string& scheme : schemes) {
     content::WebContents* contents =
         browser()->tab_strip_model()->GetActiveWebContents();
-    ui_test_utils::NavigateToURL(browser(), url);
+    ui_test_utils::NavigateToURL(browser(), GURL(scheme + "about/"));
+      ASSERT_TRUE(WaitForLoadStop(contents));
+
+      EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
+                       ->GetVirtualURL().spec().c_str(),
+                   "brave://about/");
+      EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
+                       ->GetURL().spec().c_str(),
+                   "chrome://chrome-urls/");
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest,
+    RewriteChromeSyncInternals) {
+  std::vector<std::string> schemes {
+    "brave://",
+    "chrome://",
+  };
+
+  for (const std::string& scheme : schemes) {
+    content::WebContents* contents =
+        browser()->tab_strip_model()->GetActiveWebContents();
+    ui_test_utils::NavigateToURL(browser(), GURL(scheme + "sync-internals/"));
+      ASSERT_TRUE(WaitForLoadStop(contents));
+
+      EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
+                       ->GetVirtualURL().spec().c_str(),
+                   "brave://sync/");
+      EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
+                       ->GetURL().spec().c_str(),
+                   "chrome://sync/");
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest,
+    RewriteChromeWelcomeWin10) {
+  std::vector<std::string> schemes {
+    "brave://",
+    "chrome://",
+  };
+
+  for (const std::string& scheme : schemes) {
+    content::WebContents* contents =
+        browser()->tab_strip_model()->GetActiveWebContents();
+    ui_test_utils::NavigateToURL(browser(), GURL(scheme + "welcome-win10/"));
     ASSERT_TRUE(WaitForLoadStop(contents));
-    EXPECT_STREQ(contents->GetLastCommittedURL().spec().c_str(),
-                 url.spec().c_str());
-  });
+
+    EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
+                     ->GetVirtualURL().spec().c_str(),
+                 "brave://welcome/");
+    EXPECT_STREQ(contents->GetController().GetLastCommittedEntry()
+                     ->GetURL().spec().c_str(),
+                 "chrome://welcome/");
+  }
 }
 
 IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientTest, RewriteMagnetURLURLBar) {

diff --git a/browser/ui/webui/brave_web_ui_controller_factory.cc b/browser/ui/webui/brave_web_ui_controller_factory.cc
@@ -70,7 +70,7 @@ WebUIFactoryFunction GetWebUIFactoryFunction(WebUI* web_ui,
       url.host_piece() == kDonateHost ||
 #endif
       url.host_piece() == kWelcomeHost ||
-      url.host_piece() == kBraveUIWelcomeURL ||
+      url.host_piece() == chrome::kChromeUIWelcomeURL ||
       (url.host_piece() == kBraveUISyncHost &&
           brave_sync::BraveSyncService::is_enabled()) ||
       url.host_piece() == chrome::kChromeUINewTabHost ||

diff --git a/browser/ui/webui/brave_welcome_ui_browsertest.cc b/browser/ui/webui/brave_welcome_ui_browsertest.cc
@@ -14,6 +14,7 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/webui_url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/browser/navigation_entry.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
 
@@ -38,8 +39,10 @@ IN_PROC_BROWSER_TEST_F(BraveWelcomeUIBrowserTest, PRE_StartupURLTest) {
   content::WebContents* web_contents = tab_strip->GetWebContentsAt(0);
   content::TestNavigationObserver observer(web_contents, 1);
   observer.Wait();
-  EXPECT_EQ(kBraveUIWelcomeURL,
-            tab_strip->GetWebContentsAt(0)->GetURL().possibly_invalid_spec());
+  EXPECT_STREQ("brave://welcome/",
+            tab_strip->GetWebContentsAt(0)
+                ->GetController().GetLastCommittedEntry()
+                    ->GetVirtualURL().possibly_invalid_spec().c_str());
 }
 
 // Check wheter startup url is not welcome ui at second run.

diff --git a/chromium_src/chrome/browser/browser_about_handler.cc b/chromium_src/chrome/browser/browser_about_handler.cc
@@ -3,32 +3,33 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #define FixupBrowserAboutURL FixupBrowserAboutURL_ChromiumImpl
-#define WillHandleBrowserAboutURL WillHandleBrowserAboutURL_ChromiumImpl
 #include "../../../../chrome/browser/browser_about_handler.cc"
 #undef FixupBrowserAboutURL
-#undef WillHandleBrowserAboutURL
 
 #include "brave/common/url_constants.h"
+#include "brave/common/webui_url_constants.h"
 
-bool FixupBrowserAboutURLBraveImpl(GURL* url,
-                                   content::BrowserContext* browser_context) {
-  if (url->SchemeIs(kBraveUIScheme)) {
+bool FixupBrowserAboutURL(GURL* url,
+                          content::BrowserContext* browser_context) {
+  bool result = FixupBrowserAboutURL_ChromiumImpl(url, browser_context);
+
+  // no special win10 welcome page
+  if (url->host() == chrome::kChromeUIWelcomeWin10Host)
+    *url = GURL(chrome::kChromeUIWelcomeURL);
+
+  // redirect sync-internals
+  if (url->host() == chrome::kChromeUISyncInternalsHost) {
     GURL::Replacements replacements;
-    replacements.SetSchemeStr(content::kChromeUIScheme);
+    replacements.SetHostStr(chrome::kChromeUISyncHost);
     *url = url->ReplaceComponents(replacements);
   }
-  return true;
-}
-
 
-bool FixupBrowserAboutURL(GURL* url,
-                          content::BrowserContext* browser_context) {
-  FixupBrowserAboutURLBraveImpl(url, browser_context);
-  return FixupBrowserAboutURL_ChromiumImpl(url, browser_context);
-}
+  if (url->SchemeIs(content::kChromeUIScheme) &&
+      url->host() != chrome::kChromeUINewTabHost) {
+    GURL::Replacements replacements;
+    replacements.SetSchemeStr(kBraveUIScheme);
+    *url = url->ReplaceComponents(replacements);
+  }
 
-bool WillHandleBrowserAboutURL(GURL* url,
-                               content::BrowserContext* browser_context) {
-  FixupBrowserAboutURLBraveImpl(url, browser_context);
-  return WillHandleBrowserAboutURL_ChromiumImpl(url, browser_context);
+  return result;
 }
diff --git a/chromium_src/chrome/browser/ui/browser.cc b/chromium_src/chrome/browser/ui/browser.cc
@@ -6,12 +6,9 @@
 #include "chrome/browser/ui/browser_content_setting_bubble_model_delegate.h"
 #include "brave/browser/ui/brave_browser_content_setting_bubble_model_delegate.h"
 #include "brave/browser/ui/brave_browser_command_controller.h"
-#include "brave/components/omnibox/browser/brave_location_bar_model_impl.h"
 
-#define LocationBarModelImpl BraveLocationBarModelImpl
 #define BrowserContentSettingBubbleModelDelegate BraveBrowserContentSettingBubbleModelDelegate
 #define BrowserCommandController BraveBrowserCommandController
 #include "../../../../../chrome/browser/ui/browser.cc"
 #undef BrowserContentSettingBubbleModelDelegate
 #undef BrowserCommandController
-#undef LocationBarModelImpl
diff --git a/common/webui_url_constants.cc b/common/webui_url_constants.cc
@@ -15,7 +15,6 @@ const char kDonateJS[] = "brave_donate.js";
 const char kBraveNewTabJS[] = "brave_new_tab.js";
 const char kBraveUISyncHost[] = "sync";
 const char kBraveSyncJS[] = "brave_sync.js";
-const char kBraveUIWelcomeURL[] = "chrome://welcome/";
 const char kBraveUIRewardsURL[] = "chrome://rewards/";
 const char kBraveUIAdblockURL[] = "chrome://adblock/";
 const char kBraveUIDonateURL[] = "chrome://donate/";

diff --git a/common/webui_url_constants.h b/common/webui_url_constants.h
@@ -12,7 +12,6 @@ extern const char kDonateJS[];
 extern const char kBraveNewTabJS[];
 extern const char kBraveUISyncHost[];
 extern const char kBraveSyncJS[];
-extern const char kBraveUIWelcomeURL[];
 extern const char kBraveUIRewardsURL[];
 extern const char kBraveUIAdblockURL[];
 extern const char kBraveUIDonateHost[];

diff --git a/components/brave_rewards/browser/rewards_service_browsertest.cc b/components/brave_rewards/browser/rewards_service_browsertest.cc
@@ -162,7 +162,7 @@ class BraveRewardsBrowserTest : public InProcessBrowserTest {
   }
 
   GURL rewards_url() {
-    GURL rewards_url("chrome://rewards");
+    GURL rewards_url("brave://rewards");
     return rewards_url;
   }
 

diff --git a/components/omnibox/browser/BUILD.gn b/components/omnibox/browser/BUILD.gn
@@ -1,13 +1,11 @@
 source_set("browser") {
   # Only //components/omnibox/browser target can depend on this target
   # because this target expands //components/omnibox/browser implementation.
-  visibility = [ "//components/omnibox/browser", ":unit_tests" ]
+  visibility = [ "//components/omnibox/browser" ]
 
   sources = [
     "brave_autocomplete_controller.cc",
     "brave_autocomplete_controller.h",
-    "brave_location_bar_model_impl.cc",
-    "brave_location_bar_model_impl.h",
     "constants.cc",
     "constants.h",
     "topsites_provider_data.cc",
@@ -22,18 +20,3 @@ source_set("browser") {
      "//third_party/metrics_proto",
   ]
 }
-
-source_set("unit_tests") {
-  testonly = true
-  sources = [
-    "location_bar_model_impl_unittest.cc",
-  ]
-
-  deps = [
-    ":browser",
-    "//base",
-    "//components/omnibox/browser",
-    "//testing/gtest",
-    "//url",
-  ]
-}