Bug 1744397 - Simplify refresh code: pass around delay as an unsigned int. r=smaug

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

docshell/base/nsDocShell.cpp

@@ -20,6 +20,7 @@
 #include "mozilla/AutoRestore.h"
 #include "mozilla/BasePrincipal.h"
 #include "mozilla/Casting.h"
+#include "mozilla/CheckedInt.h"
 #include "mozilla/Components.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Encoding.h"
@@ -5010,7 +5011,8 @@ void nsDocShell::SetScrollbarPreference(mozilla::ScrollbarPreference aPref) {
 //*****************************************************************************
 
 NS_IMETHODIMP
-nsDocShell::RefreshURI(nsIURI* aURI, nsIPrincipal* aPrincipal, int32_t aDelay) {
+nsDocShell::RefreshURI(nsIURI* aURI, nsIPrincipal* aPrincipal,
+                       uint32_t aDelay) {
   MOZ_ASSERT(!mIsBeingDestroyed);
 
   NS_ENSURE_ARG(aURI);
@@ -5070,7 +5072,7 @@ nsDocShell::RefreshURI(nsIURI* aURI, nsIPrincipal* aPrincipal, int32_t aDelay) {
 
 nsresult nsDocShell::ForceRefreshURIFromTimer(nsIURI* aURI,
                                               nsIPrincipal* aPrincipal,
-                                              int32_t aDelay,
+                                              uint32_t aDelay,
                                               nsITimer* aTimer) {
   MOZ_ASSERT(aTimer, "Must have a timer here");
 
@@ -5093,7 +5095,7 @@ nsresult nsDocShell::ForceRefreshURIFromTimer(nsIURI* aURI,
 
 NS_IMETHODIMP
 nsDocShell::ForceRefreshURI(nsIURI* aURI, nsIPrincipal* aPrincipal,
-                            int32_t aDelay) {
+                            uint32_t aDelay) {
   NS_ENSURE_ARG(aURI);
 
   RefPtr<nsDocShellLoadState> loadState = new nsDocShellLoadState(aURI);
@@ -5219,7 +5221,7 @@ nsresult nsDocShell::SetupRefreshURIFromHeader(nsIURI* aBaseURI,
   MOZ_ASSERT(aPrincipal);
 
   nsAutoCString uriAttrib;
-  int32_t seconds = 0;
+  CheckedInt<uint32_t> seconds(0);
   bool specifiesSeconds = false;
 
   nsACString::const_iterator iter, tokenStart, doneIterating;
@@ -5234,24 +5236,33 @@ nsresult nsDocShell::SetupRefreshURIFromHeader(nsIURI* aBaseURI,
 
   tokenStart = iter;
 
-  // skip leading + and -
-  if (iter != doneIterating && (*iter == '-' || *iter == '+')) {
+  if (iter != doneIterating) {
+    if (*iter == '-') {
+      return NS_ERROR_FAILURE;
+    }
+
+    // skip leading +
+    if (*iter == '+') {
       ++iter;
     }
+  }
 
   // parse number
   while (iter != doneIterating && (*iter >= '0' && *iter <= '9')) {
     seconds = seconds * 10 + (*iter - '0');
+    if (!seconds.isValid()) {
+      return NS_ERROR_FAILURE;
+    }
     specifiesSeconds = true;
     ++iter;
   }
 
-  if (iter != doneIterating) {
-    // if we started with a '-', number is negative
-    if (*tokenStart == '-') {
-      seconds = -seconds;
+  CheckedInt<uint32_t> milliSeconds(seconds * 1000);
+  if (!milliSeconds.isValid()) {
+    return NS_ERROR_FAILURE;
   }
 
+  if (iter != doneIterating) {
     // skip to next ';' or ','
     nsACString::const_iterator iterAfterDigit = iter;
     while (iter != doneIterating && !(*iter == ';' || *iter == ',')) {
@@ -5397,15 +5408,7 @@ nsresult nsDocShell::SetupRefreshURIFromHeader(nsIURI* aBaseURI,
         }
       }
 
-      if (NS_SUCCEEDED(rv)) {
-        // Since we can't travel back in time yet, just pretend
-        // negative numbers do nothing at all.
-        if (seconds < 0) {
-          return NS_ERROR_FAILURE;
-        }
-
-        rv = RefreshURI(uri, aPrincipal, seconds * 1000);
-      }
+      rv = RefreshURI(uri, aPrincipal, milliSeconds.value());
     }
   }
   return rv;

docshell/base/nsDocShell.h

@@ -309,7 +309,7 @@ class nsDocShell final : public nsDocLoader,
   // the timer involved out of mRefreshURIList if it's there.
   // aTimer must not be null.
   nsresult ForceRefreshURIFromTimer(nsIURI* aURI, nsIPrincipal* aPrincipal,
-                                    int32_t aDelay, nsITimer* aTimer);
+                                    uint32_t aDelay, nsITimer* aTimer);
 
   // We need dummy OnLocationChange in some cases to update the UI without
   // updating security info.

docshell/base/nsIRefreshURI.idl

@@ -25,7 +25,7 @@ interface nsIRefreshURI : nsISupports {
       * @param aMillis The number of milliseconds to wait.
       */
     void refreshURI(in nsIURI aURI, in nsIPrincipal aPrincipal,
-                    in long aMillis);
+                    in unsigned long aMillis);
 
     /**
       * Loads a URI immediately as if it were a meta refresh.
@@ -38,7 +38,7 @@ interface nsIRefreshURI : nsISupports {
       *                be delayed if it were not being forced.
       */
     void forceRefreshURI(in nsIURI aURI, in nsIPrincipal aPrincipal,
-                         in long aMillis);
+                         in unsigned long aMillis);
 
     /**
       * Cancels all timer loads.

dom/ipc/BrowserChild.cpp

@@ -3796,7 +3796,7 @@ NS_IMETHODIMP BrowserChild::OnProgressChange64(nsIWebProgress* aWebProgress,
 
 NS_IMETHODIMP BrowserChild::OnRefreshAttempted(nsIWebProgress* aWebProgress,
                                                nsIURI* aRefreshURI,
-                                               int32_t aMillis, bool aSameURI,
+                                               uint32_t aMillis, bool aSameURI,
                                                bool* aOut) {
   NS_ENSURE_ARG_POINTER(aOut);
   *aOut = true;

testing/web-platform/meta/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/parsing.html.ini

@@ -43,13 +43,6 @@
       if debug and (os == "mac"): FAIL
       if not debug and (os == "win") and (version == "6.1.7601"): FAIL
 
-  ["-0; url=foo"]
-    disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1230909
-    expected:
-      if not debug and (os == "mac") and (processor == "x86") and (bits == 32): FAIL
-      if debug and (os == "mac"): FAIL
-      if not debug and (os == "win") and (version == "6.1.7601"): FAIL
-
   ["+1; foo"]
     disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1230909
     expected:
@@ -64,13 +57,6 @@
       if debug and (os == "mac"): FAIL
       if not debug and (os == "win") and (version == "6.1.7601"): FAIL
 
-  ["-0; foo"]
-    disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1230909
-    expected:
-      if not debug and (os == "mac") and (processor == "x86") and (bits == 32): FAIL
-      if debug and (os == "mac"): FAIL
-      if not debug and (os == "win") and (version == "6.1.7601"): FAIL
-
   ["+1"]
     disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1230909
     expected:
@@ -79,14 +65,6 @@
       if not debug and (os == "win") and (version == "6.1.7601"): FAIL
       if (processor == "x86_64") and (bits == 64): FAIL
 
-  ["-1"]
-    disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1230909
-    expected:
-      if not debug and (os == "mac") and (processor == "x86") and (bits == 32): FAIL
-      if debug and (os == "mac"): FAIL
-      if not debug and (os == "win") and (version == "6.1.7601"): FAIL
-      if (processor == "x86_64") and (bits == 64): FAIL
-
   ["+0"]
     disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1230909
     expected:
@@ -95,13 +73,6 @@
       if not debug and (os == "win") and (version == "6.1.7601"): FAIL
       if (processor == "x86_64") and (bits == 64): FAIL
 
-  ["-0"]
-    disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1230909
-    expected:
-      if not debug and (os == "mac") and (processor == "x86") and (bits == 32): FAIL
-      if debug and (os == "mac"): FAIL
-      if not debug and (os == "win") and (version == "6.1.7601"): FAIL
-
   [".9; url=foo"]
     disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1230909
     expected:
@@ -115,9 +86,6 @@
   [",foo"]
     expected: FAIL
 
-  ["-0.1; url=foo"]
-    expected: FAIL
-
   [<meta>: "1\\f"]
     expected: TIMEOUT
 
@@ -172,12 +140,6 @@
   [Refresh header: "+0; url=foo"]
     expected: FAIL
 
-  [<meta>: "-0; url=foo"]
-    expected: FAIL
-
-  [Refresh header: "-0; url=foo"]
-    expected: FAIL
-
   [<meta>: "+1; foo"]
     expected: FAIL
 
@@ -190,42 +152,18 @@
   [Refresh header: "+0; foo"]
     expected: FAIL
 
-  [<meta>: "-0; foo"]
-    expected: FAIL
-
-  [Refresh header: "-0; foo"]
-    expected: FAIL
-
   [<meta>: "+1"]
     expected: FAIL
 
   [Refresh header: "+1"]
     expected: FAIL
 
-  [<meta>: "-1"]
-    expected: FAIL
-
-  [Refresh header: "-1"]
-    expected: FAIL
-
   [<meta>: "+0"]
     expected: FAIL
 
   [Refresh header: "+0"]
     expected: FAIL
 
-  [<meta>: "-0"]
-    expected: FAIL
-
-  [Refresh header: "-0"]
-    expected: FAIL
-
-  [<meta>: "-0.1; url=foo"]
-    expected: FAIL
-
-  [Refresh header: "-0.1; url=foo"]
-    expected: FAIL
-
 
 [parsing.html?11-20]
   expected: TIMEOUT
@@ -268,15 +206,7 @@
     expected: TIMEOUT
 
 
-[parsing.html?111-120]
-  [Refresh header: "-0"]
-    expected: FAIL
-
-
 [parsing.html?91-100]
-  [Refresh header: "-0; url=foo"]
-    expected: FAIL
-
   [<meta>: "+1; foo"]
     expected: FAIL
 
@@ -289,9 +219,6 @@
   [Refresh header: "+0; foo"]
     expected: FAIL
 
-  [<meta>: "-0; foo"]
-    expected: FAIL
-
 
 [parsing.html?61-70]
   [<meta>: "1; url=\\"foo'bar"]
@@ -317,43 +244,20 @@
   [Refresh header: "+0; url=foo"]
     expected: FAIL
 
-  [<meta>: "-0; url=foo"]
-    expected: FAIL
-
-
-[parsing.html?131-last]
-  [<meta>: "-0.1; url=foo"]
-    expected: FAIL
-
-  [Refresh header: "-0.1; url=foo"]
-    expected: FAIL
-
 
 [parsing.html?101-110]
-  [Refresh header: "-0; foo"]
-    expected: FAIL
-
   [<meta>: "+1"]
     expected: FAIL
 
   [Refresh header: "+1"]
     expected: FAIL
 
-  [<meta>: "-1"]
-    expected: FAIL
-
-  [Refresh header: "-1"]
-    expected: FAIL
-
   [<meta>: "+0"]
     expected: FAIL
 
   [Refresh header: "+0"]
     expected: FAIL
 
-  [<meta>: "-0"]
-    expected: FAIL
-
 
 [parsing.html?1-10]
 

toolkit/components/statusfilter/nsBrowserStatusFilter.cpp

@@ -271,7 +271,7 @@ nsBrowserStatusFilter::OnProgressChange64(nsIWebProgress* aWebProgress,
 
 NS_IMETHODIMP
 nsBrowserStatusFilter::OnRefreshAttempted(nsIWebProgress* aWebProgress,
-                                          nsIURI* aUri, int32_t aDelay,
+                                          nsIURI* aUri, uint32_t aDelay,
                                           bool aSameUri, bool* allowRefresh) {
   nsCOMPtr<nsIWebProgressListener2> listener = do_QueryInterface(mListener);
   if (!listener) {

uriloader/base/nsDocLoader.cpp

@@ -1408,7 +1408,7 @@ void nsDocLoader::FireOnStatusChange(nsIWebProgress* aWebProgress,
 }
 
 bool nsDocLoader::RefreshAttempted(nsIWebProgress* aWebProgress, nsIURI* aURI,
-                                   int32_t aDelay, bool aSameURI) {
+                                   uint32_t aDelay, bool aSameURI) {
   /*
    * Returns true if the refresh may proceed,
    * false if the refresh should be blocked.

uriloader/base/nsDocLoader.h

@@ -186,7 +186,7 @@ class nsDocLoader : public nsIDocumentLoader,
                             nsIURI* aUri, uint32_t aFlags);
 
   [[nodiscard]] bool RefreshAttempted(nsIWebProgress* aWebProgress,
-                                      nsIURI* aURI, int32_t aDelay,
+                                      nsIURI* aURI, uint32_t aDelay,
                                       bool aSameURI);
 
   // this function is overridden by the docshell, it is provided so that we

uriloader/base/nsIWebProgressListener2.idl

@@ -64,6 +64,6 @@ interface nsIWebProgressListener2 : nsIWebProgressListener {
    */
   boolean onRefreshAttempted(in nsIWebProgress aWebProgress,
                              in nsIURI aRefreshURI,
-                             in long aMillis,
+                             in unsigned long aMillis,
                              in boolean aSameURI);
 };