Bug 1680977 - Move nsNavHistory::CanAddURIToHistory to BaseHistory. r=mak

And do some minor improvements (use static prefs etc) while at it.

Differential Revision: https://phabricator.services.mozilla.com/D99584

docshell/base/BaseHistory.cpp

@@ -10,19 +10,44 @@
 #include "mozilla/dom/Document.h"
 #include "mozilla/dom/Link.h"
 #include "mozilla/dom/Element.h"
+#include "mozilla/StaticPrefs_browser.h"
 #include "mozilla/StaticPrefs_layout.h"
 
 namespace mozilla {
 
 using mozilla::dom::ContentParent;
-using mozilla::dom::Document;
-using mozilla::dom::Element;
 using mozilla::dom::Link;
 
 BaseHistory::BaseHistory() : mTrackedURIs(kTrackedUrisInitialSize) {}
 
 BaseHistory::~BaseHistory() = default;
 
+static constexpr nsLiteralCString kDisallowedSchemes[] = {
+    "about"_ns,         "blob"_ns,       "data"_ns,     "chrome"_ns,
+    "imap"_ns,          "javascript"_ns, "mailbox"_ns,  "moz-anno"_ns,
+    "news"_ns,          "page-icon"_ns,  "resource"_ns, "view-source"_ns,
+    "moz-extension"_ns,
+};
+
+bool BaseHistory::CanStore(nsIURI* aURI) {
+  nsAutoCString scheme;
+  if (NS_WARN_IF(NS_FAILED(aURI->GetScheme(scheme)))) {
+    return false;
+  }
+
+  if (!scheme.EqualsLiteral("http") && !scheme.EqualsLiteral("https")) {
+    for (const nsLiteralCString& disallowed : kDisallowedSchemes) {
+      if (scheme.Equals(disallowed)) {
+        return false;
+      }
+    }
+  }
+
+  nsAutoCString spec;
+  aURI->GetSpec(spec);
+  return spec.Length() <= StaticPrefs::browser_history_maxUrlLength();
+}
+
 void BaseHistory::ScheduleVisitedQuery(nsIURI* aURI) {
   mPendingQueries.PutEntry(aURI);
   if (mStartPendingVisitedQueriesScheduled) {

docshell/base/BaseHistory.h

@@ -18,6 +18,10 @@ class BaseHistory : public IHistory {
   void UnregisterVisitedCallback(nsIURI*, dom::Link*) final;
   void NotifyVisited(nsIURI*, VisitedStatus) final;
 
+  // Some URIs like data-uris are never going to be stored in history, so we can
+  // avoid doing IPC roundtrips for them or what not.
+  static bool CanStore(nsIURI*);
+
  protected:
   void NotifyVisitedInThisProcess(nsIURI*, VisitedStatus);
   void NotifyVisitedFromParent(nsIURI*, VisitedStatus);

modules/libpref/init/StaticPrefList.yaml

@@ -1017,6 +1017,22 @@
   value: @IS_NOT_ANDROID@
   mirror: always
 
+# The max url length we'll store in history.
+#
+# The default value is mostly a guess based on various facts:
+#
+# * IE didn't support urls longer than 2083 chars
+# * Sitemaps protocol used to support a maximum of 2048 chars
+# * Various SEO guides suggest to not go over 2000 chars
+# * Various apps/services are known to have issues over 2000 chars
+# * RFC 2616 - HTTP/1.1 suggests being cautious about depending
+#   on URI lengths above 255 bytes
+#
+- name: browser.history.maxUrlLength
+  type: uint32_t
+  value: 2000
+  mirror: always
+
 # Render animations and videos as a solid color
 - name: browser.measurement.render_anims_and_video_solid
   type: RelaxedAtomicBool

toolkit/components/places/History.cpp

@@ -1816,10 +1816,7 @@ History::VisitURI(nsIWidget* aWidget, nsIURI* aURI, nsIURI* aLastVisitedURI,
 
   nsresult rv;
   if (XRE_IsContentProcess()) {
-    bool canAddURI = false;
-    rv = nsNavHistory::CanAddURIToHistory(aURI, &canAddURI);
-    NS_ENSURE_SUCCESS(rv, rv);
-    if (!canAddURI) {
+    if (!BaseHistory::CanStore(aURI)) {
       return NS_OK;
     }
 

toolkit/components/places/nsNavHistory.cpp

@@ -879,74 +879,10 @@ nsNavHistory::CanAddURI(nsIURI* aURI, bool* canAdd) {
   NS_ENSURE_ARG_POINTER(canAdd);
 
   // If history is disabled, don't add any entry.
-  if (IsHistoryDisabled()) {
-    *canAdd = false;
-    return NS_OK;
-  }
-
-  return CanAddURIToHistory(aURI, canAdd);
-}
-
-// nsNavHistory::CanAddURIToHistory
-//
-//    Helper for nsNavHistory::CanAddURI to be callable from a child process
-
-// static
-nsresult nsNavHistory::CanAddURIToHistory(nsIURI* aURI, bool* aCanAdd) {
-  // Default to false.
-  *aCanAdd = false;
-
-  // If the url length is over a threshold, don't add it.
-  nsCString spec;
-  nsresult rv = aURI->GetSpec(spec);
-  NS_ENSURE_SUCCESS(rv, rv);
-  if (spec.Length() > MaxURILength()) {
-    return NS_OK;
-  }
-
-  nsAutoCString scheme;
-  rv = aURI->GetScheme(scheme);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  // first check the most common cases
-  if (scheme.EqualsLiteral("http") || scheme.EqualsLiteral("https")) {
-    *aCanAdd = true;
-    return NS_OK;
-  }
-
-  // now check for all bad things
-  *aCanAdd =
-      !scheme.EqualsLiteral("about") && !scheme.EqualsLiteral("blob") &&
-      !scheme.EqualsLiteral("chrome") && !scheme.EqualsLiteral("data") &&
-      !scheme.EqualsLiteral("imap") && !scheme.EqualsLiteral("javascript") &&
-      !scheme.EqualsLiteral("mailbox") && !scheme.EqualsLiteral("moz-anno") &&
-      !scheme.EqualsLiteral("news") && !scheme.EqualsLiteral("page-icon") &&
-      !scheme.EqualsLiteral("resource") &&
-      !scheme.EqualsLiteral("view-source") &&
-      !scheme.EqualsLiteral("moz-extension");
-
+  *canAdd = !IsHistoryDisabled() && BaseHistory::CanStore(aURI);
   return NS_OK;
 }
 
-// nsNavHistory::MaxURILength
-
-// static
-uint32_t nsNavHistory::MaxURILength() {
-  // Duplicates Database::MaxUrlLength() for use in
-  // child processes without a database instance.
-  static uint32_t maxSpecLength = 0;
-  if (!maxSpecLength) {
-    maxSpecLength = Preferences::GetInt(PREF_HISTORY_MAXURLLEN,
-                                        PREF_HISTORY_MAXURLLEN_DEFAULT);
-    if (maxSpecLength < 255 || maxSpecLength > INT32_MAX) {
-      maxSpecLength = PREF_HISTORY_MAXURLLEN_DEFAULT;
-    }
-  }
-  return maxSpecLength;
-}
-
-// nsNavHistory::GetNewQuery
-
 NS_IMETHODIMP
 nsNavHistory::GetNewQuery(nsINavHistoryQuery** _retval) {
   NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");

toolkit/components/places/nsNavHistory.h

@@ -173,12 +173,6 @@ class nsNavHistory final : public nsSupportsWeakReference,
   static void GetMonthName(const PRExplodedTime& aTime, nsACString& aResult);
   static void GetMonthYear(const PRExplodedTime& aTime, nsACString& aResult);
 
-  // Returns true if the provided URI spec and scheme is allowed in history
-  static nsresult CanAddURIToHistory(nsIURI* aURI, bool* aCanAdd);
-
-  // The max URI spec length allowed for a URI to be added to history
-  static uint32_t MaxURILength();
-
   // Returns whether history is enabled or not.
   bool IsHistoryDisabled() { return !mHistoryEnabled; }