From a33a4db25e070f26f287f7f14fec039680faa0fc Mon Sep 17 00:00:00 2001 From: Peter Van der Beken Date: Thu, 16 Dec 2021 22:27:08 +0000 Subject: [PATCH] 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 | 45 +++++---- docshell/base/nsDocShell.h | 2 +- docshell/base/nsIRefreshURI.idl | 4 +- dom/ipc/BrowserChild.cpp | 2 +- .../parsing.html.ini | 96 ------------------- .../statusfilter/nsBrowserStatusFilter.cpp | 2 +- uriloader/base/nsDocLoader.cpp | 2 +- uriloader/base/nsDocLoader.h | 2 +- uriloader/base/nsIWebProgressListener2.idl | 2 +- 9 files changed, 32 insertions(+), 125 deletions(-) diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 06272708fb52..5241275e2e38 100644 --- a/docshell/base/nsDocShell.cpp +++ b/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 loadState = new nsDocShellLoadState(aURI); @@ -5219,7 +5221,7 @@ nsresult nsDocShell::SetupRefreshURIFromHeader(nsIURI* aBaseURI, MOZ_ASSERT(aPrincipal); nsAutoCString uriAttrib; - int32_t seconds = 0; + CheckedInt 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 == '+')) { - ++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 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; diff --git a/docshell/base/nsDocShell.h b/docshell/base/nsDocShell.h index 7d0cdf5a98de..5ed508d6bde5 100644 --- a/docshell/base/nsDocShell.h +++ b/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. diff --git a/docshell/base/nsIRefreshURI.idl b/docshell/base/nsIRefreshURI.idl index 5dab0f38cf97..a4a578a3449c 100644 --- a/docshell/base/nsIRefreshURI.idl +++ b/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. diff --git a/dom/ipc/BrowserChild.cpp b/dom/ipc/BrowserChild.cpp index 44a04f7614f8..65942f7d4837 100644 --- a/dom/ipc/BrowserChild.cpp +++ b/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; diff --git a/testing/web-platform/meta/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/parsing.html.ini b/testing/web-platform/meta/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/parsing.html.ini index 53c11c6237af..41b511b7f4c9 100644 --- a/testing/web-platform/meta/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/parsing.html.ini +++ b/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 - [: "1\\f"] expected: TIMEOUT @@ -172,12 +140,6 @@ [Refresh header: "+0; url=foo"] expected: FAIL - [: "-0; url=foo"] - expected: FAIL - - [Refresh header: "-0; url=foo"] - expected: FAIL - [: "+1; foo"] expected: FAIL @@ -190,42 +152,18 @@ [Refresh header: "+0; foo"] expected: FAIL - [: "-0; foo"] - expected: FAIL - - [Refresh header: "-0; foo"] - expected: FAIL - [: "+1"] expected: FAIL [Refresh header: "+1"] expected: FAIL - [: "-1"] - expected: FAIL - - [Refresh header: "-1"] - expected: FAIL - [: "+0"] expected: FAIL [Refresh header: "+0"] expected: FAIL - [: "-0"] - expected: FAIL - - [Refresh header: "-0"] - expected: FAIL - - [: "-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 - [: "+1; foo"] expected: FAIL @@ -289,9 +219,6 @@ [Refresh header: "+0; foo"] expected: FAIL - [: "-0; foo"] - expected: FAIL - [parsing.html?61-70] [: "1; url=\\"foo'bar"] @@ -317,43 +244,20 @@ [Refresh header: "+0; url=foo"] expected: FAIL - [: "-0; url=foo"] - expected: FAIL - - -[parsing.html?131-last] - [: "-0.1; url=foo"] - expected: FAIL - - [Refresh header: "-0.1; url=foo"] - expected: FAIL - [parsing.html?101-110] - [Refresh header: "-0; foo"] - expected: FAIL - [: "+1"] expected: FAIL [Refresh header: "+1"] expected: FAIL - [: "-1"] - expected: FAIL - - [Refresh header: "-1"] - expected: FAIL - [: "+0"] expected: FAIL [Refresh header: "+0"] expected: FAIL - [: "-0"] - expected: FAIL - [parsing.html?1-10] diff --git a/toolkit/components/statusfilter/nsBrowserStatusFilter.cpp b/toolkit/components/statusfilter/nsBrowserStatusFilter.cpp index 86f9574c8519..d55b12233dc4 100644 --- a/toolkit/components/statusfilter/nsBrowserStatusFilter.cpp +++ b/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 listener = do_QueryInterface(mListener); if (!listener) { diff --git a/uriloader/base/nsDocLoader.cpp b/uriloader/base/nsDocLoader.cpp index 4fd3b29938cb..3a7aee26d515 100644 --- a/uriloader/base/nsDocLoader.cpp +++ b/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. diff --git a/uriloader/base/nsDocLoader.h b/uriloader/base/nsDocLoader.h index caa72ebccf0e..1e28e4f36ddb 100644 --- a/uriloader/base/nsDocLoader.h +++ b/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 diff --git a/uriloader/base/nsIWebProgressListener2.idl b/uriloader/base/nsIWebProgressListener2.idl index 87701f8d2cfe..a40d9538fb00 100644 --- a/uriloader/base/nsIWebProgressListener2.idl +++ b/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); };