Backed out changeset 21276955f659 (bug 1720926) for causing hybrid bustages on nsIPrincipal. CLOSED TREE
authorcriss <ccozmuta@mozilla.com>
Mon, 18 Oct 2021 16:30:48 +0300
changeset 596179 ee8efced380b871deac4fba285955953a4a89ef5
parent 596178 51ad19d01a3a334a9d78bb8dbe3229acd1b94a68
child 596180 7eadd05f0e7662447eed3afa22acf0a3dd4d266f
child 596181 2ffa9258ed043ad445acbf707dadaeb1a9ec6ef3
push id38889
push userncsoregi@mozilla.com
push dateMon, 18 Oct 2021 16:09:29 +0000
treeherdermozilla-central@ee8efced380b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1720926
milestone95.0a1
backs out21276955f659026ff55e579d9ca37a284552e7b3
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
Backed out changeset 21276955f659 (bug 1720926) for causing hybrid bustages on nsIPrincipal. CLOSED TREE
docshell/test/navigation/test_rate_limit_location_change.html
dom/base/Location.cpp
dom/base/Location.h
dom/base/nsHistory.cpp
dom/base/nsHistory.h
dom/webidl/History.webidl
--- a/docshell/test/navigation/test_rate_limit_location_change.html
+++ b/docshell/test/navigation/test_rate_limit_location_change.html
@@ -24,27 +24,23 @@ https://bugzilla.mozilla.org/show_bug.cg
   let inc = 0;
 
   const rateLimitedFunctions = (win) => ({
     "history.replaceState": () => win.history.replaceState(null, "test", `${win.location.href}#${inc++}`),
     "history.pushState":  () => win.history.pushState(null, "test", `${win.location.href}#${inc++}`),
     "history.back": () => win.history.back(),
     "history.forward": () => win.history.forward(),
     "history.go": () => win.history.go(-1),
-    "location.href": () => win.location.href = win.location.href + "",
     "location.hash": () => win.location.hash = inc++,
     "location.host": () => win.location.host = win.location.host + "",
     "location.hostname": () => win.location.hostname = win.location.hostname + "",
     "location.pathname": () => win.location.pathname = win.location.pathname + "",
     "location.port": () => win.location.port = win.location.port + "",
     "location.protocol": () => win.location.protocol = win.location.protocol + "",
     "location.search": () => win.location.search = win.location.search + "",
-    "location.assign": () => win.location.assign(`${win.location.href}#${inc++}`),
-    "location.replace": () => win.location.replace(`${win.location.href}#${inc++}`),
-    "location.reload": () => win.location.reload(),
   });
 
   async function test() {
     await setup();
 
     // Open new window and wait for it to load
     let win = window.open("blank.html");
     await new Promise((resolve) => SimpleTest.waitForFocus(resolve, win))
--- a/dom/base/Location.cpp
+++ b/dom/base/Location.cpp
@@ -543,23 +543,17 @@ void Location::SetSearch(const nsAString
   }
   if (NS_WARN_IF(aRv.Failed())) {
     return;
   }
 
   SetURI(uri, aSubjectPrincipal, aRv);
 }
 
-void Location::Reload(bool aForceget, nsIPrincipal& aSubjectPrincipal,
-                      ErrorResult& aRv) {
-  if (!CallerSubsumes(&aSubjectPrincipal)) {
-    aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
-    return;
-  }
-
+void Location::Reload(bool aForceget, ErrorResult& aRv) {
   nsCOMPtr<nsIDocShell> docShell(GetDocShell());
   if (!docShell) {
     return aRv.Throw(NS_ERROR_FAILURE);
   }
 
   if (StaticPrefs::dom_block_reload_from_resize_event_handler()) {
     nsCOMPtr<nsPIDOMWindowOuter> window = docShell->GetWindow();
     if (window && window->IsHandlingResizeEvent()) {
@@ -575,39 +569,24 @@ void Location::Reload(bool aForceget, ns
       if (doc && (pcx = doc->GetPresContext())) {
         pcx->RebuildAllStyleData(NS_STYLE_HINT_REFLOW,
                                  RestyleHint::RestyleSubtree());
       }
       return;
     }
   }
 
-  RefPtr<BrowsingContext> bc = GetBrowsingContext();
-  if (!bc || bc->IsDiscarded()) {
-    return;
-  }
-
-  CallerType callerType = aSubjectPrincipal.IsSystemPrincipal()
-                              ? CallerType::System
-                              : CallerType::NonSystem;
-
-  nsresult rv = bc->CheckLocationChangeRateLimit(callerType);
-  if (NS_FAILED(rv)) {
-    aRv.Throw(rv);
-    return;
-  }
-
   uint32_t reloadFlags = nsIWebNavigation::LOAD_FLAGS_NONE;
 
   if (aForceget) {
     reloadFlags = nsIWebNavigation::LOAD_FLAGS_BYPASS_CACHE |
                   nsIWebNavigation::LOAD_FLAGS_BYPASS_PROXY;
   }
 
-  rv = nsDocShell::Cast(docShell)->Reload(reloadFlags);
+  nsresult rv = nsDocShell::Cast(docShell)->Reload(reloadFlags);
   if (NS_FAILED(rv) && rv != NS_BINDING_ABORTED) {
     // NS_BINDING_ABORTED is returned when we attempt to reload a POST result
     // and the user says no at the "do you want to reload?" prompt.  Don't
     // propagate this one back to callers.
     return aRv.Throw(rv);
   }
 }
 
--- a/dom/base/Location.h
+++ b/dom/base/Location.h
@@ -38,17 +38,24 @@ class Location final : public nsISupport
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(Location)
 
   // WebIDL API:
   void Assign(const nsAString& aUrl, nsIPrincipal& aSubjectPrincipal,
               ErrorResult& aError);
 
   void Reload(bool aForceget, nsIPrincipal& aSubjectPrincipal,
-              ErrorResult& aError);
+              ErrorResult& aError) {
+    if (!CallerSubsumes(&aSubjectPrincipal)) {
+      aError.Throw(NS_ERROR_DOM_SECURITY_ERR);
+      return;
+    }
+
+    Reload(aForceget, aError);
+  }
 
   void GetHref(nsAString& aHref, nsIPrincipal& aSubjectPrincipal,
                ErrorResult& aError) {
     if (!CallerSubsumes(&aSubjectPrincipal)) {
       aError.Throw(NS_ERROR_DOM_SECURITY_ERR);
       return;
     }
 
@@ -106,16 +113,18 @@ class Location final : public nsISupport
                                JS::Handle<JSObject*> aGivenProto) override;
 
   // Non WebIDL methods:
 
   nsresult GetHref(nsAString& aHref);
 
   nsresult ToString(nsAString& aString) { return GetHref(aString); }
 
+  void Reload(bool aForceget, ErrorResult& aRv);
+
  protected:
   virtual ~Location();
 
   BrowsingContext* GetBrowsingContext() override;
   already_AddRefed<nsIDocShell> GetDocShell() override;
 
   // In the case of jar: uris, we sometimes want the place the jar was
   // fetched from as the URI instead of the jar: uri itself.  Pass in
--- a/dom/base/nsHistory.cpp
+++ b/dom/base/nsHistory.cpp
@@ -132,53 +132,48 @@ void nsHistory::GetState(JSContext* aCx,
     }
 
     return;
   }
 
   aResult.setNull();
 }
 
-void nsHistory::Go(int32_t aDelta, nsIPrincipal& aSubjectPrincipal,
-                   ErrorResult& aRv) {
+void nsHistory::Go(int32_t aDelta, CallerType aCallerType, ErrorResult& aRv) {
   LOG(("nsHistory::Go(%d)", aDelta));
   nsCOMPtr<nsPIDOMWindowInner> win(do_QueryReferent(mInnerWindow));
   if (!win || !win->HasActiveDocument()) {
     return aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
   }
 
   if (!aDelta) {
     // https://html.spec.whatwg.org/multipage/history.html#the-history-interface
     // "When the go(delta) method is invoked, if delta is zero, the user agent
     // must act as if the location.reload() method was called instead."
     RefPtr<Location> location = win->Location();
-    return location->Reload(false, aSubjectPrincipal, aRv);
+    return location->Reload(false, aRv);
   }
 
   RefPtr<ChildSHistory> session_history = GetSessionHistory();
   if (!session_history) {
     aRv.Throw(NS_ERROR_FAILURE);
     return;
   }
 
   bool userActivation =
       win->GetWindowContext()
           ? win->GetWindowContext()->HasValidTransientUserGestureActivation()
           : false;
 
-  CallerType callerType = aSubjectPrincipal.IsSystemPrincipal()
-                              ? CallerType::System
-                              : CallerType::NonSystem;
-
   // Ignore the return value from Go(), since returning errors from Go() can
   // lead to exceptions and a possible leak of history length
   // AsyncGo throws if we hit the location change rate limit.
   if (StaticPrefs::dom_window_history_async()) {
     session_history->AsyncGo(aDelta, /* aRequireUserInteraction = */ false,
-                             userActivation, callerType, aRv);
+                             userActivation, aCallerType, aRv);
   } else {
     session_history->Go(aDelta, /* aRequireUserInteraction = */ false,
                         userActivation, IgnoreErrors());
   }
 }
 
 void nsHistory::Back(CallerType aCallerType, ErrorResult& aRv) {
   nsCOMPtr<nsPIDOMWindowInner> win(do_QueryReferent(mInnerWindow));
--- a/dom/base/nsHistory.h
+++ b/dom/base/nsHistory.h
@@ -40,17 +40,17 @@ class nsHistory final : public nsISuppor
 
   uint32_t GetLength(mozilla::ErrorResult& aRv) const;
   mozilla::dom::ScrollRestoration GetScrollRestoration(
       mozilla::ErrorResult& aRv);
   void SetScrollRestoration(mozilla::dom::ScrollRestoration aMode,
                             mozilla::ErrorResult& aRv);
   void GetState(JSContext* aCx, JS::MutableHandle<JS::Value> aResult,
                 mozilla::ErrorResult& aRv) const;
-  void Go(int32_t aDelta, nsIPrincipal& aSubjectPrincipal,
+  void Go(int32_t aDelta, mozilla::dom::CallerType aCallerType,
           mozilla::ErrorResult& aRv);
   void Back(mozilla::dom::CallerType aCallerType, mozilla::ErrorResult& aRv);
   void Forward(mozilla::dom::CallerType aCallerType, mozilla::ErrorResult& aRv);
   void PushState(JSContext* aCx, JS::Handle<JS::Value> aData,
                  const nsAString& aTitle, const nsAString& aUrl,
                  mozilla::dom::CallerType aCallerType,
                  mozilla::ErrorResult& aRv);
   void ReplaceState(JSContext* aCx, JS::Handle<JS::Value> aData,
--- a/dom/webidl/History.webidl
+++ b/dom/webidl/History.webidl
@@ -16,17 +16,17 @@ enum ScrollRestoration { "auto", "manual
 [Exposed=Window]
 interface History {
   [Throws]
   readonly attribute unsigned long length;
   [Throws]
   attribute ScrollRestoration scrollRestoration;
   [Throws]
   readonly attribute any state;
-  [Throws, NeedsSubjectPrincipal]
+  [Throws, NeedsCallerType]
   void go(optional long delta = 0);
   [Throws, NeedsCallerType]
   void back();
   [Throws, NeedsCallerType]
   void forward();
   [Throws, NeedsCallerType]
   void pushState(any data, DOMString title, optional DOMString? url = null);
   [Throws, NeedsCallerType]