From a7054c85a4bca4c9079ad23c02eaa75ee0cc6907 Mon Sep 17 00:00:00 2001 From: athenian200 Date: Mon, 23 Dec 2019 09:09:57 -0600 Subject: [PATCH 01/11] Issue #1332 - Backport promise-based media playback https://bugzilla.mozilla.org/show_bug.cgi?id=1244768 I happened to find an older version of the promise-based media playback patch in Bugzilla, the one that was originally submitted for review. It had the DocShell changes I already knew how to deal with, and had fewer of the audio wrapper and nsISupports changes that were confusing me in the later patch. I was able to do a better job getting this back into a UXP-appropriate configuration than I could have with the final version. I'm honestly still a little unsure about some of the minor refactoring done in the patch itself, insisting on already_AddRefed promises and such, but I don't really know how to avoid those completely. Still, I think it's better than it was. --- dom/html/HTMLMediaElement.cpp | 304 ++++++++++++++++++++++++++--- dom/html/HTMLMediaElement.h | 38 +++- dom/webidl/HTMLMediaElement.webidl | 2 +- 3 files changed, 312 insertions(+), 32 deletions(-) diff --git a/dom/html/HTMLMediaElement.cpp b/dom/html/HTMLMediaElement.cpp index 050d1ac69d..884f65762e 100644 --- a/dom/html/HTMLMediaElement.cpp +++ b/dom/html/HTMLMediaElement.cpp @@ -8,6 +8,7 @@ #include "mozilla/dom/HTMLMediaElementBinding.h" #include "mozilla/dom/HTMLSourceElement.h" #include "mozilla/dom/ElementInlines.h" +#include "mozilla/dom/Promise.h" #include "mozilla/ArrayUtils.h" #include "mozilla/MathAlgorithms.h" #include "mozilla/AsyncEventDispatcher.h" @@ -171,6 +172,22 @@ static const unsigned short MEDIA_ERR_NETWORK = 2; static const unsigned short MEDIA_ERR_DECODE = 3; static const unsigned short MEDIA_ERR_SRC_NOT_SUPPORTED = 4; +static void +ResolvePromisesWithUndefined(const nsTArray>& aPromises) +{ + for (auto& promise : aPromises) { + promise->MaybeResolveWithUndefined(); + } +} + +static void +RejectPromises(const nsTArray>& aPromises, nsresult aError) +{ + for (auto& promise : aPromises) { + promise->MaybeReject(aError); + } +} + // Under certain conditions there may be no-one holding references to // a media element from script, DOM parent, etc, but the element may still // fire meaningful events in the future so we can't destroy it yet: @@ -261,6 +278,75 @@ public: } }; +/* + * If no error is passed while constructing an instance, the instance will + * resolve the passed promises with undefined; otherwise, the instance will + * reject the passed promises with the passed error. + * + * The constructor appends the constructed instance into the passed media + * element's mPendingPlayPromisesRunners member and once the the runner is run + * (whether fulfilled or canceled), it removes itself from + * mPendingPlayPromisesRunners. + */ +class HTMLMediaElement::nsResolveOrRejectPendingPlayPromisesRunner : public nsMediaEvent +{ + nsTArray> mPromises; + nsresult mError; + +public: + nsResolveOrRejectPendingPlayPromisesRunner(HTMLMediaElement* aElement, + nsTArray>&& aPromises, + nsresult aError = NS_OK) + : nsMediaEvent(aElement) + , mPromises(Move(aPromises)) + , mError(aError) + { + mElement->mPendingPlayPromisesRunners.AppendElement(this); + } + + void ResolveOrReject() + { + if (NS_SUCCEEDED(mError)) { + ResolvePromisesWithUndefined(mPromises); + } else { + RejectPromises(mPromises, mError); + } + } + + NS_IMETHOD Run() override + { + if (!IsCancelled()) { + ResolveOrReject(); + } + + mElement->mPendingPlayPromisesRunners.RemoveElement(this); + return NS_OK; + } +}; + +class HTMLMediaElement::nsNotifyAboutPlayingRunner : public nsResolveOrRejectPendingPlayPromisesRunner +{ +public: + nsNotifyAboutPlayingRunner(HTMLMediaElement* aElement, + nsTArray>&& aPendingPlayPromises) + : nsResolveOrRejectPendingPlayPromisesRunner(aElement, + Move(aPendingPlayPromises)) + { + } + + NS_IMETHOD Run() override + { + if (IsCancelled()) { + mElement->mPendingPlayPromisesRunners.RemoveElement(this); + return NS_OK; + } + + mElement->DispatchEvent(NS_LITERAL_STRING("playing")); + return nsResolveOrRejectPendingPlayPromisesRunner::Run(); + } +}; + + class nsSourceErrorEventRunner : public nsMediaEvent { private: @@ -826,6 +912,7 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(HTMLMediaElement, nsGenericHTM NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mMediaKeys) #endif NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mSelectedVideoStreamTrack) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPendingPlayPromises) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(HTMLMediaElement, nsGenericHTMLElement) @@ -853,6 +940,7 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(HTMLMediaElement, nsGenericHTMLE NS_IMPL_CYCLE_COLLECTION_UNLINK(mMediaKeys) #endif NS_IMPL_CYCLE_COLLECTION_UNLINK(mSelectedVideoStreamTrack) + NS_IMPL_CYCLE_COLLECTION_UNLINK(mPendingPlayPromises) NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(HTMLMediaElement) @@ -1044,6 +1132,14 @@ void HTMLMediaElement::AbortExistingLoads() // with a different load ID to silently be cancelled. mCurrentLoadID++; + // Immediately reject or resolve the already-dispatched + // nsResolveOrRejectPendingPlayPromisesRunners. These runners won't be + // executed again later since the mCurrentLoadID had been changed. + for (auto& runner : mPendingPlayPromisesRunners) { + runner->ResolveOrReject(); + } + mPendingPlayPromisesRunners.Clear(); + if (mChannelLoader) { mChannelLoader->Cancel(); mChannelLoader = nullptr; @@ -1107,7 +1203,10 @@ void HTMLMediaElement::AbortExistingLoads() NS_ASSERTION(!mDecoder && !mSrcStream, "How did someone setup a new stream/decoder already?"); // ChangeNetworkState() will call UpdateAudioChannelPlayingState() // indirectly which depends on mPaused. So we need to update mPaused first. - mPaused = true; + if (!mPaused) { + mPaused = true; + RejectPromises(TakePendingPlayPromises(), NS_ERROR_DOM_MEDIA_ABORT_ERR); + } ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_EMPTY); ChangeReadyState(nsIDOMHTMLMediaElement::HAVE_NOTHING); @@ -1149,6 +1248,7 @@ void HTMLMediaElement::NoSupportedMediaSourceError(const nsACString& aErrorDetai mErrorSink->SetError(MEDIA_ERR_SRC_NOT_SUPPORTED, aErrorDetails); ChangeDelayLoadStatus(false); UpdateAudioChannelPlayingState(); + RejectPromises(TakePendingPlayPromises(), NS_ERROR_DOM_MEDIA_NOT_SUPPORTED_ERR); } typedef void (HTMLMediaElement::*SyncSectionFn)(); @@ -1953,14 +2053,7 @@ HTMLMediaElement::Seek(double aTime, // aTime should be non-NaN. MOZ_ASSERT(!mozilla::IsNaN(aTime)); - nsCOMPtr global = do_QueryInterface(OwnerDoc()->GetInnerWindow()); - - if (!global) { - aRv.Throw(NS_ERROR_UNEXPECTED); - return nullptr; - } - - RefPtr promise = Promise::Create(global, aRv); + RefPtr promise = CreateDOMPromise(aRv); if (NS_WARN_IF(aRv.Failed())) { return nullptr; @@ -2208,6 +2301,7 @@ HTMLMediaElement::Pause(ErrorResult& aRv) if (!oldPaused) { FireTimeUpdate(false); DispatchAsyncEvent(NS_LITERAL_STRING("pause")); + AsyncRejectPendingPlayPromises(NS_ERROR_DOM_MEDIA_ABORT_ERR); } } @@ -3200,36 +3294,86 @@ HTMLMediaElement::NotifyXPCOMShutdown() ShutdownDecoder(); } -void +already_AddRefed HTMLMediaElement::Play(ErrorResult& aRv) { if (!IsAllowedToPlay()) { MaybeDoLoad(); - return; + + // A blocked media element will be resumed later, so we return a pending + // promise which might be resolved/rejected depends on the result of + // resuming the blocked media element. + RefPtr promise = CreateDOMPromise(aRv); + + if (NS_WARN_IF(aRv.Failed())) { + return nullptr; + } + + mPendingPlayPromises.AppendElement(promise); + return promise.forget(); } - nsresult rv = PlayInternal(); - if (NS_FAILED(rv)) { - aRv.Throw(rv); - } + RefPtr promise = PlayInternal(aRv); OpenUnsupportedMediaWithExternalAppIfNeeded(); + + return promise.forget(); } -nsresult -HTMLMediaElement::PlayInternal() +already_AddRefed +HTMLMediaElement::PlayInternal(ErrorResult& aRv) { + MOZ_ASSERT(!aRv.Failed()); + + // 4.8.12.8 + // When the play() method on a media element is invoked, the user agent must + // run the following steps. + + // 4.8.12.8 - Step 1: + // If the media element is not allowed to play, return a promise rejected + // with a "NotAllowedError" DOMException and abort these steps. + if (!IsAllowedToPlay()) { + // NOTE: for promise-based-play, will return a rejected promise here. + aRv.Throw(NS_ERROR_DOM_MEDIA_NOT_ALLOWED_ERR); + return nullptr; + } + + // 4.8.12.8 - Step 2: + // If the media element's error attribute is not null and its code + // attribute has the value MEDIA_ERR_SRC_NOT_SUPPORTED, return a promise + // rejected with a "NotSupportedError" DOMException and abort these steps. + if (GetError() && GetError()->Code() == MEDIA_ERR_SRC_NOT_SUPPORTED) { + aRv.Throw(NS_ERROR_DOM_MEDIA_NOT_SUPPORTED_ERR); + return nullptr; + } + + // 4.8.12.8 - Step 3: + // Let promise be a new promise and append promise to the list of pending + // play promises. + RefPtr promise = CreateDOMPromise(aRv); + if (NS_WARN_IF(aRv.Failed())) { + return nullptr; + } + mPendingPlayPromises.AppendElement(promise); + // Play was not blocked so assume user interacted with the element. mHasUserInteraction = true; StopSuspendingAfterFirstFrame(); SetPlayedOrSeeked(true); + // 4.8.12.8 - Step 4: + // If the media element's networkState attribute has the value NETWORK_EMPTY, + // invoke the media element's resource selection algorithm. MaybeDoLoad(); if (mSuspendedForPreloadNone) { ResumeLoad(PRELOAD_ENOUGH); } + // 4.8.12.8 - Step 5: + // If the playback has ended and the direction of playback is forwards, + // seek to the earliest possible position of the media resource. + // Even if we just did Load() or ResumeLoad(), we could already have a decoder // here if we managed to clone an existing decoder. if (mDecoder) { @@ -3239,7 +3383,14 @@ HTMLMediaElement::PlayInternal() if (!mPausedForInactiveDocumentOrChannel) { nsresult rv = mDecoder->Play(); if (NS_FAILED(rv)) { - return rv; + // We don't need to remove the _promise_ from _mPendingPlayPromises_ here. + // If something wrong between |mPendingPlayPromises.AppendElement(promise);| + // and here, the _promise_ should already have been rejected. Otherwise, + // the _promise_ won't be returned to JS at all, so just leave it in the + // _mPendingPlayPromises_ and let it be resolved/rejected with the + // following actions and the promise-resolution won't be observed at all. + aRv.Throw(rv); + return nullptr; } } } @@ -3248,7 +3399,7 @@ HTMLMediaElement::PlayInternal() mCurrentPlayRangeStart = CurrentTime(); } - bool oldPaused = mPaused; + const bool oldPaused = mPaused; mPaused = false; mAutoplaying = false; SetAudioChannelSuspended(nsISuspendedTypes::NONE_SUSPENDED); @@ -3265,8 +3416,27 @@ HTMLMediaElement::PlayInternal() // media, the event would be pending until media is resumed. // TODO: If the playback has ended, then the user agent must set // seek to the effective start. + + // 4.8.12.8 - Step 6: + // If the media element's paused attribute is true, run the following steps: if (oldPaused) { + // 6.1. Change the value of paused to false. (Already done.) + // This step is uplifted because the "block-media-playback" feature needs + // the mPaused to be false before UpdateAudioChannelPlayingState() being + // called. + + // 6.2. If the show poster flag is true, set the element's show poster flag + // to false and run the time marches on steps. + + // 6.3. Queue a task to fire a simple event named play at the element. DispatchAsyncEvent(NS_LITERAL_STRING("play")); + + // 6.4. If the media element's readyState attribute has the value + // HAVE_NOTHING, HAVE_METADATA, or HAVE_CURRENT_DATA, queue a task to + // fire a simple event named waiting at the element. + // Otherwise, the media element's readyState attribute has the value + // HAVE_FUTURE_DATA or HAVE_ENOUGH_DATA: notify about playing for the + // element. switch (mReadyState) { case nsIDOMHTMLMediaElement::HAVE_NOTHING: DispatchAsyncEvent(NS_LITERAL_STRING("waiting")); @@ -3279,12 +3449,20 @@ HTMLMediaElement::PlayInternal() case nsIDOMHTMLMediaElement::HAVE_FUTURE_DATA: case nsIDOMHTMLMediaElement::HAVE_ENOUGH_DATA: FireTimeUpdate(false); - DispatchAsyncEvent(NS_LITERAL_STRING("playing")); + NotifyAboutPlaying(); break; } + } else if (mReadyState >= nsIDOMHTMLMediaElement::HAVE_FUTURE_DATA) { + // 7. Otherwise, if the media element's readyState attribute has the value + // HAVE_FUTURE_DATA or HAVE_ENOUGH_DATA, take pending play promises and + // queue a task to resolve pending play promises with the result. + AsyncResolvePendingPlayPromises(); } - return NS_OK; + // 8. Set the media element's autoplaying flag to false. (Already done.) + + // 9. Return promise. + return promise.forget(); } void @@ -3302,9 +3480,9 @@ NS_IMETHODIMP HTMLMediaElement::Play() return NS_OK; } - nsresult rv = PlayInternal(); - if (NS_FAILED(rv)) { - return rv; + RefPtr toBeIgnored = PlayInternal(rv); + if (rv.Failed()) { + return rv.StealNSResult(); } OpenUnsupportedMediaWithExternalAppIfNeeded(); @@ -4463,7 +4641,12 @@ void HTMLMediaElement::PlaybackEnded() return; } - Pause(); + FireTimeUpdate(false); + + if (!mPaused) { + Pause(); + AsyncRejectPendingPlayPromises(NS_ERROR_DOM_MEDIA_ABORT_ERR); + } if (mSrcStream) { // A MediaStream that goes from inactive to active shall be eligible for @@ -4471,7 +4654,6 @@ void HTMLMediaElement::PlaybackEnded() mAutoplaying = true; } - FireTimeUpdate(false); DispatchAsyncEvent(NS_LITERAL_STRING("ended")); } @@ -4865,7 +5047,7 @@ void HTMLMediaElement::ChangeReadyState(nsMediaReadyState aState) DispatchAsyncEvent(NS_LITERAL_STRING("canplay")); if (!mPaused) { mWaitingForKey = NOT_WAITING_FOR_KEY; - DispatchAsyncEvent(NS_LITERAL_STRING("playing")); + NotifyAboutPlaying(); } } @@ -5111,7 +5293,14 @@ nsresult HTMLMediaElement::DispatchAsyncEvent(const nsAString& aName) return NS_OK; } - nsCOMPtr event = new nsAsyncEventRunner(aName, this); + nsCOMPtr event; + + if (aName.EqualsLiteral("playing")) { + event = new nsNotifyAboutPlayingRunner(this, TakePendingPlayPromises()); + } else { + event = new nsAsyncEventRunner(aName, this); + } + NS_DispatchToMainThread(event); return NS_OK; @@ -6601,5 +6790,64 @@ HTMLMediaElement::MarkAsContentSource(CallerAPI aAPI) } } +nsTArray> +HTMLMediaElement::TakePendingPlayPromises() +{ + return Move(mPendingPlayPromises); +} + +void +HTMLMediaElement::NotifyAboutPlaying() +{ + // Stick to the DispatchAsyncEvent() call path for now because we want to + // trigger some telemetry-related codes in the DispatchAsyncEvent() method. + DispatchAsyncEvent(NS_LITERAL_STRING("playing")); +} + +already_AddRefed +HTMLMediaElement::CreateDOMPromise(ErrorResult& aRv) const +{ + nsCOMPtr global = do_QueryInterface(OwnerDoc()->GetInnerWindow()); + + if (!global) { + aRv.Throw(NS_ERROR_UNEXPECTED); + return nullptr; + } + + return Promise::Create(global, aRv); +} + +void +HTMLMediaElement::AsyncResolvePendingPlayPromises() +{ + if (mShuttingDown) { + return; + } + + nsCOMPtr event + = new nsResolveOrRejectPendingPlayPromisesRunner(this, + TakePendingPlayPromises()); + + NS_DispatchToMainThread(event); +} + +void +HTMLMediaElement::AsyncRejectPendingPlayPromises(nsresult aError) +{ + if (mShuttingDown) { + return; + } + + nsCOMPtr event + = new nsResolveOrRejectPendingPlayPromisesRunner(this, + TakePendingPlayPromises(), + aError); + + NS_DispatchToMainThread(event); + +} + + + } // namespace dom } // namespace mozilla diff --git a/dom/html/HTMLMediaElement.h b/dom/html/HTMLMediaElement.h index 899e8449af..44c666f951 100644 --- a/dom/html/HTMLMediaElement.h +++ b/dom/html/HTMLMediaElement.h @@ -16,7 +16,6 @@ #include "DecoderTraits.h" #include "nsIAudioChannelAgent.h" #include "mozilla/Attributes.h" -#include "mozilla/dom/Promise.h" #include "mozilla/dom/TextTrackManager.h" #include "mozilla/WeakPtr.h" #include "MediaDecoder.h" @@ -78,6 +77,7 @@ namespace dom { class MediaError; class MediaSource; +class Promise; class TextTrackList; class AudioTrackList; class VideoTrackList; @@ -549,7 +549,7 @@ public: SetHTMLBoolAttr(nsGkAtoms::loop, aValue, aRv); } - void Play(ErrorResult& aRv); + already_AddRefed Play(ErrorResult& aRv); void Pause(ErrorResult& aRv); @@ -835,7 +835,7 @@ protected: nsTArray>> mTrackPorts; }; - nsresult PlayInternal(); + already_AddRefed PlayInternal(ErrorResult& aRv); /** Use this method to change the mReadyState member, so required * events can be fired. @@ -981,6 +981,7 @@ protected: void AbortExistingLoads(); /** + * These are the dedicated media source failure steps. * Called when all potential resources are exhausted. Changes network * state to NETWORK_NO_SOURCE, and sends error event with code * MEDIA_ERR_SRC_NOT_SUPPORTED. @@ -1285,6 +1286,8 @@ protected: void MaybeNotifyMediaResumed(SuspendTypes aSuspend); class nsAsyncEventRunner; + class nsNotifyAboutPlayingRunner; + class nsResolveOrRejectPendingPlayPromisesRunner; using nsGenericHTMLElement::DispatchEvent; // For nsAsyncEventRunner. nsresult DispatchEvent(const nsAString& aName); @@ -1293,6 +1296,24 @@ protected: // triggers play() after loaded fail. eg. preload the data before start play. void OpenUnsupportedMediaWithExternalAppIfNeeded() const; + // This method moves the mPendingPlayPromises into a temperate object. So the + // mPendingPlayPromises is cleared after this method call. + nsTArray> TakePendingPlayPromises(); + + // This method snapshots the mPendingPlayPromises by TakePendingPlayPromises() + // and queues a task to resolve them. + void AsyncResolvePendingPlayPromises(); + + // This method snapshots the mPendingPlayPromises by TakePendingPlayPromises() + // and queues a task to reject them. + void AsyncRejectPendingPlayPromises(nsresult aError); + + // This method snapshots the mPendingPlayPromises by TakePendingPlayPromises() + // and queues a task to resolve them also to dispatch a "playing" event. + void NotifyAboutPlaying(); + + already_AddRefed CreateDOMPromise(ErrorResult& aRv) const; + // The current decoder. Load() has been called on this decoder. // At most one of mDecoder and mSrcStream can be non-null. RefPtr mDecoder; @@ -1684,6 +1705,17 @@ private: Visibility mVisibilityState; UniquePtr mErrorSink; + + // A list of pending play promises. The elements are pushed during the play() + // method call and are resolved/rejected during further playback steps. + nsTArray> mPendingPlayPromises; + + // A list of already-dispatched but not yet run + // nsResolveOrRejectPendingPlayPromisesRunners. + // Runners whose Run() method is called remove themselves from this list. + // We keep track of these because the load algorithm resolves/rejects all + // already-dispatched pending play promises. + nsTArray mPendingPlayPromisesRunners; }; } // namespace dom diff --git a/dom/webidl/HTMLMediaElement.webidl b/dom/webidl/HTMLMediaElement.webidl index 313686dacb..be79d8679c 100644 --- a/dom/webidl/HTMLMediaElement.webidl +++ b/dom/webidl/HTMLMediaElement.webidl @@ -68,7 +68,7 @@ interface HTMLMediaElement : HTMLElement { [SetterThrows] attribute boolean loop; [Throws] - void play(); + Promise play(); [Throws] void pause(); From 42659b995efcf79f52e53c77029dbf25b2bf7272 Mon Sep 17 00:00:00 2001 From: athenian200 Date: Tue, 24 Dec 2019 06:53:25 -0600 Subject: [PATCH 02/11] Fix typo. --- dom/html/HTMLMediaElement.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/dom/html/HTMLMediaElement.cpp b/dom/html/HTMLMediaElement.cpp index 884f65762e..f3ef20bfc7 100644 --- a/dom/html/HTMLMediaElement.cpp +++ b/dom/html/HTMLMediaElement.cpp @@ -3480,6 +3480,7 @@ NS_IMETHODIMP HTMLMediaElement::Play() return NS_OK; } + ErrorResult rv; RefPtr toBeIgnored = PlayInternal(rv); if (rv.Failed()) { return rv.StealNSResult(); From c6694a0c4ca0726115a24a26e38871b56c2e9618 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Wed, 31 Jul 2019 14:18:54 +0000 Subject: [PATCH 03/11] Issue #1118 Part 1: Split out part of nsDocShell::AddState into a separate method. This implements the "URL and history update steps" from the HTML spec. See https://html.spec.whatwg.org/multipage/history.html --- docshell/base/nsDocShell.cpp | 138 +++++++++++++++++++++------------- docshell/base/nsIDocShell.idl | 26 +++++++ 2 files changed, 112 insertions(+), 52 deletions(-) diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index d67780317c..d4d86b83a4 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -11843,35 +11843,45 @@ nsDocShell::AddState(JS::Handle aData, const nsAString& aTitle, { // Implements History.pushState and History.replaceState - // Here's what we do, roughly in the order specified by HTML5: - // 1. Serialize aData using structured clone. - // 2. If the third argument is present, - // a. Resolve the url, relative to the first script's base URL - // b. If (a) fails, raise a SECURITY_ERR - // c. Compare the resulting absolute URL to the document's address. If - // any part of the URLs difer other than the , , and - // components, raise a SECURITY_ERR and abort. - // 3. If !aReplace: + // Here's what we do, roughly in the order specified by HTML5. The specific + // steps we are executing are at + // + // and + // . + // This function basically implements #dom-history-pushstate and + // UpdateURLAndHistory implements #url-and-history-update-steps. + // + // A. Serialize aData using structured clone. This is #dom-history-pushstate + // step 5. + // B. If the third argument is present, #dom-history-pushstate step 7. + // 7.1. Resolve the url, relative to our document. + // 7.2. If (a) fails, raise a SECURITY_ERR + // 7.4. Compare the resulting absolute URL to the document's address. If + // any part of the URLs difer other than the , , and + // components, raise a SECURITY_ERR and abort. + // C. If !aReplace, #url-and-history-update-steps steps 2.1-2.3: // Remove from the session history all entries after the current entry, // as we would after a regular navigation, and save the current // entry's scroll position (bug 590573). - // 4. As apropriate, either add a state object entry to the session history - // after the current entry with the following properties, or modify the - // current session history entry to set + // D. #url-and-history-update-steps step 2.4 or step 3. As apropriate, + // either add a state object entry to the session history after the + // current entry with the following properties, or modify the current + // session history entry to set // a. cloned data as the state object, // b. if the third argument was present, the absolute URL found in // step 2 // Also clear the new history entry's POST data (see bug 580069). - // 5. If aReplace is false (i.e. we're doing a pushState instead of a + // E. If aReplace is false (i.e. we're doing a pushState instead of a // replaceState), notify bfcache that we've navigated to a new page. - // 6. If the third argument is present, set the document's current address - // to the absolute URL found in step 2. + // F. If the third argument is present, set the document's current address + // to the absolute URL found in step B. This is + // #url-and-history-update-steps step 4. // // It's important that this function not run arbitrary scripts after step 1 // and before completing step 5. For example, if a script called // history.back() before we completed step 5, bfcache might destroy an // active content viewer. Since EvictOutOfRangeContentViewers at the end of - // step 5 might run script, we can't just put a script blocker around the + // step E might run script, we can't just put a script blocker around the // critical section. // // Note that we completely ignore the aTitle parameter. @@ -11891,7 +11901,9 @@ nsDocShell::AddState(JS::Handle aData, const nsAString& aTitle, nsCOMPtr document = GetDocument(); NS_ENSURE_TRUE(document, NS_ERROR_FAILURE); - // Step 1: Serialize aData using structured clone. + // Step A: Serialize aData using structured clone. + // https://html.spec.whatwg.org/multipage/history.html#dom-history-pushstate + // step 5. nsCOMPtr scContainer; // scContainer->Init might cause arbitrary JS to run, and this code might @@ -11934,7 +11946,9 @@ nsDocShell::AddState(JS::Handle aData, const nsAString& aTitle, NS_ENSURE_TRUE(scSize <= (uint32_t)maxStateObjSize, NS_ERROR_ILLEGAL_VALUE); - // Step 2: Resolve aURL + // Step B: Resolve aURL. + // https://html.spec.whatwg.org/multipage/history.html#dom-history-pushstate + // step 7. bool equalURIs = true; nsCOMPtr currentURI; if (sURIFixup && mCurrentURI) { @@ -11943,12 +11957,11 @@ nsDocShell::AddState(JS::Handle aData, const nsAString& aTitle, } else { currentURI = mCurrentURI; } - nsCOMPtr oldURI = currentURI; nsCOMPtr newURI; if (aURL.Length() == 0) { newURI = currentURI; } else { - // 2a: Resolve aURL relative to mURI + // 7.1: Resolve aURL relative to mURI nsIURI* docBaseURI = document->GetDocBaseURI(); if (!docBaseURI) { @@ -11964,12 +11977,12 @@ nsDocShell::AddState(JS::Handle aData, const nsAString& aTitle, rv = NS_NewURI(getter_AddRefs(newURI), aURL, charset.get(), docBaseURI); - // 2b: If 2a fails, raise a SECURITY_ERR + // 7.2: If 7.1 fails, raise a SECURITY_ERR if (NS_FAILED(rv)) { return NS_ERROR_DOM_SECURITY_ERR; } - // 2c: Same-origin check. + // 7.4 and 7.5: Same-origin check. if (!nsContentUtils::URIIsLocalFile(newURI)) { // In addition to checking that the security manager says that // the new URI has the same origin as our current URI, we also @@ -12020,10 +12033,27 @@ nsDocShell::AddState(JS::Handle aData, const nsAString& aTitle, } // end of same-origin check - // Step 3: Create a new entry in the session history. This will erase - // all SHEntries after the new entry and make this entry the current - // one. This operation may modify mOSHE, which we need later, so we - // keep a reference here. + // Step 8: call "URL and history update steps" + rv = UpdateURLAndHistory(document, newURI, scContainer, aTitle, aReplace, + currentURI, equalURIs); + NS_ENSURE_SUCCESS(rv, rv); + + return NS_OK; +} + +nsresult +nsDocShell::UpdateURLAndHistory(nsIDocument* aDocument, nsIURI* aNewURI, + nsIStructuredCloneContainer* aData, + const nsAString& aTitle, bool aReplace, + nsIURI* aCurrentURI, bool aEqualURIs) +{ + // Implements + // https://html.spec.whatwg.org/multipage/history.html#url-and-history-update-steps + + // Step 2, if aReplace is false: Create a new entry in the session + // history. This will erase all SHEntries after the new entry and make this + // entry the current one. This operation may modify mOSHE, which we need + // later, so we keep a reference here. NS_ENSURE_TRUE(mOSHE, NS_ERROR_FAILURE); nsCOMPtr oldOSHE = mOSHE; @@ -12031,7 +12061,8 @@ nsDocShell::AddState(JS::Handle aData, const nsAString& aTitle, nsCOMPtr newSHEntry; if (!aReplace) { - // Save the current scroll position (bug 590573). + // Step 2. + // Save the current scroll position (bug 590573). Step 2.3. nscoord cx = 0, cy = 0; GetCurScrollPos(ScrollOrientation_X, &cx); GetCurScrollPos(ScrollOrientation_Y, &cy); @@ -12042,10 +12073,10 @@ nsDocShell::AddState(JS::Handle aData, const nsAString& aTitle, // Since we're not changing which page we have loaded, pass // true for aCloneChildren. - rv = AddToSessionHistory(newURI, nullptr, - document->NodePrincipal(), // triggeringPrincipal - nullptr, true, - getter_AddRefs(newSHEntry)); + nsresult rv = AddToSessionHistory(aNewURI, nullptr, + aDocument->NodePrincipal(), // triggeringPrincipal + nullptr, true, + getter_AddRefs(newSHEntry)); NS_ENSURE_SUCCESS(rv, rv); NS_ENSURE_TRUE(newSHEntry, NS_ERROR_FAILURE); @@ -12068,15 +12099,16 @@ nsDocShell::AddState(JS::Handle aData, const nsAString& aTitle, mOSHE = newSHEntry; } else { + // Step 3. newSHEntry = mOSHE; - newSHEntry->SetURI(newURI); - newSHEntry->SetOriginalURI(newURI); + newSHEntry->SetURI(aNewURI); + newSHEntry->SetOriginalURI(aNewURI); newSHEntry->SetLoadReplace(false); } - // Step 4: Modify new/original session history entry and clear its POST - // data, if there is any. - newSHEntry->SetStateData(scContainer); + // Step 2.4 and 3: Modify new/original session history entry and clear its + // POST data, if there is any. + newSHEntry->SetStateData(aData); newSHEntry->SetPostData(nullptr); // If this push/replaceState changed the document's current URI and the new @@ -12084,22 +12116,24 @@ nsDocShell::AddState(JS::Handle aData, const nsAString& aTitle, // SHEntry's URI was modified in this way by a push/replaceState call // set URIWasModified to true for the current SHEntry (bug 669671). bool sameExceptHashes = true, oldURIWasModified = false; - newURI->EqualsExceptRef(currentURI, &sameExceptHashes); + aNewURI->EqualsExceptRef(aCurrentURI, &sameExceptHashes); oldOSHE->GetURIWasModified(&oldURIWasModified); newSHEntry->SetURIWasModified(!sameExceptHashes || oldURIWasModified); - // Step 5: If aReplace is false, indicating that we're doing a pushState - // rather than a replaceState, notify bfcache that we've added a page to - // the history so it can evict content viewers if appropriate. Otherwise - // call ReplaceEntry so that we notify nsIHistoryListeners that an entry - // was replaced. + // Step E as described at the top of AddState: If aReplace is false, + // indicating that we're doing a pushState rather than a replaceState, notify + // bfcache that we've added a page to the history so it can evict content + // viewers if appropriate. Otherwise call ReplaceEntry so that we notify + // nsIHistoryListeners that an entry was replaced. nsCOMPtr rootSH; GetRootSessionHistory(getter_AddRefs(rootSH)); NS_ENSURE_TRUE(rootSH, NS_ERROR_UNEXPECTED); nsCOMPtr internalSH = do_QueryInterface(rootSH); NS_ENSURE_TRUE(internalSH, NS_ERROR_UNEXPECTED); - + + nsresult rv; + if (!aReplace) { int32_t curIndex = -1; rv = rootSH->GetIndex(&curIndex); @@ -12116,7 +12150,7 @@ nsDocShell::AddState(JS::Handle aData, const nsAString& aTitle, } } - // Step 6: If the document's URI changed, update document's URI and update + // Step 4: If the document's URI changed, update document's URI and update // global history. // // We need to call FireOnLocationChange so that the browser's address bar @@ -12129,35 +12163,35 @@ nsDocShell::AddState(JS::Handle aData, const nsAString& aTitle, // notification is allowed only when we know docshell is not loading a new // document and it requires LOCATION_CHANGE_SAME_DOCUMENT flag. Otherwise, // FireOnLocationChange(...) breaks security UI. - if (!equalURIs) { - document->SetDocumentURI(newURI); + if (!aEqualURIs) { + aDocument->SetDocumentURI(aNewURI); // We can't trust SetCurrentURI to do always fire locationchange events // when we expect it to, so we hack around that by doing it ourselves... - SetCurrentURI(newURI, nullptr, false, LOCATION_CHANGE_SAME_DOCUMENT); + SetCurrentURI(aNewURI, nullptr, false, LOCATION_CHANGE_SAME_DOCUMENT); if (mLoadType != LOAD_ERROR_PAGE) { FireDummyOnLocationChange(); } - AddURIVisit(newURI, oldURI, oldURI, 0); + AddURIVisit(aNewURI, aCurrentURI, aCurrentURI, 0); // AddURIVisit doesn't set the title for the new URI in global history, // so do that here. if (mUseGlobalHistory && !UsePrivateBrowsing()) { nsCOMPtr history = services::GetHistoryService(); if (history) { - history->SetURITitle(newURI, mTitle); + history->SetURITitle(aNewURI, mTitle); } else if (mGlobalHistory) { - mGlobalHistory->SetPageTitle(newURI, mTitle); + mGlobalHistory->SetPageTitle(aNewURI, mTitle); } } // Inform the favicon service that our old favicon applies to this new // URI. - CopyFavicon(oldURI, newURI, document->NodePrincipal(), UsePrivateBrowsing()); + CopyFavicon(aCurrentURI, aNewURI, aDocument->NodePrincipal(), UsePrivateBrowsing()); } else { FireDummyOnLocationChange(); } - document->SetStateObject(scContainer); + aDocument->SetStateObject(aData); return NS_OK; } diff --git a/docshell/base/nsIDocShell.idl b/docshell/base/nsIDocShell.idl index d205e5b0cf..d2812bd9c9 100644 --- a/docshell/base/nsIDocShell.idl +++ b/docshell/base/nsIDocShell.idl @@ -26,6 +26,7 @@ interface nsIChannel; interface nsIContentViewer; interface nsIDOMEventTarget; interface nsIDocShellLoadInfo; +interface nsIDocument; interface nsIEditor; interface nsIEditingSession; interface nsISimpleEnumerator; @@ -35,6 +36,7 @@ interface nsISHEntry; interface nsILayoutHistoryState; interface nsISecureBrowserUI; interface nsIScriptGlobalObject; +interface nsIStructuredCloneContainer; interface nsIDOMStorage; interface nsIPrincipal; interface nsIWebBrowserPrint; @@ -210,6 +212,30 @@ interface nsIDocShell : nsIDocShellTreeItem void addState(in jsval aData, in DOMString aTitle, in DOMString aURL, in boolean aReplace); + /** + * Helper for addState and document.open that does just the + * history-manipulation guts. + * + * Arguments the spec defines: + * + * @param aDocument the document we're manipulating. This will get the new URI. + * @param aNewURI the new URI. + * @param aData The serialized state data. May be null. + * @param aTitle The new title. May be empty. + * @param aReplace whether this should replace the exising SHEntry. + * + * Arguments we need internally because deriving them from the + * others is a bit complicated: + * + * @param aCurrentURI the current URI we're working with. Might be null. + * @param aEqualURIs whether the two URIs involved are equal. + */ + [nostdcall] + void updateURLAndHistory(in nsIDocument aDocument, in nsIURI aNewURI, + in nsIStructuredCloneContainer aData, in AString aTitle, + in boolean aReplace, in nsIURI aCurrentURI, + in boolean aEqualURIs); + /** * Creates a DocShellLoadInfo object that you can manipulate and then pass * to loadURI. From 7d52521f959bc4b60a417128f60f2fe572fc5518 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Tue, 6 Aug 2019 08:41:51 +0000 Subject: [PATCH 04/11] Issue #1118 Part 2: Allow UpdateURLAndHistory to work even if mOSHE is null, if we're doing a replace. We're going to end up hitting this if someone does a document.open() before mOSHE has been set. We shouldn't need to worry about mLSHE, because the document.open() will cancel the corresponding load. --- docshell/base/nsDocShell.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index d4d86b83a4..2aea698477 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -12054,7 +12054,7 @@ nsDocShell::UpdateURLAndHistory(nsIDocument* aDocument, nsIURI* aNewURI, // history. This will erase all SHEntries after the new entry and make this // entry the current one. This operation may modify mOSHE, which we need // later, so we keep a reference here. - NS_ENSURE_TRUE(mOSHE, NS_ERROR_FAILURE); + NS_ENSURE_TRUE(mOSHE || aReplace, NS_ERROR_FAILURE); nsCOMPtr oldOSHE = mOSHE; mLoadType = LOAD_PUSHSTATE; @@ -12101,6 +12101,16 @@ nsDocShell::UpdateURLAndHistory(nsIDocument* aDocument, nsIURI* aNewURI, } else { // Step 3. newSHEntry = mOSHE; + + // Since we're not changing which page we have loaded, pass + if (!newSHEntry) { + nsresult rv = AddToSessionHistory( + aNewURI, nullptr, + aDocument->NodePrincipal(), // triggeringPrincipal + nullptr, true, getter_AddRefs(newSHEntry)); + NS_ENSURE_SUCCESS(rv, rv); + mOSHE = newSHEntry; + } newSHEntry->SetURI(aNewURI); newSHEntry->SetOriginalURI(aNewURI); newSHEntry->SetLoadReplace(false); @@ -12117,7 +12127,10 @@ nsDocShell::UpdateURLAndHistory(nsIDocument* aDocument, nsIURI* aNewURI, // set URIWasModified to true for the current SHEntry (bug 669671). bool sameExceptHashes = true, oldURIWasModified = false; aNewURI->EqualsExceptRef(aCurrentURI, &sameExceptHashes); - oldOSHE->GetURIWasModified(&oldURIWasModified); + // mOSHE might be null on replace. Only check if we're not replacing. + if (oldOSHE) { + oldOSHE->GetURIWasModified(&oldURIWasModified); + } newSHEntry->SetURIWasModified(!sameExceptHashes || oldURIWasModified); // Step E as described at the top of AddState: If aReplace is false, From 08c26a9adf07c67bc7b194df7a45f91119b17ca3 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Tue, 6 Aug 2019 08:52:49 +0000 Subject: [PATCH 05/11] Issue #1118 Part 3: Add a public RemoveAllListeners method on EventListenerManager. --- dom/events/EventListenerManager.cpp | 21 ++++++++++++++++++--- dom/events/EventListenerManager.h | 8 +++++++- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/dom/events/EventListenerManager.cpp b/dom/events/EventListenerManager.cpp index fe896870c9..0774c32966 100644 --- a/dom/events/EventListenerManager.cpp +++ b/dom/events/EventListenerManager.cpp @@ -166,11 +166,11 @@ EventListenerManager::~EventListenerManager() // XXX azakai: Is there any reason to not just call Disconnect // from right here, if not previously called? NS_ASSERTION(!mTarget, "didn't call Disconnect"); - RemoveAllListeners(); + RemoveAllListenersSilently(); } void -EventListenerManager::RemoveAllListeners() +EventListenerManager::RemoveAllListenersSilently() { if (mClearingListeners) { return; @@ -1329,7 +1329,7 @@ void EventListenerManager::Disconnect() { mTarget = nullptr; - RemoveAllListeners(); + RemoveAllListenersSilently(); } void @@ -1734,6 +1734,21 @@ EventListenerManager::IsApzAwareEvent(nsIAtom* aEvent) return false; } +void +EventListenerManager::RemoveAllListeners() +{ + while (!mListeners.IsEmpty()) { + size_t idx = mListeners.Length() - 1; + nsCOMPtr type = mListeners.ElementAt(idx).mTypeAtom; + EventMessage message = mListeners.ElementAt(idx).mEventMessage; + mListeners.RemoveElementAt(idx); + NotifyEventListenerRemoved(type); + if (IsDeviceType(message)) { + DisableDevice(message); + } + } +} + already_AddRefed EventListenerManager::GetScriptGlobalAndDocument(nsIDocument** aDoc) { diff --git a/dom/events/EventListenerManager.h b/dom/events/EventListenerManager.h index 6b09277881..36637cfd76 100644 --- a/dom/events/EventListenerManager.h +++ b/dom/events/EventListenerManager.h @@ -472,6 +472,12 @@ public: bool IsApzAwareListener(Listener* aListener); bool IsApzAwareEvent(nsIAtom* aEvent); + /** + * Remove all event listeners from the event target this EventListenerManager + * is for. + */ + void RemoveAllListeners(); + protected: void HandleEventInternal(nsPresContext* aPresContext, WidgetEvent* aEvent, @@ -604,7 +610,7 @@ protected: const nsAString& aTypeString, const EventListenerFlags& aFlags, bool aAllEvents = false); - void RemoveAllListeners(); + void RemoveAllListenersSilently(); void NotifyEventListenerRemoved(nsIAtom* aUserType); const EventTypeData* GetTypeDataForIID(const nsIID& aIID); const EventTypeData* GetTypeDataForEventName(nsIAtom* aName); From ab6131d1e6e51f2d9cb74471e428415e872f72ca Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Tue, 6 Aug 2019 09:58:35 +0000 Subject: [PATCH 06/11] Issue #1118 Part 4: Allow UpdateURLAndHistory to work even if there's no root session history. This can happen when someone does a document.open() on a document that has no session history. --- docshell/base/nsDocShell.cpp | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 2aea698477..ebaf07bcdf 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -12137,7 +12137,9 @@ nsDocShell::UpdateURLAndHistory(nsIDocument* aDocument, nsIURI* aNewURI, // indicating that we're doing a pushState rather than a replaceState, notify // bfcache that we've added a page to the history so it can evict content // viewers if appropriate. Otherwise call ReplaceEntry so that we notify - // nsIHistoryListeners that an entry was replaced. + // nsIHistoryListeners that an entry was replaced. We may not have a root + // session history if this call is coming from a document.open() in a docshell + // subtree that disables session history. nsCOMPtr rootSH; GetRootSessionHistory(getter_AddRefs(rootSH)); NS_ENSURE_TRUE(rootSH, NS_ERROR_UNEXPECTED); @@ -12147,19 +12149,21 @@ nsDocShell::UpdateURLAndHistory(nsIDocument* aDocument, nsIURI* aNewURI, nsresult rv; - if (!aReplace) { - int32_t curIndex = -1; - rv = rootSH->GetIndex(&curIndex); - if (NS_SUCCEEDED(rv) && curIndex > -1) { - internalSH->EvictOutOfRangeContentViewers(curIndex); - } - } else { - nsCOMPtr rootSHEntry = GetRootSHEntry(newSHEntry); + if (rootSH) { + if (!aReplace) { + int32_t curIndex = -1; + rv = rootSH->GetIndex(&curIndex); + if (NS_SUCCEEDED(rv) && curIndex > -1) { + internalSH->EvictOutOfRangeContentViewers(curIndex); + } + } else { + nsCOMPtr rootSHEntry = GetRootSHEntry(newSHEntry); - int32_t index = -1; - rv = rootSH->GetIndexOfEntry(rootSHEntry, &index); - if (NS_SUCCEEDED(rv) && index > -1) { - internalSH->ReplaceEntry(index, rootSHEntry); + int32_t index = -1; + rv = rootSH->GetIndexOfEntry(rootSHEntry, &index); + if (NS_SUCCEEDED(rv) && index > -1) { + internalSH->ReplaceEntry(index, rootSHEntry); + } } } From fc5d61608cd831aa52a274ec0918f930366fb39f Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Sun, 22 Dec 2019 19:57:30 +0100 Subject: [PATCH 07/11] Issue #1118 - Part 5: Change the way document.open() works This changes the work we do for document.open() in the following ways: - We no longer create a new Window when doing document.open(). We use the same Window but remove all the event listeners on the existing DOM tree and Window before removing the document's existing children to provide a clean slate document to use for .write(). - We no longer create a session history entry (previously would be a wyciwyg URI). We now replace the current one, effectively losing the entry for the original document. - We now support document.open() on windowless documents. --- dom/base/SimpleTreeIterator.h | 71 ++++++ dom/base/moz.build | 1 + dom/base/nsDocument.cpp | 50 ++-- dom/base/nsDocument.h | 33 ++- dom/base/nsIDocument.h | 15 +- dom/html/nsHTMLDocument.cpp | 405 +++++++++++-------------------- layout/base/nsDocumentViewer.cpp | 5 + uriloader/base/nsDocLoader.cpp | 102 +++++++- uriloader/base/nsDocLoader.h | 23 +- 9 files changed, 404 insertions(+), 301 deletions(-) create mode 100644 dom/base/SimpleTreeIterator.h diff --git a/dom/base/SimpleTreeIterator.h b/dom/base/SimpleTreeIterator.h new file mode 100644 index 0000000000..7ca5040828 --- /dev/null +++ b/dom/base/SimpleTreeIterator.h @@ -0,0 +1,71 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +/** + * This is a light-weight tree iterator for `for` loops when full iterator + * functionality isn't required. + */ + +#ifndef mozilla_dom_SimpleTreeIterator_h +#define mozilla_dom_SimpleTreeIterator_h + +#include "nsINode.h" +#include "nsTArray.h" +#include "mozilla/dom/Element.h" + +namespace mozilla { +namespace dom { + +class SimpleTreeIterator { +public: + /** + * Initialize an iterator with aRoot. After that it can be iterated with a + * range-based for loop. At the moment, that's the only supported form of use + * for this iterator. + */ + explicit SimpleTreeIterator(nsINode& aRoot) + : mCurrent(&aRoot) + { + mTree.AppendElement(&aRoot); + } + + // Basic support for range-based for loops. + // This will modify the iterator as it goes. + SimpleTreeIterator& begin() { return *this; } + + SimpleTreeIterator end() { return SimpleTreeIterator(); } + + bool operator!=(const SimpleTreeIterator& aOther) { + return mCurrent != aOther.mCurrent; + } + + void operator++() { Next(); } + + nsINode* operator*() { return mCurrent; } + +private: + // Constructor used only for end() to represent a drained iterator. + SimpleTreeIterator() + : mCurrent(nullptr) + {} + + void Next() { + MOZ_ASSERT(mCurrent, "Don't call Next() when we have no current node"); + + mCurrent = mCurrent->GetNextNode(mTree.LastElement()); + } + + // The current node. + nsINode* mCurrent; + + // The DOM tree that we're inside of right now. + AutoTArray mTree; +}; + +} // namespace dom +} // namespace mozilla + +#endif // mozilla_dom_SimpleTreeIterator_h diff --git a/dom/base/moz.build b/dom/base/moz.build index ebb76d6174..75ddefdedd 100755 --- a/dom/base/moz.build +++ b/dom/base/moz.build @@ -210,6 +210,7 @@ EXPORTS.mozilla.dom += [ 'ScreenOrientation.h', 'ScriptSettings.h', 'ShadowRoot.h', + 'SimpleTreeIterator.h', 'StructuredCloneHolder.h', 'StructuredCloneTags.h', 'StyleSheetList.h', diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index e2be6b6645..0f663636f3 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -1968,22 +1968,10 @@ nsDocument::Reset(nsIChannel* aChannel, nsILoadGroup* aLoadGroup) } void -nsDocument::ResetToURI(nsIURI *aURI, nsILoadGroup *aLoadGroup, - nsIPrincipal* aPrincipal) -{ - NS_PRECONDITION(aURI, "Null URI passed to ResetToURI"); - - if (gDocumentLeakPRLog && MOZ_LOG_TEST(gDocumentLeakPRLog, LogLevel::Debug)) { - PR_LogPrint("DOCUMENT %p ResetToURI %s", this, - aURI->GetSpecOrDefault().get()); - } - - mSecurityInfo = nullptr; - - mDocumentLoadGroup = nullptr; - +nsDocument::DisconnectNodeTree() { // Delete references to sub-documents and kill the subdocument map, - // if any. It holds strong references + // if any. This is not strictly needed, but makes the node tree + // teardown a bit faster. delete mSubDocuments; mSubDocuments = nullptr; @@ -2019,6 +2007,22 @@ nsDocument::ResetToURI(nsIURI *aURI, nsILoadGroup *aLoadGroup, "After removing all children, there should be no root elem"); } mInUnlinkOrDeletion = oldVal; +} + +void +nsDocument::ResetToURI(nsIURI *aURI, nsILoadGroup *aLoadGroup, + nsIPrincipal* aPrincipal) +{ + NS_PRECONDITION(aURI, "Null URI passed to ResetToURI"); + + if (gDocumentLeakPRLog && MOZ_LOG_TEST(gDocumentLeakPRLog, LogLevel::Debug)) { + PR_LogPrint("DOCUMENT %p ResetToURI %s", this, + aURI->GetSpecOrDefault().get()); + } + + mSecurityInfo = nullptr; + + mDocumentLoadGroup = nullptr; // Reset our stylesheets ResetStylesheetsToURI(aURI); @@ -2029,6 +2033,8 @@ nsDocument::ResetToURI(nsIURI *aURI, nsILoadGroup *aLoadGroup, mListenerManager = nullptr; } + DisconnectNodeTree(); + // Release the stylesheets list. mDOMStyleSheets = nullptr; @@ -9077,7 +9083,8 @@ nsDocument::CloneDocHelper(nsDocument* clone) const } void -nsDocument::SetReadyStateInternal(ReadyState rs) +nsDocument::SetReadyStateInternal(ReadyState rs, + bool updateTimingInformation) { mReadyState = rs; if (rs == READYSTATE_UNINITIALIZED) { @@ -9086,7 +9093,12 @@ nsDocument::SetReadyStateInternal(ReadyState rs) // transition undetectable by Web content. return; } - if (mTiming) { + + if (updateTimingInformation && READYSTATE_LOADING == rs) { + mLoadingTimeStamp = mozilla::TimeStamp::Now(); + } + + if (updateTimingInformation && mTiming) { switch (rs) { case READYSTATE_LOADING: mTiming->NotifyDOMLoading(nsIDocument::GetDocumentURI()); @@ -9102,10 +9114,6 @@ nsDocument::SetReadyStateInternal(ReadyState rs) break; } } - // At the time of loading start, we don't have timing object, record time. - if (READYSTATE_LOADING == rs) { - mLoadingTimeStamp = mozilla::TimeStamp::Now(); - } RefPtr asyncDispatcher = new AsyncEventDispatcher(this, NS_LITERAL_STRING("readystatechange"), diff --git a/dom/base/nsDocument.h b/dom/base/nsDocument.h index ac600eb433..c2b1d3cdbd 100644 --- a/dom/base/nsDocument.h +++ b/dom/base/nsDocument.h @@ -704,7 +704,11 @@ public: virtual void BeginLoad() override; virtual void EndLoad() override; - virtual void SetReadyStateInternal(ReadyState rs) override; + // Set the readystate of the document. If updateTimingInformation is true, + // this will record relevant timestamps in the document's performance timing. + // Some consumers like document.open() don't want to do that. + virtual void SetReadyStateInternal(ReadyState rs, + bool updateTimingInformation = true) override; virtual void ContentStateChanged(nsIContent* aContent, mozilla::EventStates aStateMask) @@ -916,6 +920,14 @@ public: UpdateFrameRequestCallbackSchedulingState(); } + void SetLoadEventFiring(bool aFiring) override { mLoadEventFiring = aFiring; } + + bool SkipLoadEventAfterClose() override { + bool skip = mSkipLoadEventAfterClose; + mSkipLoadEventAfterClose = false; + return skip; + } + virtual nsIDocument* GetTemplateContentsOwner() override; NS_DECL_CYCLE_COLLECTION_SKIPPABLE_SCRIPT_HOLDER_CLASS_AMBIGUOUS(nsDocument, @@ -1255,6 +1267,11 @@ protected: */ Element* GetTitleElement(); + /** + * Perform tree disconnection needed by ResetToURI and document.open() + */ + void DisconnectNodeTree(); + public: // Get our title virtual void GetTitle(nsString& aTitle) override; @@ -1458,6 +1475,20 @@ public: // additional sheets and sheets from the nsStyleSheetService. bool mStyleSetFilled:1; + // The HTML spec has a "iframe load in progress" flag, but that doesn't seem + // to have the right semantics. See . + // What we have instead is a flag that is set while the window's 'load' event is + // firing if this document is the window's document. + bool mLoadEventFiring : 1; + + // The HTML spec has a "mute iframe load" flag, but that doesn't seem to have + // the right semantics. See . + // What we have instead is a flag that is set if completion of our document + // via document.close() should skip firing the load event. Note that this + // flag is only relevant for HTML documents, but lives here for reasons that + // are documented above on SkipLoadEventAfterClose(). + bool mSkipLoadEventAfterClose : 1; + uint8_t mPendingFullscreenRequests; uint8_t mXMLDeclarationBits; diff --git a/dom/base/nsIDocument.h b/dom/base/nsIDocument.h index d76a12d713..e2b34f7e38 100644 --- a/dom/base/nsIDocument.h +++ b/dom/base/nsIDocument.h @@ -1448,7 +1448,7 @@ public: virtual void EndLoad() = 0; enum ReadyState { READYSTATE_UNINITIALIZED = 0, READYSTATE_LOADING = 1, READYSTATE_INTERACTIVE = 3, READYSTATE_COMPLETE = 4}; - virtual void SetReadyStateInternal(ReadyState rs) = 0; + virtual void SetReadyStateInternal(ReadyState rs, bool updateTimingInformation = true) = 0; ReadyState GetReadyStateEnum() { return mReadyState; @@ -2186,6 +2186,19 @@ public: mAllowXULXBL = eTriTrue; } + /** + * Flag whether we're about to fire the window's load event for this document. + */ + virtual void SetLoadEventFiring(bool aFiring) = 0; + + /** + * Test whether we should be firing a load event for this document after a + * document.close(). + * This method should only be called at the point when the load event is about + * to be fired, since it resets `skip`. + */ + virtual bool SkipLoadEventAfterClose() = 0; + /** * Returns the template content owner document that owns the content of * HTMLTemplateElement. diff --git a/dom/html/nsHTMLDocument.cpp b/dom/html/nsHTMLDocument.cpp index d64c277271..112e957cb7 100644 --- a/dom/html/nsHTMLDocument.cpp +++ b/dom/html/nsHTMLDocument.cpp @@ -15,6 +15,7 @@ #include "nsPrintfCString.h" #include "nsReadableUtils.h" #include "nsUnicharUtils.h" +#include "nsIDocumentLoader.h" #include "nsIHTMLContentSink.h" #include "nsIXMLContentSink.h" #include "nsHTMLParts.h" @@ -84,6 +85,7 @@ #include "mozilla/dom/EncodingUtils.h" #include "mozilla/dom/FallbackEncoding.h" +#include "mozilla/EventListenerManager.h" #include "mozilla/LoadInfo.h" #include "nsIEditingSession.h" #include "nsIEditor.h" @@ -107,12 +109,14 @@ #include "nsIImageDocument.h" #include "mozilla/dom/HTMLBodyElement.h" #include "mozilla/dom/HTMLDocumentBinding.h" +#include "mozilla/dom/SimpleTreeIterator.h" #include "nsCharsetSource.h" #include "nsIStringBundle.h" #include "nsDOMClassInfo.h" #include "nsFocusManager.h" #include "nsIFrame.h" #include "nsIContent.h" +#include "nsIStructuredCloneContainer.h" #include "nsLayoutStylesheetCache.h" #include "mozilla/StyleSheet.h" #include "mozilla/StyleSheetInlines.h" @@ -842,6 +846,24 @@ nsHTMLDocument::EndLoad() if (turnOnEditing) { EditingStateChanged(); } + + if (!GetWindow()) { + // This is a document that's not in a window. For example, this could be an + // XMLHttpRequest responseXML document, or a document created via DOMParser + // or DOMImplementation. We don't reach this code normally for such + // documents (which is not obviously correct), but can reach it via + // document.open()/document.close(). + // + // Such documents don't fire load events, but per spec should set their + // readyState to "complete" when parsing and all loading of subresources is + // done. Parsing is done now, and documents not in a window don't load + // subresources, so just go ahead and mark ourselves as complete. + SetReadyStateInternal(nsIDocument::READYSTATE_COMPLETE, + /* updateTimingInformation = */ false); + + // Reset mSkipLoadEventAfterClose just in case. + mSkipLoadEventAfterClose = false; + } } void @@ -1410,19 +1432,21 @@ already_AddRefed nsHTMLDocument::Open(JSContext* cx, const nsAString& aType, const nsAString& aReplace, - ErrorResult& rv) + ErrorResult& aError) { - // Implements the "When called with two arguments (or fewer)" steps here: - // https://html.spec.whatwg.org/multipage/webappapis.html#opening-the-input-stream + // Implements + // NS_ASSERTION(nsContentUtils::CanCallerAccess(static_cast(this)), "XOW should have caught this!"); + + // Step 1 - Throw if we're the wrong type of document. if (!IsHTMLDocument() || mDisableDocWrite || !IsMasterDocument()) { - // No calling document.open() on XHTML - rv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR); + aError.Throw(NS_ERROR_DOM_INVALID_STATE_ERR); return nullptr; } + // Set up the content type for insertion nsAutoCString contentType; contentType.AssignLiteral("text/html"); @@ -1435,51 +1459,7 @@ nsHTMLDocument::Open(JSContext* cx, contentType.AssignLiteral("text/plain"); } - // If we already have a parser we ignore the document.open call. - if (mParser || mParserAborted) { - // The WHATWG spec says: "If the document has an active parser that isn't - // a script-created parser, and the insertion point associated with that - // parser's input stream is not undefined (that is, it does point to - // somewhere in the input stream), then the method does nothing. Abort - // these steps and return the Document object on which the method was - // invoked." - // Note that aborting a parser leaves the parser "active" with its - // insertion point "not undefined". We track this using mParserAborted, - // because aborting a parser nulls out mParser. - nsCOMPtr ret = this; - return ret.forget(); - } - - // No calling document.open() without a script global object - if (!mScriptGlobalObject) { - nsCOMPtr ret = this; - return ret.forget(); - } - - nsPIDOMWindowOuter* outer = GetWindow(); - if (!outer || (GetInnerWindow() != outer->GetCurrentInnerWindow())) { - nsCOMPtr ret = this; - return ret.forget(); - } - - // check whether we're in the middle of unload. If so, ignore this call. - nsCOMPtr shell(mDocumentContainer); - if (!shell) { - // We won't be able to create a parser anyway. - nsCOMPtr ret = this; - return ret.forget(); - } - - bool inUnload; - shell->GetIsInUnload(&inUnload); - if (inUnload) { - nsCOMPtr ret = this; - return ret.forget(); - } - - // Note: We want to use GetEntryDocument here because this document - // should inherit the security information of the document that's opening us, - // (since if it's secure, then it's presumably trusted). + // Step 3 - Get the entryDocument for security checks nsCOMPtr callerDoc = GetEntryDocument(); if (!callerDoc) { // If we're called from C++ or in some other way without an originating @@ -1489,67 +1469,39 @@ nsHTMLDocument::Open(JSContext* cx, // change the principals of a document for security reasons we'll have to // refuse to go ahead with this call. - rv.Throw(NS_ERROR_DOM_SECURITY_ERR); + aError.Throw(NS_ERROR_DOM_SECURITY_ERR); return nullptr; } - // Grab a reference to the calling documents security info (if any) - // and URIs as they may be lost in the call to Reset(). - nsCOMPtr securityInfo = callerDoc->GetSecurityInfo(); - nsCOMPtr uri = callerDoc->GetDocumentURI(); - nsCOMPtr baseURI = callerDoc->GetBaseURI(); - nsCOMPtr callerPrincipal = callerDoc->NodePrincipal(); - nsCOMPtr callerChannel = callerDoc->GetChannel(); - - // We're called from script. Make sure the script is from the same - // origin, not just that the caller can access the document. This is - // needed to keep document principals from ever changing, which is - // needed because of the way we use our XOW code, and is a sane - // thing to do anyways. - - bool equals = false; - if (NS_FAILED(callerPrincipal->Equals(NodePrincipal(), &equals)) || - !equals) { - -#ifdef DEBUG - nsCOMPtr callerDocURI = callerDoc->GetDocumentURI(); - nsCOMPtr thisURI = nsIDocument::GetDocumentURI(); - printf("nsHTMLDocument::Open callerDoc %s this %s\n", - callerDocURI ? callerDocURI->GetSpecOrDefault().get() : "", - thisURI ? thisURI->GetSpecOrDefault().get() : ""); -#endif - - rv.Throw(NS_ERROR_DOM_SECURITY_ERR); + // Step 4 - Throw if we're not same-origin + if (!callerDoc->NodePrincipal()->Equals(NodePrincipal())) { + aError.Throw(NS_ERROR_DOM_SECURITY_ERR); return nullptr; } - // Stop current loads targeted at the window this document is in. - if (mScriptGlobalObject) { - nsCOMPtr cv; - shell->GetContentViewer(getter_AddRefs(cv)); - - if (cv) { - bool okToUnload; - if (NS_SUCCEEDED(cv->PermitUnload(&okToUnload)) && !okToUnload) { - // We don't want to unload, so stop here, but don't throw an - // exception. - nsCOMPtr ret = this; - return ret.forget(); - } - - // Now double-check that our invariants still hold. - if (!mScriptGlobalObject) { - nsCOMPtr ret = this; - return ret.forget(); - } - - nsPIDOMWindowOuter* outer = GetWindow(); - if (!outer || (GetInnerWindow() != outer->GetCurrentInnerWindow())) { - nsCOMPtr ret = this; - return ret.forget(); - } + // Step 5 - If we have an active parser, abort with no-op + if (mParser || mParserAborted) { + nsCOMPtr ret = this; + return ret.forget(); + } + + // Step 6 - Check if document.open() is called during unload + nsCOMPtr shell(mDocumentContainer); + if (shell) { + bool inUnload; + shell->GetIsInUnload(&inUnload); + if (inUnload) { + nsCOMPtr ret = this; + return ret.forget(); } + } + // Step 7 - Stop existing navigation of our browsing context (and all + // other loads it's doing) if we're the active document of our browsing + // context. If there's no existing navigation, we don't want to stop + // anything. + if (shell && IsCurrentActiveDocument() && + mScriptGlobalObject) { nsCOMPtr webnav(do_QueryInterface(shell)); webnav->Stop(nsIWebNavigation::STOP_NETWORK); @@ -1560,189 +1512,123 @@ nsHTMLDocument::Open(JSContext* cx, EnsureOnloadBlocker(); } - // The open occurred after the document finished loading. - // So we reset the document and then reinitialize it. - nsCOMPtr channel; - nsCOMPtr group = do_QueryReferent(mDocumentLoadGroup); - rv = NS_NewChannel(getter_AddRefs(channel), - uri, - callerDoc, - nsILoadInfo::SEC_FORCE_INHERIT_PRINCIPAL, - nsIContentPolicy::TYPE_OTHER, - group); - - if (rv.Failed()) { - return nullptr; - } - - if (callerChannel) { - nsLoadFlags callerLoadFlags; - rv = callerChannel->GetLoadFlags(&callerLoadFlags); - if (rv.Failed()) { - return nullptr; - } - - nsLoadFlags loadFlags; - rv = channel->GetLoadFlags(&loadFlags); - if (rv.Failed()) { - return nullptr; - } - - loadFlags |= callerLoadFlags & nsIRequest::INHIBIT_PERSISTENT_CACHING; - - rv = channel->SetLoadFlags(loadFlags); - if (rv.Failed()) { - return nullptr; - } - - // If the user has allowed mixed content on the rootDoc, then we should propogate it - // down to the new document channel. - bool rootHasSecureConnection = false; - bool allowMixedContent = false; - bool isDocShellRoot = false; - nsresult rvalue = shell->GetAllowMixedContentAndConnectionData(&rootHasSecureConnection, &allowMixedContent, &isDocShellRoot); - if (NS_SUCCEEDED(rvalue) && allowMixedContent && isDocShellRoot) { - shell->SetMixedContentChannel(channel); + // Step 8 - Clear all event listeners out of our DOM tree + for (nsINode* node : SimpleTreeIterator(*this)) { + if (EventListenerManager* elm = node->GetExistingListenerManager()) { + elm->RemoveAllListeners(); } } - // Before we reset the doc notify the globalwindow of the change, - // but only if we still have a window (i.e. our window object the - // current inner window in our outer window). - - // Hold onto ourselves on the offchance that we're down to one ref - nsCOMPtr kungFuDeathGrip = this; - - if (nsPIDOMWindowInner *window = GetInnerWindow()) { - // Remember the old scope in case the call to SetNewDocument changes it. - nsCOMPtr oldScope(do_QueryReferent(mScopeObject)); - -#ifdef DEBUG - bool willReparent = mWillReparent; - mWillReparent = true; - - nsDocument* templateContentsOwner = - static_cast(mTemplateContentsOwner.get()); - - if (templateContentsOwner) { - templateContentsOwner->mWillReparent = true; + // Step 9 - Clear event listeners from our window, if we have one. + // + // Note that we explicitly want the inner window, and only if we're its + // document. We want to do this (per spec) even when we're not the "active + // document", so we can't go through GetWindow(), because it might forward to + // the wrong inner. + if (nsPIDOMWindowInner* win = GetInnerWindow()) { + if (win->GetExtantDoc() == this) { + if (EventListenerManager* elm = + nsGlobalWindow::Cast(win)->GetExistingListenerManager()) { + elm->RemoveAllListeners(); + } } -#endif + } - // Per spec, we pass false here so that a new Window is created. - rv = window->SetNewDocument(this, nullptr, - /* aForceReuseInnerWindow */ false); - if (rv.Failed()) { + // Step 10 - Remove all of our DOM children without firing any mutation events. + DisconnectNodeTree(); + + // --- At this point our tree is clean and we can switch to the new URI --- + + // Step 11 - If we're the current document in our docshell, do the + // equivalent of pushState() with the new URL we should have. + if (shell && IsCurrentActiveDocument()) { + nsCOMPtr newURI = callerDoc->GetDocumentURI(); + + // UpdateURLAndHistory might do various member-setting, so make sure we're + // holding strong refs to all the refcounted args on the stack. We can + // assume that our caller is holding on to "this" already. + nsCOMPtr currentURI = nsIDocument::GetDocumentURI(); + bool equalURIs; + nsresult rv = currentURI->Equals(newURI, &equalURIs); + if (NS_WARN_IF(NS_FAILED(rv))) { + aError.Throw(rv); + return nullptr; + } + nsCOMPtr stateContainer(mStateObjectContainer); + rv = shell->UpdateURLAndHistory(this, newURI, stateContainer, EmptyString(), + /* aReplace = */ true, currentURI, + equalURIs); + if (NS_WARN_IF(NS_FAILED(rv))) { + aError.Throw(rv); return nullptr; } -#ifdef DEBUG - if (templateContentsOwner) { - templateContentsOwner->mWillReparent = willReparent; - } + // And use the security info of the caller document as well, since + // it's the thing providing our data. + mSecurityInfo = callerDoc->GetSecurityInfo(); - mWillReparent = willReparent; -#endif - - // Now make sure we're not flagged as the initial document anymore, now - // that we've had stuff done to us. From now on, if anyone tries to - // document.open() us, they get a new inner window. + // This is not mentioned in the spec, but that's probably a spec bug. + // See . + // Since our URL may be changing away from about:blank here, we really want + // to unset this flag on any document.open(), since only about:blank can be + // an initial document. SetIsInitialDocument(false); - nsCOMPtr newScope(do_QueryReferent(mScopeObject)); - JS::Rooted wrapper(cx, GetWrapper()); - if (oldScope && newScope != oldScope && wrapper) { - JSAutoCompartment ac(cx, wrapper); - rv = mozilla::dom::ReparentWrapper(cx, wrapper); - if (rv.Failed()) { - return nullptr; - } + // And let our docloader know that it will need to track our load event. + nsDocShell::Cast(shell)->SetDocumentOpenedButNotLoaded(); + } - // Also reparent the template contents owner document - // because its global is set to the same as this document. - if (mTemplateContentsOwner) { - JS::Rooted contentsOwnerWrapper(cx, - mTemplateContentsOwner->GetWrapper()); - if (contentsOwnerWrapper) { - rv = mozilla::dom::ReparentWrapper(cx, contentsOwnerWrapper); - if (rv.Failed()) { - return nullptr; - } - } - } - } - } + // Step 12 + mSkipLoadEventAfterClose = mLoadEventFiring; + + // Preliminary to steps 13-16. Set our ready state to uninitialized before + // we do anything else, so we can then proceed to later ready state levels. + SetReadyStateInternal(READYSTATE_UNINITIALIZED, + /* updateTimingInformation = */ false); mDidDocumentOpen = true; - // Call Reset(), this will now do the full reset - Reset(channel, group); - if (baseURI) { - mDocumentBaseURI = baseURI; - } - - // Store the security info of the caller now that we're done - // resetting the document. - mSecurityInfo = securityInfo; + // Step 13 - Set our compatibility mode to standards. + SetCompatibilityMode(eCompatibility_FullStandards); + // Step 14 - Create a new parser associated with document. + // This also does step 16 implicitly. mParserAborted = false; mParser = nsHtml5Module::NewHtml5Parser(); - nsHtml5Module::Initialize(mParser, this, uri, shell, channel); + nsHtml5Module::Initialize(mParser, this, nsIDocument::GetDocumentURI(), shell, nullptr); if (mReferrerPolicySet) { // CSP may have set the referrer policy, so a speculative parser should // start with the new referrer policy. nsHtml5TreeOpExecutor* executor = nullptr; - executor = static_cast (mParser->GetContentSink()); + executor = static_cast(mParser->GetContentSink()); if (executor && mReferrerPolicySet) { - executor->SetSpeculationReferrerPolicy(static_cast(mReferrerPolicy)); + executor->SetSpeculationReferrerPolicy( + static_cast(mReferrerPolicy)); } } - // This will be propagated to the parser when someone actually calls write() - SetContentTypeInternal(contentType); + if (shell) { + // Prepare the docshell and the document viewer for the impending + // out-of-band document.write() + shell->PrepareForNewContentModel(); - // Prepare the docshell and the document viewer for the impending - // out of band document.write() - shell->PrepareForNewContentModel(); - - // Now check whether we were opened with a "replace" argument. If - // so, we need to tell the docshell to not create a new history - // entry for this load. Otherwise, make sure that we're doing a normal load, - // not whatever type of load was previously done on this docshell. - shell->SetLoadType(aReplace.LowerCaseEqualsLiteral("replace") ? - LOAD_NORMAL_REPLACE : LOAD_NORMAL); - - nsCOMPtr cv; - shell->GetContentViewer(getter_AddRefs(cv)); - if (cv) { - cv->LoadStart(this); + nsCOMPtr cv; + shell->GetContentViewer(getter_AddRefs(cv)); + if (cv) { + cv->LoadStart(this); + } } - // Add a wyciwyg channel request into the document load group - NS_ASSERTION(!mWyciwygChannel, "nsHTMLDocument::Open(): wyciwyg " - "channel already exists!"); + // Step 15. + SetReadyStateInternal(nsIDocument::READYSTATE_LOADING, + /* updateTimingInformation = */ false); - // In case the editor is listening and will see the new channel - // being added, make sure mWriteLevel is non-zero so that the editor - // knows that document.open/write/close() is being called on this - // document. - ++mWriteLevel; + // Step 16 happened with step 14 above. - CreateAndAddWyciwygChannel(); - - --mWriteLevel; - - SetReadyStateInternal(nsIDocument::READYSTATE_LOADING); - - // After changing everything around, make sure that the principal on the - // document's compartment exactly matches NodePrincipal(). - DebugOnly wrapper = GetWrapperPreserveColor(); - MOZ_ASSERT_IF(wrapper, - JS_GetCompartmentPrincipals(js::GetObjectCompartment(wrapper)) == - nsJSPrincipals::get(NodePrincipal())); - - return kungFuDeathGrip.forget(); -} + // Step 17. + nsCOMPtr ret = this; + return ret.forget(); +} NS_IMETHODIMP nsHTMLDocument::Clear() @@ -1806,15 +1692,6 @@ nsHTMLDocument::Close(ErrorResult& rv) if (GetShell()) { FlushPendingNotifications(Flush_Layout); } - - // Removing the wyciwygChannel here is wrong when document.close() is - // called from within the document itself. However, legacy requires the - // channel to be removed here. Otherwise, the load event never fires. - NS_ASSERTION(mWyciwygChannel, "nsHTMLDocument::Close(): Trying to remove " - "nonexistent wyciwyg channel!"); - RemoveWyciwygChannel(); - NS_ASSERTION(!mWyciwygChannel, "nsHTMLDocument::Close(): " - "nsIWyciwygChannel could not be removed!"); } void diff --git a/layout/base/nsDocumentViewer.cpp b/layout/base/nsDocumentViewer.cpp index 5478c61b00..8baf1a464b 100644 --- a/layout/base/nsDocumentViewer.cpp +++ b/layout/base/nsDocumentViewer.cpp @@ -998,6 +998,9 @@ nsDocumentViewer::LoadComplete(nsresult aStatus) // will depend on whether it's cached! if(window && (NS_SUCCEEDED(aStatus) || aStatus == NS_ERROR_PARSED_DATA_CACHED)) { + // If this code changes, the code in nsDocLoader::DocLoaderIsEmpty + // that fires load events for document.open() cases might need to + // be updated too. nsEventStatus status = nsEventStatus_eIgnore; WidgetEvent event(true, eLoad); event.mFlags.mBubbles = false; @@ -1063,7 +1066,9 @@ nsDocumentViewer::LoadComplete(nsresult aStatus) MakeUnique("document::Load")); } + d->SetLoadEventFiring(true); EventDispatcher::Dispatch(window, mPresContext, &event, nullptr, &status); + d->SetLoadEventFiring(false); if (timing) { timing->NotifyLoadEventEnd(); } diff --git a/uriloader/base/nsDocLoader.cpp b/uriloader/base/nsDocLoader.cpp index 69885b93f8..a4beb48278 100644 --- a/uriloader/base/nsDocLoader.cpp +++ b/uriloader/base/nsDocLoader.cpp @@ -4,6 +4,8 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "nspr.h" +#include "mozilla/BasicEvents.h" +#include "mozilla/EventDispatcher.h" #include "mozilla/Logging.h" #include "nsDocLoader.h" @@ -23,6 +25,7 @@ #include "nsQueryObject.h" #include "nsIDOMWindow.h" +#include "nsPIDOMWindow.h" #include "nsIStringBundle.h" #include "nsIScriptSecurityManager.h" @@ -36,7 +39,10 @@ #include "nsIAsyncVerifyRedirectCallback.h" using mozilla::DebugOnly; +using mozilla::eLoad; +using mozilla::EventDispatcher; using mozilla::LogLevel; +using mozilla::WidgetEvent; static NS_DEFINE_CID(kThisImplCID, NS_THIS_DOCLOADER_IMPL_CID); @@ -114,7 +120,8 @@ nsDocLoader::nsDocLoader() mIsLoadingDocument(false), mIsRestoringDocument(false), mDontFlushLayout(false), - mIsFlushingLayout(false) + mIsFlushingLayout(false), + mDocumentOpenedButNotLoaded(false) { ClearInternalProgress(); @@ -289,7 +296,7 @@ nsDocLoader::IsBusy() } /* Is this document loader busy? */ - if (!mIsLoadingDocument) { + if (!IsBlockingLoadEvent()) { return false; } @@ -414,6 +421,7 @@ nsDocLoader::OnStartRequest(nsIRequest *request, nsISupports *aCtxt) if (!mIsLoadingDocument && (loadFlags & nsIChannel::LOAD_DOCUMENT_URI)) { bJustStartedLoading = true; mIsLoadingDocument = true; + mDocumentOpenedButNotLoaded = false; ClearInternalProgress(); // only clear our progress if we are starting a new load.... } @@ -480,10 +488,11 @@ nsDocLoader::OnStopRequest(nsIRequest *aRequest, mLoadGroup->GetActiveCount(&count); MOZ_LOG(gDocLoaderLog, LogLevel::Debug, - ("DocLoader:%p: OnStopRequest[%p](%s) status=%x mIsLoadingDocument=%s, %u active URLs", + ("DocLoader:%p: OnStopRequest[%p](%s) status=%x" + "mIsLoadingDocument=%s, mDocumentOpenedButNotLoaded=%s, %u active URLs", this, aRequest, name.get(), aStatus, (mIsLoadingDocument ? "true" : "false"), - count)); + (mDocumentOpenedButNotLoaded ? "true" : "false"), count)); } bool bFireTransferring = false; @@ -598,10 +607,9 @@ nsDocLoader::OnStopRequest(nsIRequest *aRequest, RemoveRequestInfo(aRequest); // - // Only fire the DocLoaderIsEmpty(...) if the document loader has initiated a - // load. This will handle removing the request from our hashtable as needed. + // Only fire the DocLoaderIsEmpty(...) if we may need to fire onload. // - if (mIsLoadingDocument) { + if (IsBlockingLoadEvent()) { nsCOMPtr ds = do_QueryInterface(static_cast(this)); bool doNotFlushLayout = false; if (ds) { @@ -647,7 +655,7 @@ NS_IMETHODIMP nsDocLoader::GetDocumentChannel(nsIChannel ** aChannel) void nsDocLoader::DocLoaderIsEmpty(bool aFlushLayout) { - if (mIsLoadingDocument) { + if (IsBlockingLoadEvent()) { /* In the unimagineably rude circumstance that onload event handlers triggered by this function actually kill the window ... ok, it's not unimagineable; it's happened ... this deathgrip keeps this object @@ -660,7 +668,9 @@ void nsDocLoader::DocLoaderIsEmpty(bool aFlushLayout) } NS_ASSERTION(!mIsFlushingLayout, "Someone screwed up"); - NS_ASSERTION(mDocumentRequest, "No Document Request!"); + // We may not have a document request if we are in a document.open() situation. + NS_ASSERTION(mDocumentRequest || mDocumentOpenedButNotLoaded, + "No Document Request!"); // The load group for this DocumentLoader is idle. Flush if we need to. if (aFlushLayout && !mDontFlushLayout) { @@ -687,9 +697,14 @@ void nsDocLoader::DocLoaderIsEmpty(bool aFlushLayout) // And now check whether we're really busy; that might have changed with // the layout flush. - // Note, mDocumentRequest can be null if the flushing above re-entered this - // method. - if (!IsBusy() && mDocumentRequest) { + // + // Note, mDocumentRequest can be null while mDocumentOpenedButNotLoaded is + // false if the flushing above re-entered this method. Exit in that case. + if (IsBusy() || (!mDocumentRequest && !mDocumentOpenedButNotLoaded)) { + return; + } + + if (mDocumentRequest) { // Clear out our request info hash, now that our load really is done and // we don't need it anymore to CalculateMaxProgress(). ClearInternalProgress(); @@ -715,7 +730,7 @@ void nsDocLoader::DocLoaderIsEmpty(bool aFlushLayout) // mLoadGroup->SetDefaultLoadRequest(nullptr); - // Take a ref to our parent now so that we can call DocLoaderIsEmpty() on + // Take a ref to our parent now so that we can call ChildDoneWithOnload() on // it even if our onload handler removes us from the docloader tree. RefPtr parent = mParent; @@ -733,6 +748,67 @@ void nsDocLoader::DocLoaderIsEmpty(bool aFlushLayout) parent->ChildDoneWithOnload(this); } } + } else { + MOZ_ASSERT(mDocumentOpenedButNotLoaded); + mDocumentOpenedButNotLoaded = false; + + // Make sure we do the ChildEnteringOnload/ChildDoneWithOnload even if we + // plan to skip firing our own load event, because otherwise we might + // never end up firing our parent's load event. + RefPtr parent = mParent; + if (!parent || parent->ChildEnteringOnload(this)) { + nsresult loadGroupStatus = NS_OK; + mLoadGroup->GetStatus(&loadGroupStatus); + // Make sure we're not canceling the loadgroup. If we are, then we should + // not fire a load event just like in the normal navigation case. + if (NS_SUCCEEDED(loadGroupStatus) || + loadGroupStatus == NS_ERROR_PARSED_DATA_CACHED) { + // Can "doc" or "window" ever come back null here? Our state machine + // is complicated enough, so I wouldn't bet against it... + nsCOMPtr doc = do_GetInterface(GetAsSupports(this)); + if (doc) { + doc->SetReadyStateInternal(nsIDocument::READYSTATE_COMPLETE, + /* updateTimingInformation = */ false); + + nsCOMPtr window = doc->GetWindow(); + if (window && !doc->SkipLoadEventAfterClose()) { + MOZ_LOG(gDocLoaderLog, LogLevel::Debug, + ("DocLoader:%p: Firing load event for document.open\n", + this)); + + // This is a very cut-down version of + // nsDocumentViewer::LoadComplete that doesn't do various things + // that are not relevant here because this wasn't an actual + // navigation. + WidgetEvent event(true, eLoad); + event.mFlags.mBubbles = false; + event.mFlags.mCancelable = false; + // Dispatching to |window|, but using |document| as the target, + // per spec. + event.mTarget = doc; + nsEventStatus unused = nsEventStatus_eIgnore; + doc->SetLoadEventFiring(true); + EventDispatcher::Dispatch(window, nullptr, &event, nullptr, + &unused); + doc->SetLoadEventFiring(false); + + // Now unsuppress painting on the presshell, if we + // haven't done that yet. + nsCOMPtr shell = doc->GetShell(); + if (shell && !shell->IsDestroying()) { + shell->UnsuppressPainting(); + + if (!shell->IsDestroying()) { + shell->LoadComplete(); + } + } + } + } + } + if (parent) { + parent->ChildDoneWithOnload(this); + } + } } } } diff --git a/uriloader/base/nsDocLoader.h b/uriloader/base/nsDocLoader.h index 481b1397bc..b469b8e079 100644 --- a/uriloader/base/nsDocLoader.h +++ b/uriloader/base/nsDocLoader.h @@ -109,6 +109,8 @@ public: unsigned long mNotifyMask; }; + void SetDocumentOpenedButNotLoaded() { mDocumentOpenedButNotLoaded = true; } + protected: virtual ~nsDocLoader(); @@ -302,6 +304,15 @@ protected: bool mIsFlushingLayout; private: + /** + * This flag indicates that the loader is waiting for completion of + * a document.open-triggered "document load". This is set when + * document.open() happens and sets up a new parser and cleared out + * when we go to fire our load event or end up with a new document + * channel. + */ + bool mDocumentOpenedButNotLoaded; + static const PLDHashTableOps sRequestInfoHashOps; // A list of kids that are in the middle of their onload calls and will let @@ -324,10 +335,20 @@ private: nsRequestInfo *GetRequestInfo(nsIRequest* aRequest); void ClearRequestInfoHash(); int64_t CalculateMaxProgress(); -/// void DumpChannelInfo(void); + /// void DumpChannelInfo(void); // used to clear our internal progress state between loads... void ClearInternalProgress(); + + /** + * Used to test whether we might need to fire a load event. This + * can happen when we have a document load going on, or when we've + * had document.open() called and haven't fired the corresponding + * load event yet. + */ + bool IsBlockingLoadEvent() const { + return mIsLoadingDocument || mDocumentOpenedButNotLoaded; + } }; NS_DEFINE_STATIC_IID_ACCESSOR(nsDocLoader, NS_THIS_DOCLOADER_IMPL_CID) From b753aec16498c0ba5beab0648b99b4d04681ce53 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Sun, 22 Dec 2019 23:48:40 +0100 Subject: [PATCH 08/11] Issue #1118 - Part 6: Fix various tests that are no longer correct. The behavior change of document.open() requires these tests to be changed to account for the new spec behavior. --- .../test/navigation/file_bug1379762-2.html | 43 ------------ .../navigation/file_document_write_1.html | 23 ++----- docshell/test/navigation/mochitest.ini | 4 +- .../test/navigation/test_sessionhistory.html | 10 --- dom/base/test/test_x-frame-options.html | 26 ++++--- dom/html/test/mochitest.ini | 1 - dom/html/test/test_bug172261.html | 67 ------------------- dom/html/test/test_bug255820.html | 38 ++--------- dom/tests/mochitest/bugs/test_bug346659.html | 2 +- js/xpconnect/crashtests/crashtests.list | 2 +- layout/style/test/test_bug1232829.html | 1 - .../tests/mochitest/test_bug715739.html | 34 +++++++--- .../mixedcontent/test_bug329869.html | 2 +- ...erse_the_history_write_after_load_1-1.html | 5 +- ...averse_the_history_write_after_load_1.html | 5 +- .../reload_document_open_write-1.html | 7 +- .../reload_document_open_write.html | 6 +- 17 files changed, 71 insertions(+), 205 deletions(-) delete mode 100644 docshell/test/navigation/file_bug1379762-2.html delete mode 100644 dom/html/test/test_bug172261.html diff --git a/docshell/test/navigation/file_bug1379762-2.html b/docshell/test/navigation/file_bug1379762-2.html deleted file mode 100644 index 86033cb2e8..0000000000 --- a/docshell/test/navigation/file_bug1379762-2.html +++ /dev/null @@ -1,43 +0,0 @@ - - - - - Bug 1379762 - - - - diff --git a/docshell/test/navigation/file_document_write_1.html b/docshell/test/navigation/file_document_write_1.html index e0281f7cdb..169046a9b3 100644 --- a/docshell/test/navigation/file_document_write_1.html +++ b/docshell/test/navigation/file_document_write_1.html @@ -1,27 +1,16 @@ diff --git a/docshell/test/navigation/mochitest.ini b/docshell/test/navigation/mochitest.ini index 8cff81ad13..e2ee307e46 100644 --- a/docshell/test/navigation/mochitest.ini +++ b/docshell/test/navigation/mochitest.ini @@ -58,8 +58,8 @@ skip-if = (toolkit == 'android') || (!debug && (os == 'mac' || os == 'win')) # B [test_reserved.html] skip-if = (toolkit == 'android') || (debug && e10s) #too slow on Android 4.3 aws only; bug 1030403; bug 1263213 for debug e10s [test_sessionhistory.html] -skip-if = toolkit == 'android' #RANDOM -support-files = file_bug1379762-1.html file_bug1379762-2.html +skip-if = toolkit == 'android' #RANDOM on Android +support-files = file_bug1379762-1.html [test_sibling-matching-parent.html] [test_sibling-off-domain.html] [test_triggeringprincipal_frame_nav.html] diff --git a/docshell/test/navigation/test_sessionhistory.html b/docshell/test/navigation/test_sessionhistory.html index 10b0cbcafe..e5978acfa4 100644 --- a/docshell/test/navigation/test_sessionhistory.html +++ b/docshell/test/navigation/test_sessionhistory.html @@ -33,7 +33,6 @@ var testFiles = "file_scrollRestoration.html", "file_bug1300461.html", "file_bug1379762-1.html", - "file_bug1379762-2.html", ]; var testCount = 0; // Used by the test files. @@ -51,15 +50,6 @@ function nextTest_() { } } -// Needed by file_document_write_1.html -window.file_document_write_1_loadCount = 0; -function isTestDynamic() { - var dyn = testWindow.document.getElementById("dynamic"); - is(dyn, null, "Should have gone back to the static page!"); - nextTest(); - testWindow.close(); -} - function nextTest() { setTimeout(nextTest_, 0); } diff --git a/dom/base/test/test_x-frame-options.html b/dom/base/test/test_x-frame-options.html index a0c7acdc32..8e8cffcc3d 100644 --- a/dom/base/test/test_x-frame-options.html +++ b/dom/base/test/test_x-frame-options.html @@ -113,19 +113,25 @@ var testFramesLoaded = function() { // test that a document can be framed under a javascript: URL opened by the // same site as the frame +// We can't set a load event listener before calling document.open/document.write, because those will remove such listeners. So we need to define a function that the new window will be able to call. +function frameInJSURILoaded(win) { + var test = win.document.getElementById("sameorigin3") + .contentDocument.getElementById("test"); + ok(test != null, "frame under javascript: URL should have loaded."); + win.close(); + + // run last test + if (!isUnique) { + testFrameInDataURI(); + } else { + testFrameNotLoadedInDataURI(); + } +} + var testFrameInJSURI = function() { var html = ''; var win = window.open(); - win.onload = function() { - var test = win.document.getElementById("sameorigin3") - .contentDocument.getElementById("test"); - ok(test != null, "frame under javascript: URL should have loaded."); - win.close(); - - // run last test - testFrameInDataURI(); - } - win.location.href = "javascript:document.write('"+html+"');document.close();"; + win.location.href = "javascript:document.open(); onload = opener.frameInJSURILoaded.bind(null, window); document.write('"+html+"');document.close();"; } // test that a document can be framed under a data: URL opened by the diff --git a/dom/html/test/mochitest.ini b/dom/html/test/mochitest.ini index b9da7def8e..024de1cd91 100644 --- a/dom/html/test/mochitest.ini +++ b/dom/html/test/mochitest.ini @@ -529,7 +529,6 @@ skip-if = toolkit == 'android' # plugins not supported [test_bug196523.html] [test_bug199692.html] skip-if = toolkit == 'android' #bug 811644 -[test_bug172261.html] [test_bug255820.html] [test_bug259332.html] [test_bug311681.html] diff --git a/dom/html/test/test_bug172261.html b/dom/html/test/test_bug172261.html deleted file mode 100644 index 2b5d752cd0..0000000000 --- a/dom/html/test/test_bug172261.html +++ /dev/null @@ -1,67 +0,0 @@ - - - - - Test for Bug 172261 - - - - -Mozilla Bug 172261 -

- -

- -
-
-
- - - diff --git a/dom/html/test/test_bug255820.html b/dom/html/test/test_bug255820.html index 20727fee4e..18073497b7 100644 --- a/dom/html/test/test_bug255820.html +++ b/dom/html/test/test_bug255820.html @@ -28,7 +28,7 @@ SimpleTest.waitForExplicitFinish(); is(document.characterSet, "UTF-8", "Unexpected character set for our document"); -var testsLeft = 4; +var testsLeft = 3; function testFinished() { --testsLeft; @@ -42,29 +42,11 @@ function charsetTestFinished(id, doc, charsetTarget) { testFinished(); } -function f2Continue() { -// Commented out pending discussion at the WHATWG -// $("f2"). -// setAttribute("onload", -// "charsetTestFinished('f2 reloaded', this.contentDocument, 'us-ascii');"); - $("f2"). - setAttribute("onload", - "testFinished();"); - $("f2").contentWindow.location.reload(); -} - function f3Continue() { var doc = $("f3").contentDocument; is(doc.defaultView.getComputedStyle(doc.body, "").color, "rgb(0, 180, 0)", - "Wrong color before reload"); - $("f3"). - setAttribute("onload", - 'var doc = this.contentDocument; ' + - 'is(doc.defaultView.getComputedStyle(doc.body, "").color, ' + - ' "rgb(0, 180, 0)",' + - ' "Wrong color after reload");' + - "charsetTestFinished('f1', this.contentDocument, 'UTF-8')"); - $("f3").contentWindow.location.reload(); + "Wrong color"); + charsetTestFinished('f3', doc, "UTF-8"); } function runTest() { @@ -74,12 +56,7 @@ function runTest() { doc.open(); doc.write(''); doc.close(); - is(doc.characterSet, "UTF-8", - "Unexpected character set for first frame after write"); - $("f1"). - setAttribute("onload", - "charsetTestFinished('f1', this.contentDocument, 'UTF-8')"); - $("f1").contentWindow.location.reload(); + charsetTestFinished("f1", doc, "UTF-8"); doc = $("f2").contentDocument; is(doc.characterSet, "UTF-8", @@ -96,12 +73,11 @@ function runTest() { "Unexpected character set for second frame after write"); $("f2"). setAttribute("onload", - "charsetTestFinished('f2', this.contentDocument, 'UTF-8');" + - "f2Continue()"); + "charsetTestFinished('f2', this.contentDocument, 'UTF-8');"); doc = $("f3").contentDocument; is(doc.characterSet, "UTF-8", - "Unexpected initial character set for first frame"); + "Unexpected initial character set for third frame"); doc.open(); var str = ''; str += ''; @@ -111,7 +87,7 @@ function runTest() { doc.write(str); doc.close(); is(doc.characterSet, "UTF-8", - "Unexpected character set for first frame after write"); + "Unexpected character set for third frame after write"); $("f3").setAttribute("onload", "f3Continue()"); } diff --git a/dom/tests/mochitest/bugs/test_bug346659.html b/dom/tests/mochitest/bugs/test_bug346659.html index 78c1fc659f..8596de7b19 100644 --- a/dom/tests/mochitest/bugs/test_bug346659.html +++ b/dom/tests/mochitest/bugs/test_bug346659.html @@ -108,7 +108,7 @@ function messageReceiver(evt) { is(testResult, "undefined", "Props on new window's child should go away when loading"); break; case 6: - is(testResult, "undefined", "Props on new window's child should go away when writing"); + is(testResult, "6", "Props on new window's child should go away when writing"); break; case 7: is(testResult, "7", "Props on different-domain window opened from different-domain new window can stay"); diff --git a/js/xpconnect/crashtests/crashtests.list b/js/xpconnect/crashtests/crashtests.list index 7325e26016..51f4b8643e 100644 --- a/js/xpconnect/crashtests/crashtests.list +++ b/js/xpconnect/crashtests/crashtests.list @@ -43,7 +43,7 @@ load 732870.html load 751995.html load 761831.html asserts(0-1) load 752038.html # We may hit bug 645229 here. -asserts(1) load 753162.html # We hit bug 675518 or bug 680086 here. +load 753162.html load 754311.html asserts(0-1) load 786142.html # We may hit bug 645229 here. load 797583.html diff --git a/layout/style/test/test_bug1232829.html b/layout/style/test/test_bug1232829.html index 8981d56e07..65bea2014d 100644 --- a/layout/style/test/test_bug1232829.html +++ b/layout/style/test/test_bug1232829.html @@ -19,7 +19,6 @@ function boom() { setTimeout(function() { var frameDoc = document.querySelector("iframe").contentDocument; frameDoc.write("3"); - frameDoc.defaultView.history.back(); requestAnimationFrame(function() { popup.close(); ok(true, "Didn't crash"); diff --git a/parser/htmlparser/tests/mochitest/test_bug715739.html b/parser/htmlparser/tests/mochitest/test_bug715739.html index 597160a195..e59fdf3326 100644 --- a/parser/htmlparser/tests/mochitest/test_bug715739.html +++ b/parser/htmlparser/tests/mochitest/test_bug715739.html @@ -33,24 +33,30 @@ function textChildren(node) { return s; } +var f, d; + function tick() { runNumber++; - var f = document.getElementsByTagName("iframe")[0]; - var d = f.contentDocument; + f = document.getElementsByTagName("iframe")[0]; + d = f.contentDocument; if (runNumber == 1) { - d.open(); - f.addEventListener("load", tick); - d.write("X"); - d.write("\u003cscript>document.write('Y');\u003c/script>"); - d.write("Z"); - d.close(); + frames[1].setTimeout(` + var d = parent.d; + var f = parent.f; + d.open(); + f.addEventListener("load", parent.tick); + d.write("X"); + d.write("\u003cscript>document.write('Y');\u003c/script>"); + d.write("Z"); + d.close(); + `); return; } if (runNumber == 2) { var text = textChildren(d.body); - is(text, "XYZ", "Wrong text before reload."); + is(text, "ABC", "Wrong text before reload."); f.contentWindow.location.reload(); return; } @@ -63,10 +69,20 @@ function tick() { } } +// We want to trigger a document.open/write with a different window as the +// entry global. Let's give that window a blob URL so we don't have to set up +// extra helper files. +var blob = new Blob(["ABC"], { type: "text/html" }); +var blobURL = URL.createObjectURL(blob); + + diff --git a/security/manager/ssl/tests/mochitest/mixedcontent/test_bug329869.html b/security/manager/ssl/tests/mochitest/mixedcontent/test_bug329869.html index 3b8fae25ed..596b656282 100644 --- a/security/manager/ssl/tests/mochitest/mixedcontent/test_bug329869.html +++ b/security/manager/ssl/tests/mochitest/mixedcontent/test_bug329869.html @@ -25,7 +25,7 @@ function afterNavigationTest() { - isSecurityState("broken", "security still broken after navigation"); + isSecurityState("secure", "when we navigate back, we're loading our secure page again and not loading an insecure script, so our security state is secure"); finish(); } diff --git a/testing/web-platform/tests/html/browsers/history/the-history-interface/traverse_the_history_write_after_load_1-1.html b/testing/web-platform/tests/html/browsers/history/the-history-interface/traverse_the_history_write_after_load_1-1.html index af0118a01e..945c8d81f8 100644 --- a/testing/web-platform/tests/html/browsers/history/the-history-interface/traverse_the_history_write_after_load_1-1.html +++ b/testing/web-platform/tests/html/browsers/history/the-history-interface/traverse_the_history_write_after_load_1-1.html @@ -5,11 +5,8 @@ opener.pages.push(2); onload = function() { setTimeout(function() { - document.write("3 diff --git a/testing/web-platform/tests/html/browsers/history/the-history-interface/traverse_the_history_write_after_load_1.html b/testing/web-platform/tests/html/browsers/history/the-history-interface/traverse_the_history_write_after_load_1.html index c2c31e76f4..404d61d0cf 100644 --- a/testing/web-platform/tests/html/browsers/history/the-history-interface/traverse_the_history_write_after_load_1.html +++ b/testing/web-platform/tests/html/browsers/history/the-history-interface/traverse_the_history_write_after_load_1.html @@ -11,12 +11,11 @@ function() { check_result = t.step_func( function() { - if (pages.length < 4) { + if (pages.length < 3) { setTimeout(check_result, 500); return } - //The pass condition here is based on the idea that the spec is wrong and browsers are right - assert_array_equals(pages, [2, 3, 2, 3], "Pages opened during history navigation"); + assert_array_equals(pages, [2, 3, 1], "Pages opened during history navigation"); t.done(); } ) diff --git a/testing/web-platform/tests/html/browsers/history/the-location-interface/reload_document_open_write-1.html b/testing/web-platform/tests/html/browsers/history/the-location-interface/reload_document_open_write-1.html index 1c5a1db8ff..e1a2e811c9 100644 --- a/testing/web-platform/tests/html/browsers/history/the-location-interface/reload_document_open_write-1.html +++ b/testing/web-platform/tests/html/browsers/history/the-location-interface/reload_document_open_write-1.html @@ -3,11 +3,16 @@