Move constants used by history component to history namespace

chrome/browser/history/url_utils.cc

 // Copyright 2013 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 "chrome/browser/history/url_utils.h"
 
 #include <algorithm>
 
+#include "chrome/common/url_constants.h"
+#include "components/dom_distiller/core/url_constants.h"
 #include "url/gurl.h"
 
 namespace history {
 
 namespace {
 
 // Comparator to enforce '\0' < '?' < '#' < '/' < other characters.
 int GetURLCharPriority(char ch) {
   switch (ch) {
     case '\0': return 0;
@@ skipping 59 matching lines @@
     new_scheme = "http";
   else
     return GURL::EmptyGURL();
   url::Component comp;
   comp.len = new_scheme.length();
   GURL::Replacements replacement;
   replacement.SetScheme(new_scheme.c_str(), comp);
   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