Move constants used by history component to history namespace

chrome/browser/history/url_utils.cc

Index: chrome/browser/history/url_utils.cc
diff --git a/chrome/browser/history/url_utils.cc b/chrome/browser/history/url_utils.cc
index 0c7369a2e0a7496bac80b4e941c6042045d4d30c..4be8768848ed43eb53a9eb241d30d5050014982b 100644
--- a/chrome/browser/history/url_utils.cc
+++ b/chrome/browser/history/url_utils.cc
@@ -6,6 +6,8 @@
 
 #include <algorithm>
 
+#include "chrome/common/url_constants.h"
+#include "components/dom_distiller/core/url_constants.h"
 #include "url/gurl.h"
 
 namespace history {
@@ -85,4 +87,28 @@ GURL ToggleHTTPAndHTTPS(const GURL& url) {
   return url.ReplaceComponents(replacement);
 }
 
+bool CanAddURL(const GURL& url) {

[sdefresne, 2014/11/21 14:11:41]

We won't be able to move this function into the component (since it depends on
chrome and content namespace). I'm okay with having such a function living in
the embedder that can be used by the rest of the embedder code, but for the
component this won't work.

My recommendation is to have:

// in //components/history/core/browser/history_client.h
class HistoryClient {
 public:
  virtual bool CanAddURL(const GURL& url) const = 0;
};

// in //components/history/core/browser/top_sites_client.h
class TopSitesClient {
 public:
  virtual bool CanAddURL(const GURL& url) const = 0;
};

// in //chrome/browser/history/chrome_history_client.cc
bool ChromeHistoryClient::CanAddURL(const GURL& url) const {
  return history::CanAddURL(url);
}

// in //chrome/browser/history/chrome_top_sites_client.cc
bool ChromeTopSitesClient::CanAddURL(const GURL& url) const {
  return history::CanAddURL(url);
}

// in HistoryService
void HistoryService::AddPage(const history::HistoryAddPageArgs& add_page_args) {
  if (history_client_ && !history_client_->CanAddURL(add_page_args.url))
    return
  ...
}

[nshaik, 2014/11/25 22:21:14]

Will do it in the other CL.

+  if (!url.is_valid())
+    return false;
+
+  // TODO: We should allow kChromeUIScheme URLs if they have been explicitly
+  // typed.  Right now, however, these are marked as typed even when triggered
+  // by a shortcut or menu action.
+  if (url.SchemeIs(url::kJavaScriptScheme) ||
+      url.SchemeIs(content::kChromeDevToolsScheme) ||
+      url.SchemeIs(content::kChromeUIScheme) ||
+      url.SchemeIs(content::kViewSourceScheme) ||
+      url.SchemeIs(chrome::kChromeNativeScheme) ||
+      url.SchemeIs(chrome::kChromeSearchScheme) ||
+      url.SchemeIs(dom_distiller::kDomDistillerScheme))
+    return false;
+
+  // Allow all about: and chrome: URLs except about:blank, since the user may
+  // like to see "chrome://memory/", etc. in their history and autocomplete.
+  if (url == GURL(url::kAboutBlankURL))
+    return false;
+
+  return true;
+}
+
 }  // namespace history