Bug 1843239, [Cached] Location.hash, r=peterv

Differential Revision: https://phabricator.services.mozilla.com/D183788
dothq · Jul 31, 2023 · 3931b88 · 3931b88
1 parent ae67c7a
commit 3931b88
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 8 deletions.
diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp
@@ -616,6 +616,7 @@ void BrowsingContext::SetDocShell(nsIDocShell* aDocShell) {
   }
 
   RecomputeCanExecuteScripts();
+  ClearCachedValuesOfLocations();
 }
 
 // This class implements a callback that will return the remote window proxy for
@@ -1047,6 +1048,8 @@ void BrowsingContext::PrepareForProcessChange() {
   mIsInProcess = false;
   mUserGestureStart = TimeStamp();
 
+  ClearCachedValuesOfLocations();
+
   // NOTE: For now, clear our nsDocShell reference, as we're primarily in a
   // different process now. This may need to change in the future with
   // Cross-Process BFCache.
@@ -1413,6 +1416,9 @@ BrowsingContext::~BrowsingContext() {
     sBrowsingContexts->Remove(Id());
   }
   UnregisterBrowserId(this);
+
+  ClearCachedValuesOfLocations();
+  mLocations.clear();
 }
 
 /* static */
@@ -3753,6 +3759,17 @@ void BrowsingContext::ResetLocationChangeRateLimit() {
   mLocationChangeRateLimitSpanStart = TimeStamp();
 }
 
+void BrowsingContext::LocationCreated(dom::Location* aLocation) {
+  MOZ_ASSERT(!aLocation->isInList());
+  mLocations.insertBack(aLocation);
+}
+
+void BrowsingContext::ClearCachedValuesOfLocations() {
+  for (dom::Location* loc = mLocations.getFirst(); loc; loc = loc->getNext()) {
+    loc->ClearCachedValues();
+  }
+}
+
 }  // namespace dom
 
 namespace ipc {

diff --git a/docshell/base/BrowsingContext.h b/docshell/base/BrowsingContext.h
@@ -68,6 +68,7 @@ class ChildSHistory;
 class ContentParent;
 class Element;
 struct LoadingSessionHistoryInfo;
+class Location;
 template <typename>
 struct Nullable;
 template <typename T>
@@ -948,6 +949,9 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
     return GetIsUnderHiddenEmbedderElement();
   }
 
+  void LocationCreated(dom::Location* aLocation);
+  void ClearCachedValuesOfLocations();
+
  protected:
   virtual ~BrowsingContext();
   BrowsingContext(WindowContext* aParentWindow, BrowsingContextGroup* aGroup,
@@ -1406,6 +1410,8 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
   // Used by CheckLocationChangeRateLimit. Do not apply cross-process.
   uint32_t mLocationChangeRateLimitCount;
   mozilla::TimeStamp mLocationChangeRateLimitSpanStart;
+
+  mozilla::LinkedList<dom::Location> mLocations;
 };
 
 /**

diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
@@ -1550,7 +1550,7 @@ bool nsDocShell::SetCurrentURI(nsIURI* aURI, nsIRequest* aRequest,
     mTitleValidForCurrentURI = false;
   }
 
-  mCurrentURI = aURI;
+  SetCurrentURIInternal(aURI);
 
 #ifdef DEBUG
   mLastOpenedURI = aURI;
@@ -1576,6 +1576,13 @@ bool nsDocShell::SetCurrentURI(nsIURI* aURI, nsIRequest* aRequest,
   return !aFireOnLocationChange;
 }
 
+void nsDocShell::SetCurrentURIInternal(nsIURI* aURI) {
+  mCurrentURI = aURI;
+  if (mBrowsingContext) {
+    mBrowsingContext->ClearCachedValuesOfLocations();
+  }
+}
+
 NS_IMETHODIMP
 nsDocShell::GetCharset(nsACString& aCharset) {
   aCharset.Truncate();
@@ -4567,7 +4574,7 @@ nsDocShell::Destroy() {
   nsDocLoader::Destroy();
 
   mParentWidget = nullptr;
-  mCurrentURI = nullptr;
+  SetCurrentURIInternal(nullptr);
 
   if (mScriptGlobal) {
     mScriptGlobal->DetachFromDocShell(!mWillChangeProcess);
@@ -8182,7 +8189,7 @@ nsresult nsDocShell::SetupNewViewer(nsIContentViewer* aNewViewer,
     viewer->Close(nullptr);
     viewer->Destroy();
     mContentViewer = nullptr;
-    mCurrentURI = nullptr;
+    SetCurrentURIInternal(nullptr);
     NS_WARNING("ContentViewer Initialization failed");
     return NS_ERROR_FAILURE;
   }

diff --git a/docshell/base/nsDocShell.h b/docshell/base/nsDocShell.h
@@ -1160,7 +1160,10 @@ class nsDocShell final : public nsDocLoader,
                                const nsAString& aTitle, bool aReplace,
                                nsIURI* aCurrentURI, bool aEqualURIs);
 
- private:  // data members
+ private:
+  void SetCurrentURIInternal(nsIURI* aURI);
+
+  // data members
   nsString mTitle;
   nsCString mOriginalUriString;
   nsTObserverArray<nsWeakPtr> mPrivacyObservers;
@@ -1205,6 +1208,7 @@ class nsDocShell final : public nsDocLoader,
   mozilla::UniquePtr<mozilla::ObservedDocShell> mObserved;
 
   // mCurrentURI should be marked immutable on set if possible.
+  // Change mCurrentURI only through SetCurrentURIInternal method.
   nsCOMPtr<nsIURI> mCurrentURI;
   nsCOMPtr<nsIReferrerInfo> mReferrerInfo;
 

diff --git a/dom/base/Location.cpp b/dom/base/Location.cpp
@@ -42,13 +42,18 @@ namespace mozilla::dom {
 Location::Location(nsPIDOMWindowInner* aWindow,
                    BrowsingContext* aBrowsingContext)
     : mInnerWindow(aWindow) {
-  // aBrowsingContext can be null if it gets called after nsDocShell::Destory().
+  // aBrowsingContext can be null if it gets called after nsDocShell::Destroy().
   if (aBrowsingContext) {
     mBrowsingContextId = aBrowsingContext->Id();
+    aBrowsingContext->LocationCreated(this);
   }
 }
 
-Location::~Location() = default;
+Location::~Location() {
+  if (isInList()) {
+    remove();
+  }
+}
 
 // QueryInterface implementation for Location
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(Location)
@@ -646,4 +651,8 @@ JSObject* Location::WrapObject(JSContext* aCx,
   return Location_Binding::Wrap(aCx, this, aGivenProto);
 }
 
+void Location::ClearCachedValues() {
+  Location_Binding::ClearCachedHashValue(this);
+}
+
 }  // namespace mozilla::dom
diff --git a/dom/base/Location.h b/dom/base/Location.h
@@ -9,6 +9,7 @@
 
 #include "js/TypeDecls.h"
 #include "mozilla/ErrorResult.h"
+#include "mozilla/LinkedList.h"
 #include "mozilla/dom/BrowsingContext.h"
 #include "mozilla/dom/LocationBase.h"
 #include "nsCycleCollectionParticipant.h"
@@ -28,7 +29,8 @@ namespace mozilla::dom {
 
 class Location final : public nsISupports,
                        public LocationBase,
-                       public nsWrapperCache {
+                       public nsWrapperCache,
+                       public LinkedListElement<Location> {
  public:
   typedef BrowsingContext::LocationProxy RemoteProxy;
 
@@ -110,6 +112,8 @@ class Location final : public nsISupports,
 
   nsresult ToString(nsAString& aString) { return GetHref(aString); }
 
+  void ClearCachedValues();
+
  protected:
   virtual ~Location();
 

diff --git a/dom/webidl/Location.webidl b/dom/webidl/Location.webidl
@@ -11,6 +11,9 @@
  * and create derivative works of this document.
  */
 
+// Consider to cache more attributes, if the getters show up badly in
+// the performance profiles.
+
 [LegacyUnforgeable,
  Exposed=Window]
 interface Location {
@@ -30,7 +33,7 @@ interface Location {
            attribute USVString pathname;
   [Throws, NeedsSubjectPrincipal]
            attribute USVString search;
-  [Throws, NeedsSubjectPrincipal]
+  [Throws, NeedsSubjectPrincipal, Cached, Pure]
            attribute USVString hash;
 
   [Throws, NeedsSubjectPrincipal]