Bug 1627848 - Don't call SendVisitURI() if the URI won't be added to history r=mak
☠☠ backed out by f707e3fa32cb ☠ ☠
authorHaik Aftandilian <haftandilian@mozilla.com>
Mon, 27 Apr 2020 17:25:36 +0000
changeset 526373 0483abbea83d442f83363f1cf87659702fd835f7
parent 526372 1cd445272ca5c15d154ee81ace153b3f1d637cb0
child 526374 1a51afbff6bf7f93e9fbc5401104737a63bb4e6b
push id114230
push userhaftandilian@mozilla.com
push dateMon, 27 Apr 2020 23:42:18 +0000
treeherderautoland@0483abbea83d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1627848
milestone77.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1627848 - Don't call SendVisitURI() if the URI won't be added to history r=mak Avoid unnecessary IPC traffic by checking if URI's meet the history criteria before sending them to the parent with SendVisitURI. Differential Revision: https://phabricator.services.mozilla.com/D70217
toolkit/components/places/History.cpp
toolkit/components/places/nsNavHistory.cpp
toolkit/components/places/nsNavHistory.h
--- a/toolkit/components/places/History.cpp
+++ b/toolkit/components/places/History.cpp
@@ -1824,30 +1824,38 @@ History::VisitURI(nsIWidget* aWidget, ns
                   uint32_t aFlags) {
   MOZ_ASSERT(NS_IsMainThread());
   NS_ENSURE_ARG(aURI);
 
   if (mShuttingDown) {
     return NS_OK;
   }
 
+  nsresult rv;
   if (XRE_IsContentProcess()) {
+    bool canAddURI = false;
+    rv = nsNavHistory::CanAddURIToHistory(aURI, &canAddURI);
+    NS_ENSURE_SUCCESS(rv, rv);
+    if (!canAddURI) {
+      return NS_OK;
+    }
+
     NS_ENSURE_ARG(aWidget);
     BrowserChild* browserChild = aWidget->GetOwningBrowserChild();
     NS_ENSURE_TRUE(browserChild, NS_ERROR_FAILURE);
     (void)browserChild->SendVisitURI(aURI, aLastVisitedURI, aFlags);
     return NS_OK;
   }
 
   nsNavHistory* navHistory = nsNavHistory::GetHistoryService();
   NS_ENSURE_TRUE(navHistory, NS_ERROR_OUT_OF_MEMORY);
 
   // Silently return if URI is something we shouldn't add to DB.
   bool canAdd;
-  nsresult rv = navHistory->CanAddURI(aURI, &canAdd);
+  rv = navHistory->CanAddURI(aURI, &canAdd);
   NS_ENSURE_SUCCESS(rv, rv);
   if (!canAdd) {
     return NS_OK;
   }
 
   bool reload = false;
   if (aLastVisitedURI) {
     rv = aURI->Equals(aLastVisitedURI, &reload);
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -878,58 +878,80 @@ nsNavHistory::MarkPageAsFollowedBookmark
 //    we don't bail go on and allow it in.
 
 NS_IMETHODIMP
 nsNavHistory::CanAddURI(nsIURI* aURI, bool* canAdd) {
   NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
   NS_ENSURE_ARG(aURI);
   NS_ENSURE_ARG_POINTER(canAdd);
 
-  // Default to false.
-  *canAdd = false;
-
   // 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
+NS_IMETHODIMP
+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 (!mDB || spec.Length() > mDB->MaxUrlLength()) {
+  if (spec.Length() > MaxURILength()) {
     return NS_OK;
   }
 
   nsAutoCString scheme;
   rv = aURI->GetScheme(scheme);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // first check the most common cases (HTTP, HTTPS) to allow in to avoid most
-  // of the work
-  if (scheme.EqualsLiteral("http")) {
-    *canAdd = true;
-    return NS_OK;
-  }
-  if (scheme.EqualsLiteral("https")) {
-    *canAdd = true;
+  // first check the most common cases
+  if (scheme.EqualsLiteral("http") || scheme.EqualsLiteral("https")) {
+    *aCanAdd = true;
     return NS_OK;
   }
 
   // now check for all bad things
-  if (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")) {
-    return NS_OK;
+  *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");
+
+  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;
+    }
   }
-  *canAdd = true;
-  return NS_OK;
+  return maxSpecLength;
 }
 
 // nsNavHistory::GetNewQuery
 
 NS_IMETHODIMP
 nsNavHistory::GetNewQuery(nsINavHistoryQuery** _retval) {
   NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
   NS_ENSURE_ARG_POINTER(_retval);
--- a/toolkit/components/places/nsNavHistory.h
+++ b/toolkit/components/places/nsNavHistory.h
@@ -168,16 +168,22 @@ class nsNavHistory final : public nsSupp
    */
   nsIStringBundle* GetBundle();
   nsICollation* GetCollation();
   void GetStringFromName(const char* aName, nsACString& aResult);
   void GetAgeInDaysString(int32_t aInt, const char* aName, nsACString& aResult);
   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; }
 
   // Returns whether or not diacritics must match in history text searches.
   bool MatchDiacritics() const { return mMatchDiacritics; }
 
   // Constants for the columns returned by the above statement.
   static const int32_t kGetInfoIndex_PageID;