From 7d833e9c2e824c4740712db4ebecc842297a7362 Mon Sep 17 00:00:00 2001 From: Kagami Sascha Rosylight Date: Wed, 2 Mar 2022 01:32:04 +0000 Subject: [PATCH 01/26] Bug 1757633 - Move PipeToPump from header to cpp r=mgaudet Differential Revision: https://phabricator.services.mozilla.com/D139973 --- dom/streams/ReadableStreamPipeTo.cpp | 106 ++++++++++++++++++++++++ dom/streams/ReadableStreamPipeTo.h | 119 +++------------------------ 2 files changed, 116 insertions(+), 109 deletions(-) diff --git a/dom/streams/ReadableStreamPipeTo.cpp b/dom/streams/ReadableStreamPipeTo.cpp index 97d3ab75aa9e..6ea82c5a5bda 100644 --- a/dom/streams/ReadableStreamPipeTo.cpp +++ b/dom/streams/ReadableStreamPipeTo.cpp @@ -4,6 +4,7 @@ * 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/. */ +#include "mozilla/dom/AbortFollower.h" #include "mozilla/dom/AbortSignal.h" #include "mozilla/dom/ReadableStream.h" #include "mozilla/dom/ReadableStreamDefaultReader.h" @@ -13,9 +14,114 @@ #include "mozilla/dom/Promise.h" #include "mozilla/dom/PromiseNativeHandler.h" #include "mozilla/AlreadyAddRefed.h" +#include "mozilla/ErrorResult.h" +#include "nsCycleCollectionParticipant.h" +#include "nsISupportsImpl.h" namespace mozilla::dom { +struct PipeToReadRequest; +class WriteFinishedPromiseHandler; +class ShutdownActionFinishedPromiseHandler; + +// https://streams.spec.whatwg.org/#readable-stream-pipe-to (Steps 14-15.) +// +// This class implements everything that is required to read all chunks from +// the reader (source) and write them to writer (destination), while +// following the constraints given in the spec using our implementation-defined +// behavior. +// +// The cycle-collected references look roughly like this: +// clang-format off +// +// Closed promise <-- ReadableStreamDefaultReader <--> ReadableStream +// | ^ | +// |(PromiseHandler) |(mReader) |(ReadRequest) +// | | | +// |-------------> PipeToPump <------- +// ^ | | +// |---------------| | | +// | | |-------(mLastWrite) --------> +// |(PromiseHandler) | |< ---- (PromiseHandler) ---- Promise +// | | ^ +// | |(mWriter) |(mWriteRequests) +// | v | +// Closed promise <-- WritableStreamDefaultWriter <--------> WritableStream +// +// clang-format on +class PipeToPump final : public AbortFollower { + NS_DECL_CYCLE_COLLECTING_ISUPPORTS + NS_DECL_CYCLE_COLLECTION_CLASS(PipeToPump) + + friend struct PipeToReadRequest; + friend class WriteFinishedPromiseHandler; + friend class ShutdownActionFinishedPromiseHandler; + + PipeToPump(Promise* aPromise, ReadableStreamDefaultReader* aReader, + WritableStreamDefaultWriter* aWriter, bool aPreventClose, + bool aPreventAbort, bool aPreventCancel) + : mPromise(aPromise), + mReader(aReader), + mWriter(aWriter), + mPreventClose(aPreventClose), + mPreventAbort(aPreventAbort), + mPreventCancel(aPreventCancel) {} + + MOZ_CAN_RUN_SCRIPT void Start(JSContext* aCx, AbortSignal* aSignal); + + MOZ_CAN_RUN_SCRIPT_BOUNDARY void RunAbortAlgorithm() override; + + private: + ~PipeToPump() override = default; + + MOZ_CAN_RUN_SCRIPT void PerformAbortAlgorithm(JSContext* aCx, + AbortSignalImpl* aSignal); + + MOZ_CAN_RUN_SCRIPT bool SourceOrDestErroredOrClosed(JSContext* aCx); + + using ShutdownAction = already_AddRefed (*)( + JSContext*, PipeToPump*, JS::Handle>, + ErrorResult&); + + MOZ_CAN_RUN_SCRIPT void ShutdownWithAction( + JSContext* aCx, ShutdownAction aAction, + JS::Handle> aError); + MOZ_CAN_RUN_SCRIPT void ShutdownWithActionAfterFinishedWrite( + JSContext* aCx, ShutdownAction aAction, + JS::Handle> aError); + + MOZ_CAN_RUN_SCRIPT void Shutdown( + JSContext* aCx, JS::Handle> aError); + + void Finalize(JSContext* aCx, JS::Handle> aError); + + MOZ_CAN_RUN_SCRIPT void OnReadFulfilled(JSContext* aCx, + JS::Handle aChunk, + ErrorResult& aRv); + MOZ_CAN_RUN_SCRIPT void OnWriterReady(JSContext* aCx, JS::Handle); + MOZ_CAN_RUN_SCRIPT void Read(JSContext* aCx); + + MOZ_CAN_RUN_SCRIPT void OnSourceClosed(JSContext* aCx, JS::Handle); + MOZ_CAN_RUN_SCRIPT void OnSourceErrored(JSContext* aCx, + JS::Handle aError); + + MOZ_CAN_RUN_SCRIPT void OnDestClosed(JSContext* aCx, JS::Handle); + MOZ_CAN_RUN_SCRIPT void OnDestErrored(JSContext* aCx, + JS::Handle aError); + + RefPtr mPromise; + RefPtr mReader; + RefPtr mWriter; + RefPtr mLastWritePromise; + const bool mPreventClose; + const bool mPreventAbort; + const bool mPreventCancel; + bool mShuttingDown = false; +#ifdef DEBUG + bool mReadChunk = false; +#endif +}; + // This is a helper class for PipeToPump that allows it to attach // member functions as promise handlers. class PipeToPumpHandler final : public PromiseNativeHandler { diff --git a/dom/streams/ReadableStreamPipeTo.h b/dom/streams/ReadableStreamPipeTo.h index 2a5fa44b5cf8..6700aa97409f 100644 --- a/dom/streams/ReadableStreamPipeTo.h +++ b/dom/streams/ReadableStreamPipeTo.h @@ -7,125 +7,26 @@ #ifndef mozilla_dom_ReadableStreamPipeTo_h #define mozilla_dom_ReadableStreamPipeTo_h -#include "mozilla/ErrorResult.h" -#include "mozilla/dom/AbortFollower.h" -#include "nsCycleCollectionParticipant.h" -#include "nsISupportsImpl.h" +#include "mozilla/Attributes.h" +#include "mozilla/AlreadyAddRefed.h" -namespace mozilla::dom { +namespace mozilla { -struct PipeToReadRequest; -class WriteFinishedPromiseHandler; -class ShutdownActionFinishedPromiseHandler; +class ErrorResult; +namespace dom { + +class AbortSignal; +class Promise; class ReadableStream; -class ReadableStreamDefaultReader; class WritableStream; -class WritableStreamDefaultWriter; - -// https://streams.spec.whatwg.org/#readable-stream-pipe-to (Steps 14-15.) -// -// This class implements everything that is required to read all chunks from -// the reader (source) and write them to writer (destination), while -// following the constraints given in the spec using our implementation-defined -// behavior. -// -// The cycle-collected references look roughly like this: -// clang-format off -// -// Closed promise <-- ReadableStreamDefaultReader <--> ReadableStream -// | ^ | -// |(PromiseHandler) |(mReader) |(ReadRequest) -// | | | -// |-------------> PipeToPump <------- -// ^ | | -// |---------------| | | -// | | |-------(mLastWrite) --------> -// |(PromiseHandler) | |< ---- (PromiseHandler) ---- Promise -// | | ^ -// | |(mWriter) |(mWriteRequests) -// | v | -// Closed promise <-- WritableStreamDefaultWriter <--------> WritableStream -// -// clang-format on -class PipeToPump final : public AbortFollower { - NS_DECL_CYCLE_COLLECTING_ISUPPORTS - NS_DECL_CYCLE_COLLECTION_CLASS(PipeToPump) - - friend struct PipeToReadRequest; - friend class WriteFinishedPromiseHandler; - friend class ShutdownActionFinishedPromiseHandler; - - PipeToPump(Promise* aPromise, ReadableStreamDefaultReader* aReader, - WritableStreamDefaultWriter* aWriter, bool aPreventClose, - bool aPreventAbort, bool aPreventCancel) - : mPromise(aPromise), - mReader(aReader), - mWriter(aWriter), - mPreventClose(aPreventClose), - mPreventAbort(aPreventAbort), - mPreventCancel(aPreventCancel) {} - - MOZ_CAN_RUN_SCRIPT void Start(JSContext* aCx, AbortSignal* aSignal); - - MOZ_CAN_RUN_SCRIPT_BOUNDARY void RunAbortAlgorithm() override; - - private: - ~PipeToPump() override = default; - - MOZ_CAN_RUN_SCRIPT void PerformAbortAlgorithm(JSContext* aCx, - AbortSignalImpl* aSignal); - - MOZ_CAN_RUN_SCRIPT bool SourceOrDestErroredOrClosed(JSContext* aCx); - - using ShutdownAction = already_AddRefed (*)( - JSContext*, PipeToPump*, JS::Handle>, - ErrorResult&); - - MOZ_CAN_RUN_SCRIPT void ShutdownWithAction( - JSContext* aCx, ShutdownAction aAction, - JS::Handle> aError); - MOZ_CAN_RUN_SCRIPT void ShutdownWithActionAfterFinishedWrite( - JSContext* aCx, ShutdownAction aAction, - JS::Handle> aError); - - MOZ_CAN_RUN_SCRIPT void Shutdown( - JSContext* aCx, JS::Handle> aError); - - void Finalize(JSContext* aCx, JS::Handle> aError); - - MOZ_CAN_RUN_SCRIPT void OnReadFulfilled(JSContext* aCx, - JS::Handle aChunk, - ErrorResult& aRv); - MOZ_CAN_RUN_SCRIPT void OnWriterReady(JSContext* aCx, JS::Handle); - MOZ_CAN_RUN_SCRIPT void Read(JSContext* aCx); - - MOZ_CAN_RUN_SCRIPT void OnSourceClosed(JSContext* aCx, JS::Handle); - MOZ_CAN_RUN_SCRIPT void OnSourceErrored(JSContext* aCx, - JS::Handle aError); - - MOZ_CAN_RUN_SCRIPT void OnDestClosed(JSContext* aCx, JS::Handle); - MOZ_CAN_RUN_SCRIPT void OnDestErrored(JSContext* aCx, - JS::Handle aError); - - RefPtr mPromise; - RefPtr mReader; - RefPtr mWriter; - RefPtr mLastWritePromise; - const bool mPreventClose; - const bool mPreventAbort; - const bool mPreventCancel; - bool mShuttingDown = false; -#ifdef DEBUG - bool mReadChunk = false; -#endif -}; MOZ_CAN_RUN_SCRIPT already_AddRefed ReadableStreamPipeTo( ReadableStream* aSource, WritableStream* aDest, bool aPreventClose, bool aPreventAbort, bool aPreventCancel, AbortSignal* aSignal, ErrorResult& aRv); -} // namespace mozilla::dom +} // namespace dom +} // namespace mozilla #endif // mozilla_dom_ReadableStreamPipeTo_h From ad14d19c547ed2d4f57347408c2654d519a1ee94 Mon Sep 17 00:00:00 2001 From: Makoto Kato Date: Wed, 2 Mar 2022 03:48:13 +0000 Subject: [PATCH 02/26] Bug 1753574 - Remove unnecessary nullptr check for screen orientation promise. r=smaug We never return `nullptr` for the promise of screen orientation lock. So this is unnecessary now to check `nullptr`. Differential Revision: https://phabricator.services.mozilla.com/D137969 --- dom/base/ScreenOrientation.cpp | 39 ++++++++++------------------------ 1 file changed, 11 insertions(+), 28 deletions(-) diff --git a/dom/base/ScreenOrientation.cpp b/dom/base/ScreenOrientation.cpp index c956f97dc1a9..9a570bbb232b 100644 --- a/dom/base/ScreenOrientation.cpp +++ b/dom/base/ScreenOrientation.cpp @@ -188,27 +188,16 @@ ScreenOrientation::LockOrientationTask::Run() { return NS_OK; } - RefPtr lockOrientationPromise = - mScreenOrientation->LockDeviceOrientation(mOrientationLock, - mIsFullscreen); - - if (NS_WARN_IF(!lockOrientationPromise)) { - mPromise->MaybeReject(NS_ERROR_UNEXPECTED); - mDocument->ClearOrientationPendingPromise(); - return NS_OK; - } - - lockOrientationPromise->Then( - GetCurrentSerialEventTarget(), __func__, - [self = RefPtr{this}]( - const LockOrientationPromise::ResolveOrRejectValue& aValue) { - if (aValue.IsResolve()) { - return NS_OK; - } - self->mPromise->MaybeReject(NS_ERROR_DOM_ABORT_ERR); - self->mDocument->ClearOrientationPendingPromise(); - return NS_OK; - }); + mScreenOrientation->LockDeviceOrientation(mOrientationLock, mIsFullscreen) + ->Then(GetCurrentSerialEventTarget(), __func__, + [self = RefPtr{this}]( + const LockOrientationPromise::ResolveOrRejectValue& aValue) { + if (aValue.IsResolve()) { + return; + } + self->mPromise->MaybeReject(NS_ERROR_DOM_ABORT_ERR); + self->mDocument->ClearOrientationPendingPromise(); + }); BrowsingContext* bc = mDocument->GetBrowsingContext(); if (OrientationLockContains(bc->GetCurrentOrientationType()) || @@ -392,14 +381,8 @@ RefPtr ScreenOrientation::LockDeviceOrientation( } } - RefPtr halPromise = - hal::LockScreenOrientation(aOrientation); - if (!halPromise) { - return LockOrientationPromise::CreateAndReject(false, __func__); - } - mTriedToLockDeviceOrientation = true; - return halPromise; + return hal::LockScreenOrientation(aOrientation); } void ScreenOrientation::Unlock(ErrorResult& aRv) { From 3493d2727e2882794077d03a7705c0a965ccd91d Mon Sep 17 00:00:00 2001 From: Makoto Kato Date: Wed, 2 Mar 2022 03:48:14 +0000 Subject: [PATCH 03/26] Bug 1753574 - hal::LockOrientation can return error status. r=smaug,geckoview-reviewers,agi,calu From https://w3c.github.io/screen-orientation/#apply-an-orientation-lock > 7.2. Apply an orientation lock > > The steps to apply an orientation lock to a Document using orientation are as > follows: > > 1. If the user agent does not support locking the screen orientation, return > a promise rejected with a "NotSupportedError" DOMException and abort > these steps. So if orientation controller delegate isn't set, we should throw `NotSupportedError`. But, actually, we throws `AbortError`, so this isn't correct. To return any DOM error from platform implementation of `screen.orientation.lock`, I would like to change return value to `GenericPromise`'s. Differential Revision: https://phabricator.services.mozilla.com/D137970 --- dom/base/ScreenOrientation.cpp | 35 +++++++------- dom/base/ScreenOrientation.h | 2 +- hal/Hal.cpp | 2 +- hal/Hal.h | 4 +- hal/android/AndroidHal.cpp | 32 +++++++++---- hal/fallback/FallbackScreenConfiguration.cpp | 6 +-- hal/sandbox/PHal.ipdl | 2 +- hal/sandbox/SandboxHal.cpp | 47 ++++++++++--------- .../geckoview/test/OrientationDelegateTest.kt | 18 +++++++ .../org/mozilla/geckoview/GeckoRuntime.java | 20 ++++---- widget/android/jni/Conversions.cpp | 6 +++ 11 files changed, 110 insertions(+), 64 deletions(-) diff --git a/dom/base/ScreenOrientation.cpp b/dom/base/ScreenOrientation.cpp index 9a570bbb232b..fd4fe4ab07c3 100644 --- a/dom/base/ScreenOrientation.cpp +++ b/dom/base/ScreenOrientation.cpp @@ -152,8 +152,6 @@ ScreenOrientation::LockOrientationTask::LockOrientationTask( ScreenOrientation::LockOrientationTask::~LockOrientationTask() = default; -using LockOrientationPromise = MozPromise; - bool ScreenOrientation::LockOrientationTask::OrientationLockContains( OrientationType aOrientationType) { return bool(mOrientationLock & OrientationTypeToInternal(aOrientationType)); @@ -189,15 +187,16 @@ ScreenOrientation::LockOrientationTask::Run() { } mScreenOrientation->LockDeviceOrientation(mOrientationLock, mIsFullscreen) - ->Then(GetCurrentSerialEventTarget(), __func__, - [self = RefPtr{this}]( - const LockOrientationPromise::ResolveOrRejectValue& aValue) { - if (aValue.IsResolve()) { - return; - } - self->mPromise->MaybeReject(NS_ERROR_DOM_ABORT_ERR); - self->mDocument->ClearOrientationPendingPromise(); - }); + ->Then( + GetCurrentSerialEventTarget(), __func__, + [self = RefPtr{this}]( + const GenericNonExclusivePromise::ResolveOrRejectValue& aValue) { + if (aValue.IsResolve()) { + return; + } + self->mPromise->MaybeReject(aValue.RejectValue()); + self->mDocument->ClearOrientationPendingPromise(); + }); BrowsingContext* bc = mDocument->GetBrowsingContext(); if (OrientationLockContains(bc->GetCurrentOrientationType()) || @@ -352,10 +351,11 @@ already_AddRefed ScreenOrientation::LockInternal( #endif } -RefPtr ScreenOrientation::LockDeviceOrientation( +RefPtr ScreenOrientation::LockDeviceOrientation( hal::ScreenOrientation aOrientation, bool aIsFullscreen) { if (!GetOwner()) { - return LockOrientationPromise::CreateAndReject(false, __func__); + return GenericNonExclusivePromise::CreateAndReject(NS_ERROR_DOM_ABORT_ERR, + __func__); } nsCOMPtr target = GetOwner()->GetDoc(); @@ -364,7 +364,8 @@ RefPtr ScreenOrientation::LockDeviceOrientation( // This needs to be done before LockScreenOrientation call to make sure // the locking can be unlocked. if (aIsFullscreen && !target) { - return LockOrientationPromise::CreateAndReject(false, __func__); + return GenericNonExclusivePromise::CreateAndReject(NS_ERROR_DOM_ABORT_ERR, + __func__); } // We are fullscreen and lock has been accepted. @@ -377,7 +378,8 @@ RefPtr ScreenOrientation::LockDeviceOrientation( mFullscreenListener, /* aUseCapture = */ true); if (NS_WARN_IF(NS_FAILED(rv))) { - return LockOrientationPromise::CreateAndReject(false, __func__); + return GenericNonExclusivePromise::CreateAndReject(NS_ERROR_DOM_ABORT_ERR, + __func__); } } @@ -552,8 +554,7 @@ void ScreenOrientation::UpdateActiveOrientationLock( hal::LockScreenOrientation(aOrientation) ->Then( GetMainThreadSerialEventTarget(), __func__, - [](const mozilla::MozPromise::ResolveOrRejectValue& aValue) { + [](const GenericNonExclusivePromise::ResolveOrRejectValue& aValue) { NS_WARNING_ASSERTION(aValue.IsResolve(), "hal::LockScreenOrientation failed"); }); diff --git a/dom/base/ScreenOrientation.h b/dom/base/ScreenOrientation.h index c9c1fbc4a211..f43a9c7ad61c 100644 --- a/dom/base/ScreenOrientation.h +++ b/dom/base/ScreenOrientation.h @@ -74,7 +74,7 @@ class ScreenOrientation final : public DOMEventTargetHelper { // This method calls into the HAL to lock the device and sets // up listeners for full screen change. - RefPtr> LockDeviceOrientation( + RefPtr LockDeviceOrientation( hal::ScreenOrientation aOrientation, bool aIsFullscreen); // This method calls in to the HAL to unlock the device and removes diff --git a/hal/Hal.cpp b/hal/Hal.cpp index 01536b4fa5b7..6214dd0a0f0c 100644 --- a/hal/Hal.cpp +++ b/hal/Hal.cpp @@ -378,7 +378,7 @@ void NotifyWakeLockChange(const WakeLockInformation& aInfo) { WakeLockObservers()->BroadcastInformation(aInfo); } -RefPtr> LockScreenOrientation( +RefPtr LockScreenOrientation( const ScreenOrientation& aOrientation) { AssertMainThread(); RETURN_PROXY_IF_SANDBOXED(LockScreenOrientation(aOrientation), nullptr); diff --git a/hal/Hal.h b/hal/Hal.h index d4b007448546..5347bc4efe55 100644 --- a/hal/Hal.h +++ b/hal/Hal.h @@ -220,8 +220,8 @@ void NotifyWakeLockChange(const hal::WakeLockInformation& aWakeLockInfo); * Lock the screen orientation to the specific orientation. * @return A promise indicating that the screen orientation has been locked. */ -[[nodiscard]] RefPtr> -LockScreenOrientation(const hal::ScreenOrientation& aOrientation); +[[nodiscard]] RefPtr LockScreenOrientation( + const hal::ScreenOrientation& aOrientation); /** * Unlock the screen orientation. diff --git a/hal/android/AndroidHal.cpp b/hal/android/AndroidHal.cpp index 2fc18d45970a..09544f23234d 100644 --- a/hal/android/AndroidHal.cpp +++ b/hal/android/AndroidHal.cpp @@ -101,25 +101,39 @@ static bool IsSupportedScreenOrientation(hal::ScreenOrientation aOrientation) { return false; } -RefPtr> LockScreenOrientation( +RefPtr LockScreenOrientation( const hal::ScreenOrientation& aOrientation) { - using LockPromise = MozPromise; - if (!IsSupportedScreenOrientation(aOrientation)) { NS_WARNING("Unsupported screen orientation type"); - return LockPromise::CreateAndReject(false, __func__); + return GenericNonExclusivePromise::CreateAndReject( + NS_ERROR_DOM_NOT_SUPPORTED_ERR, __func__); } java::GeckoRuntime::LocalRef runtime = java::GeckoRuntime::GetInstance(); if (!runtime) { - return LockPromise::CreateAndReject(false, __func__); + return GenericNonExclusivePromise::CreateAndReject(NS_ERROR_DOM_ABORT_ERR, + __func__); } auto result = runtime->LockScreenOrientation(uint32_t(aOrientation)); auto geckoResult = java::GeckoResult::LocalRef(std::move(result)); - if (!geckoResult) { - return LockPromise::CreateAndReject(false, __func__); - } - return LockPromise::FromGeckoResult(geckoResult); + return GenericNonExclusivePromise::FromGeckoResult(geckoResult) + ->Then( + GetCurrentSerialEventTarget(), __func__, + [](const GenericNonExclusivePromise::ResolveOrRejectValue& aValue) { + if (aValue.IsResolve()) { + if (aValue.ResolveValue()) { + return GenericNonExclusivePromise::CreateAndResolve(true, + __func__); + } + // Delegated orientation controller returns failure for + // lock. + return GenericNonExclusivePromise::CreateAndReject( + NS_ERROR_DOM_ABORT_ERR, __func__); + } + // Browser side doesn't implement orientation delegate. + return GenericNonExclusivePromise::CreateAndReject( + NS_ERROR_DOM_NOT_SUPPORTED_ERR, __func__); + }); } void UnlockScreenOrientation() { diff --git a/hal/fallback/FallbackScreenConfiguration.cpp b/hal/fallback/FallbackScreenConfiguration.cpp index bd4b98ede21f..e298855b90ce 100644 --- a/hal/fallback/FallbackScreenConfiguration.cpp +++ b/hal/fallback/FallbackScreenConfiguration.cpp @@ -6,10 +6,10 @@ namespace mozilla::hal_impl { -RefPtr> LockScreenOrientation( +RefPtr LockScreenOrientation( const hal::ScreenOrientation& aOrientation) { - return mozilla::MozPromise::CreateAndReject(false, - __func__); + return GenericNonExclusivePromise::CreateAndReject( + NS_ERROR_DOM_NOT_SUPPORTED_ERR, __func__); } void UnlockScreenOrientation() {} diff --git a/hal/sandbox/PHal.ipdl b/hal/sandbox/PHal.ipdl index 71dc9ff152c6..45203477466b 100644 --- a/hal/sandbox/PHal.ipdl +++ b/hal/sandbox/PHal.ipdl @@ -82,7 +82,7 @@ parent: returns (WakeLockInformation aWakeLockInfo); async LockScreenOrientation(ScreenOrientation aOrientation) - returns (bool allowed); + returns (nsresult result); async UnlockScreenOrientation(); child: diff --git a/hal/sandbox/SandboxHal.cpp b/hal/sandbox/SandboxHal.cpp index 76c705554793..fe579d13192f 100644 --- a/hal/sandbox/SandboxHal.cpp +++ b/hal/sandbox/SandboxHal.cpp @@ -69,21 +69,24 @@ void GetCurrentNetworkInformation(NetworkInformation* aNetworkInfo) { Hal()->SendGetCurrentNetworkInformation(aNetworkInfo); } -RefPtr> LockScreenOrientation( +RefPtr LockScreenOrientation( const hal::ScreenOrientation& aOrientation) { return Hal() ->SendLockScreenOrientation(aOrientation) - ->Then( - GetCurrentSerialEventTarget(), __func__, - [=](const mozilla::MozPromise::ResolveOrRejectValue& aValue) { - if (aValue.IsResolve() && aValue.ResolveValue()) { - return mozilla::MozPromise::CreateAndResolve( - true, __func__); - } - return mozilla::MozPromise::CreateAndReject( - false, __func__); - }); + ->Then(GetCurrentSerialEventTarget(), __func__, + [](const mozilla::MozPromise::ResolveOrRejectValue& aValue) { + if (aValue.IsResolve()) { + if (NS_SUCCEEDED(aValue.ResolveValue())) { + return GenericNonExclusivePromise::CreateAndResolve( + true, __func__); + } + return GenericNonExclusivePromise::CreateAndReject( + aValue.ResolveValue(), __func__); + } + return GenericNonExclusivePromise::CreateAndReject( + NS_ERROR_FAILURE, __func__); + }); } void UnlockScreenOrientation() { Hal()->SendUnlockScreenOrientation(); } @@ -223,15 +226,17 @@ class HalParent : public PHalParent, // fullscreen. We don't have that information currently. hal::LockScreenOrientation(aOrientation) - ->Then(GetMainThreadSerialEventTarget(), __func__, - [aResolve](const mozilla::MozPromise< - bool, bool, false>::ResolveOrRejectValue& aValue) { - if (aValue.IsResolve()) { - aResolve(aValue.ResolveValue()); - } else { - aResolve(false); - } - }); + ->Then( + GetMainThreadSerialEventTarget(), __func__, + [aResolve](const GenericNonExclusivePromise::ResolveOrRejectValue& + aValue) { + if (aValue.IsResolve()) { + MOZ_ASSERT(aValue.ResolveValue()); + aResolve(NS_OK); + return; + } + aResolve(aValue.RejectValue()); + }); return IPC_OK(); } diff --git a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/OrientationDelegateTest.kt b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/OrientationDelegateTest.kt index 1ed996010935..8a0f89969bce 100644 --- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/OrientationDelegateTest.kt +++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/OrientationDelegateTest.kt @@ -178,4 +178,22 @@ class OrientationDelegateTest : BaseSessionTest() { } }) } + + @Test fun orientationLockUnsupported() { + // If no delegate, orientation.lock must throws NotSupportedError + sessionRule.setPrefsUntilTestEnd(mapOf("dom.screenorientation.allow-lock" to true)) + goFullscreen() + + val promise = mainSession.evaluatePromiseJS(""" + new Promise(r => { + screen.orientation.lock('landscape-primary') + .then(() => r("successful")) + .catch(e => r(e.name)) + }) + """.trimIndent()) + + assertThat("The operation must throw NotSupportedError", + promise.value, + equalTo("NotSupportedError")) + } } diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java index 48d6395d2db0..9c90b36bd69f 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java +++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java @@ -919,16 +919,18 @@ public final class GeckoRuntime implements Parcelable { final OrientationController.OrientationDelegate delegate = getOrientationController().getDelegate(); if (delegate == null) { - res.complete(false); - } else { - final GeckoResult response = - delegate.onOrientationLock(toAndroidOrientation(aOrientation)); - if (response == null) { - res.complete(false); - } else { - res.completeFrom(response.map(v -> v == AllowOrDeny.ALLOW)); - } + // Delegate is not set + res.completeExceptionally(new Exception("Not supported")); + return; } + final GeckoResult response = + delegate.onOrientationLock(toAndroidOrientation(aOrientation)); + if (response == null) { + // Delegate is default. So lock orientation is not implemented + res.completeExceptionally(new Exception("Not supported")); + return; + } + res.completeFrom(response.map(v -> v == AllowOrDeny.ALLOW)); }); return res; } diff --git a/widget/android/jni/Conversions.cpp b/widget/android/jni/Conversions.cpp index e8d5413ca304..ff4517daf2ed 100644 --- a/widget/android/jni/Conversions.cpp +++ b/widget/android/jni/Conversions.cpp @@ -103,5 +103,11 @@ nsString Java2Native(mozilla::jni::Object::Param aData, JNIEnv* aEnv) { return result; } +template <> +nsresult Java2Native(mozilla::jni::Object::Param aData, JNIEnv* aEnv) { + MOZ_ASSERT(aData.IsInstanceOf()); + return NS_ERROR_FAILURE; +} + } // namespace jni } // namespace mozilla From 7e130a3c59d1159bd46a4f7ff4424c3ef2eb686c Mon Sep 17 00:00:00 2001 From: Makoto Kato Date: Wed, 2 Mar 2022 03:48:14 +0000 Subject: [PATCH 04/26] Bug 1753574 - Don't resolve lock promise until LockDeviceOrientation is finished. r=smaug,geckoview-reviewers,agi Even if platform implementation always returns error, `orientation.lock('any')` is always successful. Because we don't wait for `LockDeviceOrientation`'s result when current screen orientation is same as lock type. If `LockDeviceOrientation` is failed, we should reject the promise. Differential Revision: https://phabricator.services.mozilla.com/D137971 --- dom/base/ScreenOrientation.cpp | 35 ++++++++++++++----- .../geckoview/test/OrientationDelegateTest.kt | 12 +++++++ 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/dom/base/ScreenOrientation.cpp b/dom/base/ScreenOrientation.cpp index fd4fe4ab07c3..1c5e92fb16a1 100644 --- a/dom/base/ScreenOrientation.cpp +++ b/dom/base/ScreenOrientation.cpp @@ -191,22 +191,39 @@ ScreenOrientation::LockOrientationTask::Run() { GetCurrentSerialEventTarget(), __func__, [self = RefPtr{this}]( const GenericNonExclusivePromise::ResolveOrRejectValue& aValue) { + if (self->mPromise->State() != Promise::PromiseState::Pending) { + // mPromise is already resolved or rejected by + // DispatchChangeEventAndResolvePromise() or + // AbortInProcessOrientationPromises(). + return; + } + + if (self->mDocument->GetOrientationPendingPromise() != + self->mPromise) { + // mPromise is old promise now and document has new promise by + // later `orientation.lock` call. Old promise is already rejected + // by AbortInProcessOrientationPromises() + return; + } if (aValue.IsResolve()) { + if (BrowsingContext* bc = self->mDocument->GetBrowsingContext()) { + if (self->OrientationLockContains( + bc->GetCurrentOrientationType()) || + (self->mOrientationLock == + hal::ScreenOrientation::Default && + bc->GetCurrentOrientationAngle() == 0)) { + // Orientation lock will not cause an orientation change, so + // we need to manually resolve the promise here. + self->mPromise->MaybeResolveWithUndefined(); + self->mDocument->ClearOrientationPendingPromise(); + } + } return; } self->mPromise->MaybeReject(aValue.RejectValue()); self->mDocument->ClearOrientationPendingPromise(); }); - BrowsingContext* bc = mDocument->GetBrowsingContext(); - if (OrientationLockContains(bc->GetCurrentOrientationType()) || - (mOrientationLock == hal::ScreenOrientation::Default && - bc->GetCurrentOrientationAngle() == 0)) { - // Orientation lock will not cause an orientation change. - mPromise->MaybeResolveWithUndefined(); - mDocument->ClearOrientationPendingPromise(); - } - return NS_OK; } diff --git a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/OrientationDelegateTest.kt b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/OrientationDelegateTest.kt index 8a0f89969bce..fca1eea3704d 100644 --- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/OrientationDelegateTest.kt +++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/OrientationDelegateTest.kt @@ -195,5 +195,17 @@ class OrientationDelegateTest : BaseSessionTest() { assertThat("The operation must throw NotSupportedError", promise.value, equalTo("NotSupportedError")) + + val promise2 = mainSession.evaluatePromiseJS(""" + new Promise(r => { + screen.orientation.lock(screen.orientation.type) + .then(() => r("successful")) + .catch(e => r(e.name)) + }) + """.trimIndent()) + + assertThat("The operation must throw NotSupportedError even if same orientation", + promise2.value, + equalTo("NotSupportedError")) } } From 8f28529e9733684014f4c08f426f45f734b197b2 Mon Sep 17 00:00:00 2001 From: Makoto Kato Date: Wed, 2 Mar 2022 03:48:14 +0000 Subject: [PATCH 05/26] Bug 1753574 - Use previous orientation type to compare whether orientation type isn't changed. r=smaug Previous fix was not enough to check whether orientation type is same or not. If lock is resolved after `ScreenOrientation::MaybeChanged` is called, `LockOrientationTask::Run` resolves promise unfortunately. I should check previous orientation type instead. Differential Revision: https://phabricator.services.mozilla.com/D137972 --- dom/base/ScreenOrientation.cpp | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/dom/base/ScreenOrientation.cpp b/dom/base/ScreenOrientation.cpp index 1c5e92fb16a1..5179bb46852c 100644 --- a/dom/base/ScreenOrientation.cpp +++ b/dom/base/ScreenOrientation.cpp @@ -186,10 +186,18 @@ ScreenOrientation::LockOrientationTask::Run() { return NS_OK; } + BrowsingContext* bc = mDocument->GetBrowsingContext(); + if (!bc) { + mPromise->MaybeResolveWithUndefined(); + mDocument->ClearOrientationPendingPromise(); + return NS_OK; + } + + OrientationType previousOrientationType = bc->GetCurrentOrientationType(); mScreenOrientation->LockDeviceOrientation(mOrientationLock, mIsFullscreen) ->Then( GetCurrentSerialEventTarget(), __func__, - [self = RefPtr{this}]( + [self = RefPtr{this}, previousOrientationType]( const GenericNonExclusivePromise::ResolveOrRejectValue& aValue) { if (self->mPromise->State() != Promise::PromiseState::Pending) { // mPromise is already resolved or rejected by @@ -206,9 +214,13 @@ ScreenOrientation::LockOrientationTask::Run() { return; } if (aValue.IsResolve()) { + // LockDeviceOrientation won't change orientation, so change + // event isn't fired. if (BrowsingContext* bc = self->mDocument->GetBrowsingContext()) { - if (self->OrientationLockContains( - bc->GetCurrentOrientationType()) || + OrientationType currentOrientationType = + bc->GetCurrentOrientationType(); + if ((previousOrientationType == currentOrientationType && + self->OrientationLockContains(currentOrientationType)) || (self->mOrientationLock == hal::ScreenOrientation::Default && bc->GetCurrentOrientationAngle() == 0)) { From add20048b9e3e4a734f0092e1005b00e2321db52 Mon Sep 17 00:00:00 2001 From: Shane Caraveo Date: Wed, 2 Mar 2022 03:59:48 +0000 Subject: [PATCH 06/26] Bug 1748550 Event Persistence subclass to simplify use r=rpl,robwu Differential Revision: https://phabricator.services.mozilla.com/D139087 --- toolkit/components/extensions/.eslintrc.js | 1 + .../components/extensions/ExtensionChild.jsm | 11 +-- .../components/extensions/ExtensionCommon.jsm | 84 ++++++++++++++++++- .../extensions/parent/ext-alarms.js | 2 +- .../extensions/parent/ext-browserSettings.js | 3 +- .../extensions/parent/ext-captivePortal.js | 28 ++----- .../extensions/parent/ext-privacy.js | 3 +- .../components/extensions/parent/ext-proxy.js | 9 +- .../extensions/parent/ext-webRequest.js | 4 +- .../xpcshell/test_ext_persistent_events.js | 2 +- 10 files changed, 111 insertions(+), 36 deletions(-) diff --git a/toolkit/components/extensions/.eslintrc.js b/toolkit/components/extensions/.eslintrc.js index 8b68a8c6abcc..0a2b7a384dff 100644 --- a/toolkit/components/extensions/.eslintrc.js +++ b/toolkit/components/extensions/.eslintrc.js @@ -13,6 +13,7 @@ module.exports = { Cu: true, AppConstants: true, ExtensionAPI: true, + ExtensionAPIPersistent: true, ExtensionCommon: true, ExtensionUtils: true, extensions: true, diff --git a/toolkit/components/extensions/ExtensionChild.jsm b/toolkit/components/extensions/ExtensionChild.jsm index ae3b5f47db38..5c6931730ad9 100644 --- a/toolkit/components/extensions/ExtensionChild.jsm +++ b/toolkit/components/extensions/ExtensionChild.jsm @@ -160,13 +160,14 @@ Services.obs.addObserver(StrongPromise, "extensions-onMessage-witness"); // Simple single-event emitter-like helper, exposes the EventManager api. class SimpleEventAPI extends EventManager { constructor(context, name) { - super({ context, name }); - this.fires = new Set(); - this.register = fire => { - this.fires.add(fire); + let fires = new Set(); + let register = fire => { + fires.add(fire); fire.location = context.getCaller(); - return () => this.fires.delete(fire); + return () => fires.delete(fire); }; + super({ context, name, register }); + this.fires = fires; } emit(...args) { return [...this.fires].map(fire => fire.asyncWithoutClone(...args)); diff --git a/toolkit/components/extensions/ExtensionCommon.jsm b/toolkit/components/extensions/ExtensionCommon.jsm index 57ab43643d0e..f33b3777fb9d 100644 --- a/toolkit/components/extensions/ExtensionCommon.jsm +++ b/toolkit/components/extensions/ExtensionCommon.jsm @@ -363,6 +363,72 @@ class ExtensionAPI extends EventEmitter { } } +/** + * Subclass to add APIs commonly used with persistent events. + * If a namespace uses events, it should use this subclass. + * + * this.apiNamespace = class extends ExtensionAPIPersistent {}; + */ +class ExtensionAPIPersistent extends ExtensionAPI { + /** + * Check for event entry. + * + * @param {string} event The event name e.g. onStateChanged + * @returns {boolean} + */ + hasEventRegistrar(event) { + return ( + this.PERSISTENT_EVENTS && Object.hasOwn(this.PERSISTENT_EVENTS, event) + ); + } + + /** + * Get the event registration fuction + * + * @param {string} event The event name e.g. onStateChanged + * @returns {Function} register is used to start the listener + * register returns an object containing + * a convert and unregister function. + */ + getEventRegistrar(event) { + if (this.hasEventRegistrar(event)) { + return this.PERSISTENT_EVENTS[event].bind(this); + } + } + + /** + * Used when instantiating an EventManager instance to register the listener. + * + * @param {Object} options used for event registration + * @param {BaseContext} options.context Passed when creating an EventManager instance. + * @param {string} options.event The function passed to the listener to fire the event. + * @param {Function} options.fire The function passed to the listener to fire the event. + * @returns {Function} the unregister function used in the EventManager. + */ + registerEventListener(options) { + let register = this.getEventRegistrar(options.event); + if (register) { + return register(options).unregister; + } + } + + /** + * Used to prime a listener for when the background script is not running. + * + * @param {string} event The event name e.g. onStateChanged or captiveURL.onChange. + * @param {Function} fire The function passed to the listener to fire the event. + * @param {Array} params Params passed to the event listener. + * @param {boolean} isInStartup unused here but passed for subclass use. + * @returns {Object} the unregister and convert functions used in the EventManager. + */ + primeListener(event, fire, params, isInStartup) { + let register = this.getEventRegistrar(event); + if (register) { + return register({ fire, isInStartup }, ...params); + } + } +} + /** * A wrapper around a window that returns the window iff the inner window * matches the inner window at the construction of this wrapper. @@ -1743,6 +1809,7 @@ class SchemaAPIManager extends EventEmitter { Cr, Cu, ExtensionAPI, + ExtensionAPIPersistent, ExtensionCommon, MatchGlob, MatchPattern, @@ -2145,6 +2212,9 @@ class EventManager { * The API module name, required for persistent events. * @param {string} params.event * The API event name, required for persistent events. + * @param {ExtensionAPI} params.extensionApi + * The API intance. If the API uses the ExtensionAPIPersistent class, some simplification is + * possible by passing the api (self or this) and the internal register function will be used. * @param {string} [params.name] * A name used only for debugging. If not provided, name is built from module and event. * @param {functon} params.register @@ -2160,6 +2230,7 @@ class EventManager { event, name, register, + extensionApi, inputHandling = false, } = params; this.context = context; @@ -2168,11 +2239,21 @@ class EventManager { this.name = name; this.register = register; this.inputHandling = inputHandling; - if (!name) { this.name = `${module}.${event}`; } + if (!this.register && extensionApi instanceof ExtensionAPIPersistent) { + this.register = fire => { + return extensionApi.registerEventListener({ context, event, fire }); + }; + } + if (!this.register) { + throw new Error( + `EventManager requires register method for ${this.name}.` + ); + } + this.canPersistEvents = module && event && @@ -2328,7 +2409,6 @@ class EventManager { try { let handler = api.primeListener( - extension, event, fire, listener.params, diff --git a/toolkit/components/extensions/parent/ext-alarms.js b/toolkit/components/extensions/parent/ext-alarms.js index 3a4ff42d91c1..9c4479f7e1ff 100644 --- a/toolkit/components/extensions/parent/ext-alarms.js +++ b/toolkit/components/extensions/parent/ext-alarms.js @@ -102,7 +102,7 @@ this.alarms = class extends ExtensionAPI { }; } - primeListener(extension, event, fire) { + primeListener(event, fire) { if (event == "onAlarm") { return this.registerOnAlarm(fire); } diff --git a/toolkit/components/extensions/parent/ext-browserSettings.js b/toolkit/components/extensions/parent/ext-browserSettings.js index 108e840db0c0..7844ddf0722e 100644 --- a/toolkit/components/extensions/parent/ext-browserSettings.js +++ b/toolkit/components/extensions/parent/ext-browserSettings.js @@ -375,7 +375,8 @@ this.browserSettings = class extends ExtensionAPI { }; } - primeListener(extension, event, fire) { + primeListener(event, fire) { + let { extension } = this; if (event == "homepageOverride") { return this.homePageOverrideListener(fire); } diff --git a/toolkit/components/extensions/parent/ext-captivePortal.js b/toolkit/components/extensions/parent/ext-captivePortal.js index c2c7e84ba6e3..836bf055e4be 100644 --- a/toolkit/components/extensions/parent/ext-captivePortal.js +++ b/toolkit/components/extensions/parent/ext-captivePortal.js @@ -29,7 +29,7 @@ const CAPTIVE_URL_PREF = "captivedetect.canonicalURL"; var { ExtensionError } = ExtensionUtils; -this.captivePortal = class extends ExtensionAPI { +this.captivePortal = class extends ExtensionAPIPersistent { checkCaptivePortalEnabled() { if (!gCaptivePortalEnabled) { throw new ExtensionError("Captive Portal detection is not enabled"); @@ -52,7 +52,7 @@ this.captivePortal = class extends ExtensionAPI { } PERSISTENT_EVENTS = { - onStateChanged: fire => { + onStateChanged({ fire }) { this.checkCaptivePortalEnabled(); let observer = (subject, topic) => { @@ -75,7 +75,7 @@ this.captivePortal = class extends ExtensionAPI { }, }; }, - onConnectivityAvailable: fire => { + onConnectivityAvailable({ fire }) { this.checkCaptivePortalEnabled(); let observer = (subject, topic, data) => { @@ -95,7 +95,7 @@ this.captivePortal = class extends ExtensionAPI { }, }; }, - "captiveURL.onChange": fire => { + "captiveURL.onChange": ({ fire }) => { let listener = (text, id) => { fire.async({ levelOfControl: "not_controllable", @@ -114,12 +114,6 @@ this.captivePortal = class extends ExtensionAPI { }, }; - primeListener(extension, event, fire) { - if (Object.hasOwn(this.PERSISTENT_EVENTS, event)) { - return this.PERSISTENT_EVENTS[event](fire); - } - } - getAPI(context) { let self = this; return { @@ -136,18 +130,13 @@ this.captivePortal = class extends ExtensionAPI { context, module: "captivePortal", event: "onStateChanged", - register: fire => { - return self.PERSISTENT_EVENTS.onStateChanged(fire).unregister; - }, + extensionApi: self, }).api(), onConnectivityAvailable: new EventManager({ context, module: "captivePortal", event: "onConnectivityAvailable", - register: fire => { - return self.PERSISTENT_EVENTS.onConnectivityAvailable(fire) - .unregister; - }, + extensionApi: self, }).api(), canonicalURL: getSettingsAPI({ context, @@ -160,10 +149,7 @@ this.captivePortal = class extends ExtensionAPI { context, module: "captivePortal", event: "captiveURL.onChange", - register: fire => { - return self.PERSISTENT_EVENTS["captiveURL.onChange"](fire) - .unregister; - }, + extensionApi: self, }).api(), }), }, diff --git a/toolkit/components/extensions/parent/ext-privacy.js b/toolkit/components/extensions/parent/ext-privacy.js index 017e786683f0..40b939c9a1e4 100644 --- a/toolkit/components/extensions/parent/ext-privacy.js +++ b/toolkit/components/extensions/parent/ext-privacy.js @@ -450,7 +450,8 @@ ExtensionPreferencesManager.addSetting("network.tlsVersionRestriction", { }); this.privacy = class extends ExtensionAPI { - primeListener(extension, event, fire) { + primeListener(event, fire) { + let { extension } = this; let listener = getPrimedSettingsListener({ extension, name: event, diff --git a/toolkit/components/extensions/parent/ext-proxy.js b/toolkit/components/extensions/parent/ext-proxy.js index 41e2733dcada..a4f3dac7b26b 100644 --- a/toolkit/components/extensions/parent/ext-proxy.js +++ b/toolkit/components/extensions/parent/ext-proxy.js @@ -125,9 +125,14 @@ function registerProxyFilterEvent( } this.proxy = class extends ExtensionAPI { - primeListener(extension, event, fire, params) { + primeListener(event, fire, params) { if (event === "onRequest") { - return registerProxyFilterEvent(undefined, extension, fire, ...params); + return registerProxyFilterEvent( + undefined, + this.extension, + fire, + ...params + ); } } diff --git a/toolkit/components/extensions/parent/ext-webRequest.js b/toolkit/components/extensions/parent/ext-webRequest.js index b419a6bc707f..8d83400c5984 100644 --- a/toolkit/components/extensions/parent/ext-webRequest.js +++ b/toolkit/components/extensions/parent/ext-webRequest.js @@ -123,10 +123,10 @@ function makeWebRequestEvent(context, event) { } this.webRequest = class extends ExtensionAPI { - primeListener(extension, event, fire, params, isInStartup) { + primeListener(event, fire, params, isInStartup) { // During early startup if the listener does not use blocking we do not prime it. if (!isInStartup || params[1]?.includes("blocking")) { - return registerEvent(extension, event, fire, ...params); + return registerEvent(this.extension, event, fire, ...params); } } diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_persistent_events.js b/toolkit/components/extensions/test/xpcshell/test_ext_persistent_events.js index 4d159f2cf144..aadfb17a5d4c 100644 --- a/toolkit/components/extensions/test/xpcshell/test_ext_persistent_events.js +++ b/toolkit/components/extensions/test/xpcshell/test_ext_persistent_events.js @@ -11,7 +11,7 @@ const { ExtensionAPI } = ExtensionCommon; /* global EventManager */ const API = class extends ExtensionAPI { static namespace = undefined; - primeListener(extension, event, fire, params) { + primeListener(event, fire, params) { // eslint-disable-next-line no-undef let { eventName, throwError, ignoreListener } = this.constructor.testOptions || {}; From d3868ecd835d9f45b31f10f7befe8a9e98342a5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A3o=20Gottwald?= Date: Wed, 2 Mar 2022 04:54:57 +0000 Subject: [PATCH 07/26] Bug 1757572 - Remove --toolbarbutton-focus-outline which at this point is just an alias for --focus-outline. r=Itiel Differential Revision: https://phabricator.services.mozilla.com/D139945 --- .../identity-block/identity-block.inc.css | 8 ++--- .../themes/shared/notification-icons.inc.css | 8 ++--- browser/themes/shared/toolbarbuttons.inc.css | 33 +++++++------------ .../themes/shared/urlbar-searchbar.inc.css | 6 ++-- toolkit/themes/shared/notification.css | 24 ++++++-------- 5 files changed, 33 insertions(+), 46 deletions(-) diff --git a/browser/themes/shared/identity-block/identity-block.inc.css b/browser/themes/shared/identity-block/identity-block.inc.css index 6dcffef1af8a..2c6e0e21f6cf 100644 --- a/browser/themes/shared/identity-block/identity-block.inc.css +++ b/browser/themes/shared/identity-block/identity-block.inc.css @@ -43,10 +43,10 @@ color: var(--urlbar-box-hover-text-color); } -.identity-box-button:not(:active):-moz-focusring, -#tracking-protection-icon-container:not(:active):-moz-focusring { - outline: var(--toolbarbutton-focus-outline); - outline-offset: -2px; +.identity-box-button:not(:active):focus-visible, +#tracking-protection-icon-container:not(:active):focus-visible { + outline: var(--focus-outline); + outline-offset: calc(-1 * var(--focus-outline-width)); } .identity-box-button { diff --git a/browser/themes/shared/notification-icons.inc.css b/browser/themes/shared/notification-icons.inc.css index 210d88224d30..9d983f060d73 100644 --- a/browser/themes/shared/notification-icons.inc.css +++ b/browser/themes/shared/notification-icons.inc.css @@ -24,10 +24,10 @@ fill: currentColor; } -.blocked-permission-icon:-moz-focusring, -.notification-anchor-icon:-moz-focusring { - outline: var(--toolbarbutton-focus-outline); - outline-offset: calc(var(--urlbar-icon-padding) - 2px); +.blocked-permission-icon:focus-visible, +.notification-anchor-icon:focus-visible { + outline: var(--focus-outline); + outline-offset: calc(var(--urlbar-icon-padding) - var(--focus-outline-width)); } /* This class can be used alone or in combination with the class defining the diff --git a/browser/themes/shared/toolbarbuttons.inc.css b/browser/themes/shared/toolbarbuttons.inc.css index 7ffc1eaa00e2..2d008ed82bd3 100644 --- a/browser/themes/shared/toolbarbuttons.inc.css +++ b/browser/themes/shared/toolbarbuttons.inc.css @@ -20,10 +20,6 @@ --toolbarbutton-height: 0; } -:root { - --toolbarbutton-focus-outline: var(--focus-outline); -} - :root[uidensity=compact] { --toolbarbutton-outer-padding: 3px; --toolbarbutton-inner-padding: 6px; @@ -188,30 +184,25 @@ toolbar .toolbarbutton-1:not([disabled=true]):is([open],[checked],:hover:active) } /* Keyboard focus styling */ -#PersonalToolbar .toolbarbutton-1:-moz-focusring, -findbar toolbarbutton.tabbable:-moz-focusring, -toolbarbutton.bookmark-item:not(.subviewbutton):-moz-focusring, -toolbar:not(#PersonalToolbar) .toolbarbutton-1:-moz-focusring > .toolbarbutton-icon, -toolbar:not(#PersonalToolbar) .toolbarbutton-1:-moz-focusring > .toolbarbutton-text, -toolbar:not(#PersonalToolbar) .toolbarbutton-1:-moz-focusring > .toolbarbutton-badge-stack { +#PersonalToolbar .toolbarbutton-1:focus-visible, +findbar toolbarbutton.tabbable:focus-visible, +toolbarbutton.bookmark-item:not(.subviewbutton):focus-visible, +toolbar:not(#PersonalToolbar) .toolbarbutton-1:focus-visible > .toolbarbutton-icon, +toolbar:not(#PersonalToolbar) .toolbarbutton-1:focus-visible > .toolbarbutton-text, +toolbar:not(#PersonalToolbar) .toolbarbutton-1:focus-visible > .toolbarbutton-badge-stack { color: inherit; - outline: var(--toolbarbutton-focus-outline); + outline: var(--focus-outline); outline-offset: calc(var(--focus-outline-width) * -1); } -:root[uidensity=compact] findbar toolbarbutton.tabbable:-moz-focusring, -:root[uidensity=compact] toolbar:not(#PersonalToolbar) .toolbarbutton-1:-moz-focusring > .toolbarbutton-icon, -:root[uidensity=compact] toolbar:not(#PersonalToolbar) .toolbarbutton-1:-moz-focusring > .toolbarbutton-text, -:root[uidensity=compact] toolbar:not(#PersonalToolbar) .toolbarbutton-1:-moz-focusring > .toolbarbutton-badge-stack { +:root[uidensity=compact] findbar toolbarbutton.tabbable:focus-visible, +:root[uidensity=compact] toolbar:not(#PersonalToolbar) .toolbarbutton-1:focus-visible > .toolbarbutton-icon, +:root[uidensity=compact] toolbar:not(#PersonalToolbar) .toolbarbutton-1:focus-visible > .toolbarbutton-text, +:root[uidensity=compact] toolbar:not(#PersonalToolbar) .toolbarbutton-1:focus-visible > .toolbarbutton-badge-stack { outline-offset: calc(var(--focus-outline-width) * -1 - 1px); } -#PersonalToolbar .toolbarbutton-1:-moz-focusring, -toolbarbutton.bookmark-item:not(.subviewbutton):-moz-focusring { - outline-offset: -2px; -} - -toolbar .toolbarbutton-1:-moz-focusring { +toolbar .toolbarbutton-1:focus-visible { outline: none; } diff --git a/browser/themes/shared/urlbar-searchbar.inc.css b/browser/themes/shared/urlbar-searchbar.inc.css index 023ea8c06073..bc550056b93d 100644 --- a/browser/themes/shared/urlbar-searchbar.inc.css +++ b/browser/themes/shared/urlbar-searchbar.inc.css @@ -410,9 +410,9 @@ color: var(--urlbar-box-hover-text-color); } -.urlbar-page-action:-moz-focusring { - outline: var(--toolbarbutton-focus-outline); - outline-offset: -2px; +.urlbar-page-action:focus-visible { + outline: var(--focus-outline); + outline-offset: calc(-1 * var(--focus-outline-width)); } #urlbar-go-button, diff --git a/toolkit/themes/shared/notification.css b/toolkit/themes/shared/notification.css index 10c8c14f4c84..55b60f523070 100644 --- a/toolkit/themes/shared/notification.css +++ b/toolkit/themes/shared/notification.css @@ -121,19 +121,20 @@ notification[type="critical"] > hbox > .messageImage { .messageCloseButton > .toolbarbutton-icon { padding: 6px; - /* Override width from close-icon.css */ - width: 32px !important; + width: 32px; /* Close button needs to be clickable from the edge of the window */ margin-inline-end: 8px; } -.messageCloseButton:-moz-focusring { +.messageCloseButton:focus-visible { /* Override the dotted outline from button.css */ outline: none; } -.messageCloseButton:-moz-focusring > .toolbarbutton-icon { - outline: var(--toolbarbutton-focus-outline, 2px solid currentColor); +.messageCloseButton:focus-visible > .toolbarbutton-icon { + outline: var(--focus-outline); + outline-offset: calc(var(--focus-outline-width) * -1); + border-radius: var(--toolbarbutton-border-radius, 4px); } .notification-button { @@ -159,19 +160,14 @@ notification[type="critical"] > hbox > .messageImage { background-color: var(--notification-button-background-active); } -.notification-button:-moz-focusring { - outline: var(--toolbarbutton-focus-outline, 2px solid currentColor); +.notification-button:focus-visible { + outline: var(--focus-outline); + outline-offset: var(--focus-outline-offset); } .notification-button.primary { background-color: var(--notification-primary-button-background); - /* Override button.css hover & active styles */ - color: var(--notification-primary-button-text) !important; -} - -.notification-button.primary:-moz-focusring { - outline: 4px solid rgba(0, 96, 223, 0.5); - outline-offset: -1px; + color: var(--notification-primary-button-text); } .notification-button.primary:not([disabled]):hover { From e4685b149d1cae10dbeb84b459ecaac492c1a29a Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Wed, 2 Mar 2022 05:30:18 +0000 Subject: [PATCH 08/26] Bug 1751110 - Clamp the max size for partial pre-render to nscoord_MAX. r=boris Differential Revision: https://phabricator.services.mozilla.com/D139911 --- layout/base/nsLayoutUtils.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index 0efaeaf8bba7..84018a14cda3 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -9237,7 +9237,9 @@ static nsSize ComputeMaxSizeForPartialPrerender(nsIFrame* aFrame, // so that the result bound's width and height would be pretty much same // as the one rotated by the inverse matrix. result = transform2D.TransformBounds(result); - return nsSize(result.width, result.height); + return nsSize( + result.width < (float)nscoord_MAX ? result.width : nscoord_MAX, + result.height < (float)nscoord_MAX ? result.height : nscoord_MAX); } /* static */ From b17a876d9eaa8a9f16d1aad431161c6883b0f875 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 2 Mar 2022 05:52:45 +0000 Subject: [PATCH 09/26] Bug 1757218 - WebMIDI permission should apply to subdomains. r=mixedpuppy Differential Revision: https://phabricator.services.mozilla.com/D139982 --- dom/base/nsContentUtils.cpp | 7 +-- dom/base/nsContentUtils.h | 3 +- dom/midi/MIDIPermissionRequest.cpp | 41 +++++++------ dom/midi/tests/file_empty.html | 1 + dom/midi/tests/mochitest.ini | 1 + .../tests/test_midi_permission_gated.html | 57 ++++++++++++------- 6 files changed, 67 insertions(+), 43 deletions(-) create mode 100644 dom/midi/tests/file_empty.html diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index 7fb3a07bd2ec..37838e5c5f8d 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -3798,8 +3798,8 @@ bool nsContentUtils::IsExactSitePermDeny(nsIPrincipal* aPrincipal, true); } -bool nsContentUtils::HasExactSitePerm(nsIPrincipal* aPrincipal, - const nsACString& aType) { +bool nsContentUtils::HasSitePerm(nsIPrincipal* aPrincipal, + const nsACString& aType) { if (!aPrincipal) { return false; } @@ -3809,8 +3809,7 @@ bool nsContentUtils::HasExactSitePerm(nsIPrincipal* aPrincipal, NS_ENSURE_TRUE(permMgr, false); uint32_t perm; - nsresult rv = - permMgr->TestExactPermissionFromPrincipal(aPrincipal, aType, &perm); + nsresult rv = permMgr->TestPermissionFromPrincipal(aPrincipal, aType, &perm); NS_ENSURE_SUCCESS(rv, false); return perm != nsIPermissionManager::UNKNOWN_ACTION; diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h index 48d8bd4754fd..486b2a8e6cca 100644 --- a/dom/base/nsContentUtils.h +++ b/dom/base/nsContentUtils.h @@ -899,8 +899,7 @@ class nsContentUtils { const nsACString& aType); // Returns true if the pref exists and is not UNKNOWN_ACTION. - static bool HasExactSitePerm(nsIPrincipal* aPrincipal, - const nsACString& aType); + static bool HasSitePerm(nsIPrincipal* aPrincipal, const nsACString& aType); // Returns true if aDoc1 and aDoc2 have equal NodePrincipal()s. static bool HaveEqualPrincipals(Document* aDoc1, Document* aDoc2); diff --git a/dom/midi/MIDIPermissionRequest.cpp b/dom/midi/MIDIPermissionRequest.cpp index 0f43484baf63..d5efbd50b280 100644 --- a/dom/midi/MIDIPermissionRequest.cpp +++ b/dom/midi/MIDIPermissionRequest.cpp @@ -49,9 +49,9 @@ NS_IMETHODIMP MIDIPermissionRequest::GetTypes(nsIArray** aTypes) { NS_ENSURE_ARG_POINTER(aTypes); nsTArray options; - if (mNeedsSysex) { - options.AppendElement(u"sysex"_ns); - } + // NB: We always request midi-sysex, and the base |midi| permission is unused. + // This could be cleaned up at some point. + options.AppendElement(u"sysex"_ns); return nsContentPermissionUtils::CreatePermissionArray(mType, options, aTypes); } @@ -84,27 +84,36 @@ MIDIPermissionRequest::Run() { return NS_OK; } - // If we already have sysex perms, allow. - if (nsContentUtils::IsExactSitePermAllow(mPrincipal, "midi-sysex"_ns)) { + // Both the spec and our original implementation of WebMIDI have two + // conceptual permission levels: with and without sysex functionality. + // However, our current implementation just has one level, and requires the + // more-powerful |midi-sysex| permission irrespective of the mode requested in + // requestMIDIAccess. + constexpr auto kPermName = "midi-sysex"_ns; + + // First, check for an explicit allow/deny. Note that we want to support + // granting a permission on the base domain and then using it on a subdomain, + // which is why we use the non-"Exact" variants of these APIs. See bug + // 1757218. + if (nsContentUtils::IsSitePermAllow(mPrincipal, kPermName)) { Allow(JS::UndefinedHandleValue); return NS_OK; } - // If midi is addon-gated, the we only pass through to ask permission if - // the permission already exists. This allows for the user possibly changing - // the permission to ask, or deny and having the existing UX properly handle - // the outcome. - if ( -#ifndef RELEASE_OR_BETA - StaticPrefs::dom_webmidi_gated() && -#endif - !nsContentUtils::HasExactSitePerm(mPrincipal, "midi"_ns) && - !nsContentUtils::HasExactSitePerm(mPrincipal, "midi-sysex"_ns)) { + if (nsContentUtils::IsSitePermDeny(mPrincipal, kPermName)) { Cancel(); return NS_OK; } - // If we have no perms, or only have midi and are asking for sysex, pop dialog + // If the add-on is not installed, auto-deny. + if (!nsContentUtils::HasSitePerm(mPrincipal, kPermName)) { + Cancel(); + return NS_OK; + } + + // We can only get here if the add-on is installed, but the user has + // subsequently changed the permission from ALLOW to ASK. In that unusual + // case, throw up a prompt. if (NS_FAILED(nsContentPermissionUtils::AskPermission(this, mWindow))) { Cancel(); return NS_ERROR_FAILURE; diff --git a/dom/midi/tests/file_empty.html b/dom/midi/tests/file_empty.html new file mode 100644 index 000000000000..495c23ec8a81 --- /dev/null +++ b/dom/midi/tests/file_empty.html @@ -0,0 +1 @@ + diff --git a/dom/midi/tests/mochitest.ini b/dom/midi/tests/mochitest.ini index b3545207fc07..52845b74117d 100644 --- a/dom/midi/tests/mochitest.ini +++ b/dom/midi/tests/mochitest.ini @@ -1,6 +1,7 @@ [DEFAULT] support-files = MIDITestUtils.js + file_empty.html scheme = https diff --git a/dom/midi/tests/test_midi_permission_gated.html b/dom/midi/tests/test_midi_permission_gated.html index d8e6fee3cfce..b4f2d8560dd1 100644 --- a/dom/midi/tests/test_midi_permission_gated.html +++ b/dom/midi/tests/test_midi_permission_gated.html @@ -6,6 +6,7 @@ + + diff --git a/layout/base/crashtests/crashtests.list b/layout/base/crashtests/crashtests.list index 0b6e6a10c8e6..a7b61b166a15 100644 --- a/layout/base/crashtests/crashtests.list +++ b/layout/base/crashtests/crashtests.list @@ -564,4 +564,5 @@ load 1729578.html load 1729581.html load 1734007.html load 1745860.html +pref(layout.accessiblecaret.enabled,true) load 1746989.html load 1752649.html diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index 84018a14cda3..41bf1ff8635c 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -2441,11 +2441,12 @@ nsLayoutUtils::TransformResult nsLayoutUtils::TransformRect( 0.5f, std::numeric_limits::max() * devPixelsPerAppUnitFromFrame, std::numeric_limits::max() * devPixelsPerAppUnitFromFrame)); - aRect.x = NSToCoordRound(toDevPixels.x / devPixelsPerAppUnitToFrame); - aRect.y = NSToCoordRound(toDevPixels.y / devPixelsPerAppUnitToFrame); - aRect.width = NSToCoordRound(toDevPixels.width / devPixelsPerAppUnitToFrame); + aRect.x = NSToCoordRoundWithClamp(toDevPixels.x / devPixelsPerAppUnitToFrame); + aRect.y = NSToCoordRoundWithClamp(toDevPixels.y / devPixelsPerAppUnitToFrame); + aRect.width = + NSToCoordRoundWithClamp(toDevPixels.width / devPixelsPerAppUnitToFrame); aRect.height = - NSToCoordRound(toDevPixels.height / devPixelsPerAppUnitToFrame); + NSToCoordRoundWithClamp(toDevPixels.height / devPixelsPerAppUnitToFrame); return TRANSFORM_SUCCEEDED; } From b83969977f79112b0de728288de067934f0859d5 Mon Sep 17 00:00:00 2001 From: Nicolas Chevobbe Date: Wed, 2 Mar 2022 07:00:03 +0000 Subject: [PATCH 11/26] Bug 1755692 - Fix error in `observeActivity` `start` event listener. r=rpl. The event listener callback wasn't bound to this, so the access to `activityErrorMap` was failing. This patch slightly modifies the event listener so we use a fat arrow function, as well as adding the `once` option, since we were removing the event listener the first time the callback was called. Differential Revision: https://phabricator.services.mozilla.com/D138894 --- .../extensions/webrequest/WebRequest.jsm | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/toolkit/components/extensions/webrequest/WebRequest.jsm b/toolkit/components/extensions/webrequest/WebRequest.jsm index bb6cfa403d6e..c62ee962639f 100644 --- a/toolkit/components/extensions/webrequest/WebRequest.jsm +++ b/toolkit/components/extensions/webrequest/WebRequest.jsm @@ -748,17 +748,19 @@ HttpObserverManager = { // errorCheck is called in ChannelWrapper::onStartRequest, we should check // the errorString after onStartRequest to make sure errors have a chance // to be processed before we fall back to a generic error string. - let onStart = function() { - channel.removeEventListener("start", onStart); - if (!channel.errorString) { - this.runChannelListener(channel, "onErrorOccurred", { - error: - this.activityErrorsMap.get(lastActivity) || - `NS_ERROR_NET_UNKNOWN_${lastActivity}`, - }); - } - }; - channel.addEventListener("start", onStart); + channel.addEventListener( + "start", + () => { + if (!channel.errorString) { + this.runChannelListener(channel, "onErrorOccurred", { + error: + this.activityErrorsMap.get(lastActivity) || + `NS_ERROR_NET_UNKNOWN_${lastActivity}`, + }); + } + }, + { once: true } + ); } else if ( lastActivity !== this.GOOD_LAST_ACTIVITY && lastActivity !== From 967543493dccc64f7a695ed29552f9b64b22f280 Mon Sep 17 00:00:00 2001 From: Jens Stutte Date: Wed, 2 Mar 2022 07:49:22 +0000 Subject: [PATCH 12/26] Bug 1750527: Remove MOZ_CRASH_UNLESS_FUZZING and give IPC_FAIL a reason. r=jesup Differential Revision: https://phabricator.services.mozilla.com/D139226 --- dom/localstorage/ActorsChild.cpp | 2 +- dom/localstorage/ActorsParent.cpp | 84 +++++++++++++------------------ 2 files changed, 36 insertions(+), 50 deletions(-) diff --git a/dom/localstorage/ActorsChild.cpp b/dom/localstorage/ActorsChild.cpp index 24db4d1db707..1626b16a72ee 100644 --- a/dom/localstorage/ActorsChild.cpp +++ b/dom/localstorage/ActorsChild.cpp @@ -141,7 +141,7 @@ mozilla::ipc::IPCResult LSObserverChild::RecvObserve( QM_TRY_INSPECT(const auto& principal, PrincipalInfoToPrincipal(aPrincipalInfo), - IPC_FAIL_NO_REASON(this)); + IPC_FAIL(this, "PrincipalInfoToPrincipal failed!")); Storage::NotifyChange(/* aStorage */ nullptr, principal, aKey, aOldValue.AsString(), aNewValue.AsString(), diff --git a/dom/localstorage/ActorsParent.cpp b/dom/localstorage/ActorsParent.cpp index d0d85c8feef0..bd4eabd07bc3 100644 --- a/dom/localstorage/ActorsParent.cpp +++ b/dom/localstorage/ActorsParent.cpp @@ -3221,13 +3221,13 @@ PBackgroundLSDatabaseParent* AllocPBackgroundLSDatabaseParent( } if (NS_WARN_IF(!gPreparedDatastores)) { - MOZ_CRASH_UNLESS_FUZZING(); + MOZ_ASSERT_UNLESS_FUZZING(false); return nullptr; } PreparedDatastore* preparedDatastore = gPreparedDatastores->Get(aDatastoreId); if (NS_WARN_IF(!preparedDatastore)) { - MOZ_CRASH_UNLESS_FUZZING(); + MOZ_ASSERT_UNLESS_FUZZING(false); return nullptr; } @@ -3296,13 +3296,13 @@ PBackgroundLSObserverParent* AllocPBackgroundLSObserverParent( } if (NS_WARN_IF(!gPreparedObsevers)) { - MOZ_CRASH_UNLESS_FUZZING(); + MOZ_ASSERT_UNLESS_FUZZING(false); return nullptr; } RefPtr observer = gPreparedObsevers->Get(aObserverId); if (NS_WARN_IF(!observer)) { - MOZ_CRASH_UNLESS_FUZZING(); + MOZ_ASSERT_UNLESS_FUZZING(false); return nullptr; } @@ -5495,7 +5495,7 @@ mozilla::ipc::IPCResult Database::RecvDeleteMe() { IProtocol* mgr = Manager(); if (!PBackgroundLSDatabaseParent::Send__delete__(this)) { - return IPC_FAIL_NO_REASON(mgr); + return IPC_FAIL(mgr, "Send__delete__ failed!"); } return IPC_OK(); } @@ -5504,8 +5504,7 @@ mozilla::ipc::IPCResult Database::RecvAllowToClose() { AssertIsOnBackgroundThread(); if (NS_WARN_IF(mAllowedToClose)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "mAllowedToClose already set!"); } AllowToClose(); @@ -5520,12 +5519,12 @@ PBackgroundLSSnapshotParent* Database::AllocPBackgroundLSSnapshotParent( AssertIsOnBackgroundThread(); if (NS_WARN_IF(aIncreasePeakUsage && aMinSize < 0)) { - MOZ_CRASH_UNLESS_FUZZING(); + MOZ_ASSERT_UNLESS_FUZZING(false); return nullptr; } if (NS_WARN_IF(mAllowedToClose)) { - MOZ_CRASH_UNLESS_FUZZING(); + MOZ_ASSERT_UNLESS_FUZZING(false); return nullptr; } @@ -5692,7 +5691,7 @@ mozilla::ipc::IPCResult Snapshot::RecvDeleteMe() { IProtocol* mgr = Manager(); if (!PBackgroundLSSnapshotParent::Send__delete__(this)) { - return IPC_FAIL_NO_REASON(mgr); + return IPC_FAIL(mgr, "Send__delete__ failed!"); } return IPC_OK(); } @@ -5705,13 +5704,11 @@ mozilla::ipc::IPCResult Snapshot::RecvAsyncCheckpoint( MOZ_ASSERT(mPeakUsage >= mUsage); if (NS_WARN_IF(aWriteInfos.IsEmpty())) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "aWriteInfos is empty!"); } if (NS_WARN_IF(mHasOtherProcessObservers)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "mHasOtherProcessObservers already set!"); } mDatastore->BeginUpdateBatch(mUsage); @@ -5760,13 +5757,11 @@ mozilla::ipc::IPCResult Snapshot::RecvAsyncCheckpointAndNotify( MOZ_ASSERT(mPeakUsage >= mUsage); if (NS_WARN_IF(aWriteAndNotifyInfos.IsEmpty())) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "aWriteAndNotifyInfos is empty!"); } if (NS_WARN_IF(!mHasOtherProcessObservers)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "mHasOtherProcessObservers is not set!"); } mDatastore->BeginUpdateBatch(mUsage); @@ -5825,7 +5820,7 @@ mozilla::ipc::IPCResult Snapshot::RecvAsyncFinish() { AssertIsOnBackgroundThread(); if (NS_WARN_IF(mFinishReceived)) { - MOZ_CRASH_UNLESS_FUZZING(); + MOZ_ASSERT_UNLESS_FUZZING(false); return IPC_FAIL(this, "Already finished"); } @@ -5838,7 +5833,7 @@ mozilla::ipc::IPCResult Snapshot::RecvSyncFinish() { AssertIsOnBackgroundThread(); if (NS_WARN_IF(mFinishReceived)) { - MOZ_CRASH_UNLESS_FUZZING(); + MOZ_ASSERT_UNLESS_FUZZING(false); return IPC_FAIL(this, "Already finished"); } @@ -5851,23 +5846,19 @@ mozilla::ipc::IPCResult Snapshot::RecvLoaded() { AssertIsOnBackgroundThread(); if (NS_WARN_IF(mFinishReceived)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "mFinishReceived already set!"); } if (NS_WARN_IF(mLoadedReceived)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "mLoadedReceived already set!"); } if (NS_WARN_IF(mLoadedAllItems)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "mLoadedAllItems already set!"); } if (NS_WARN_IF(mLoadKeysReceived)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "mLoadKeysReceived already set!"); } mLoadedReceived = true; @@ -5890,23 +5881,23 @@ mozilla::ipc::IPCResult Snapshot::RecvLoadValueAndMoreItems( MOZ_ASSERT(mDatastore); if (NS_WARN_IF(mFinishReceived)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "mFinishReceived already set!"); } if (NS_WARN_IF(mLoadedReceived)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "mLoadedReceived already set!"); } if (NS_WARN_IF(mLoadedAllItems)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "mLoadedAllItems already set!"); } - if (mLoadedItems.Contains(aKey) || mUnknownItems.Contains(aKey)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + if (mLoadedItems.Contains(aKey)) { + return IPC_FAIL(this, "mLoadedItems already contains aKey!"); + } + + if (mUnknownItems.Contains(aKey)) { + return IPC_FAIL(this, "mUnknownItems already contains aKey!"); } if (auto entry = mValues.Lookup(aKey)) { @@ -6031,18 +6022,15 @@ mozilla::ipc::IPCResult Snapshot::RecvLoadKeys(nsTArray* aKeys) { MOZ_ASSERT(mDatastore); if (NS_WARN_IF(mFinishReceived)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "mFinishReceived already set!"); } if (NS_WARN_IF(mLoadedReceived)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "mLoadedReceived already set!"); } if (NS_WARN_IF(mLoadKeysReceived)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "mLoadKeysReceived already set!"); } mLoadKeysReceived = true; @@ -6062,13 +6050,11 @@ mozilla::ipc::IPCResult Snapshot::RecvIncreasePeakUsage(const int64_t& aMinSize, MOZ_ASSERT(aSize); if (NS_WARN_IF(aMinSize <= 0)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "aMinSize not valid!"); } if (NS_WARN_IF(mFinishReceived)) { - MOZ_CRASH_UNLESS_FUZZING(); - return IPC_FAIL_NO_REASON(this); + return IPC_FAIL(this, "mFinishReceived already set!"); } int64_t size = @@ -6136,7 +6122,7 @@ mozilla::ipc::IPCResult Observer::RecvDeleteMe() { IProtocol* mgr = Manager(); if (!PBackgroundLSObserverParent::Send__delete__(this)) { - return IPC_FAIL_NO_REASON(mgr); + return IPC_FAIL(mgr, "Send__delete__ failed!"); } return IPC_OK(); } @@ -6510,7 +6496,7 @@ mozilla::ipc::IPCResult LSRequestBase::RecvCancel() { IProtocol* mgr = Manager(); if (!PBackgroundLSRequestParent::Send__delete__(this, NS_ERROR_FAILURE)) { - return IPC_FAIL_NO_REASON(mgr); + return IPC_FAIL(mgr, "Send__delete__ failed!"); } return IPC_OK(); From 24a866651843435d0117efabc8d01a631aae5907 Mon Sep 17 00:00:00 2001 From: William Durand Date: Wed, 2 Mar 2022 08:49:37 +0000 Subject: [PATCH 13/26] Bug 1756495 - Ensure script registration is complete when a new process is spawned during the registration. r=robwu Differential Revision: https://phabricator.services.mozilla.com/D139503 --- .../extensions/parent/ext-contentScripts.js | 6 +- .../extensions/parent/ext-scripting.js | 23 +-- .../extensions/parent/ext-userScripts.js | 6 +- .../test_ext_scripting_contentScripts.html | 13 +- ..._ext_contentscript_dynamic_registration.js | 184 ++++++++++++++++++ .../test/xpcshell/xpcshell-common.ini | 1 + 6 files changed, 204 insertions(+), 29 deletions(-) create mode 100644 toolkit/components/extensions/test/xpcshell/test_ext_contentscript_dynamic_registration.js diff --git a/toolkit/components/extensions/parent/ext-contentScripts.js b/toolkit/components/extensions/parent/ext-contentScripts.js index 16b99f0d6c03..902f925a7636 100644 --- a/toolkit/components/extensions/parent/ext-contentScripts.js +++ b/toolkit/components/extensions/parent/ext-contentScripts.js @@ -195,14 +195,14 @@ this.contentScripts = class extends ExtensionAPI { const scriptOptions = contentScript.serialize(); + extension.registeredContentScripts.set(scriptId, scriptOptions); + extension.updateContentScripts(); + await extension.broadcast("Extension:RegisterContentScripts", { id: extension.id, scripts: [{ scriptId, options: scriptOptions }], }); - extension.registeredContentScripts.set(scriptId, scriptOptions); - extension.updateContentScripts(); - return scriptId; }, diff --git a/toolkit/components/extensions/parent/ext-scripting.js b/toolkit/components/extensions/parent/ext-scripting.js index ba8bd4c25245..71b65fea4f38 100644 --- a/toolkit/components/extensions/parent/ext-scripting.js +++ b/toolkit/components/extensions/parent/ext-scripting.js @@ -215,22 +215,16 @@ this.scripting = class extends ExtensionAPI { scriptsToRegister.set(script.id, makeInternalContentScript(script)); } - for (const [id, { scriptId }] of scriptsToRegister.entries()) { + for (const [id, { scriptId, options }] of scriptsToRegister) { scriptIdsMap.set(id, scriptId); + extension.registeredContentScripts.set(scriptId, options); } + extension.updateContentScripts(); await extension.broadcast("Extension:RegisterContentScripts", { id: extension.id, scripts: Array.from(scriptsToRegister.values()), }); - - for (const { scriptId, options } of scriptsToRegister.values()) { - extension.registeredContentScripts.set(scriptId, options); - } - - // TODO: Bug 1756495 - Registration may be incomplete when a new - // process spawns during the registration. - extension.updateContentScripts(); }, getRegisteredContentScripts: async details => { @@ -244,14 +238,6 @@ this.scripting = class extends ExtensionAPI { .map(([id, scriptId]) => { const options = extension.registeredContentScripts.get(scriptId); - if (!options) { - // When we call `getRegisteredContentScripts()` during a registration, - // `options` might be `undefined`. This happens when `scriptIdsMap` - // is already updated but `extension.registeredContentScripts` is not - // yet due to the broadcast. - return; - } - return { id, allFrames: options.allFrames, @@ -262,8 +248,7 @@ this.scripting = class extends ExtensionAPI { matches: options.matches, runAt: options.runAt, }; - }) - .filter(script => script); + }); }, }, }; diff --git a/toolkit/components/extensions/parent/ext-userScripts.js b/toolkit/components/extensions/parent/ext-userScripts.js index 70541fb5643c..1e315d6aeb90 100644 --- a/toolkit/components/extensions/parent/ext-userScripts.js +++ b/toolkit/components/extensions/parent/ext-userScripts.js @@ -127,14 +127,14 @@ this.userScripts = class extends ExtensionAPI { const scriptOptions = userScript.serialize(); + extension.registeredContentScripts.set(scriptId, scriptOptions); + extension.updateContentScripts(); + await extension.broadcast("Extension:RegisterContentScripts", { id: extension.id, scripts: [{ scriptId, options: scriptOptions }], }); - extension.registeredContentScripts.set(scriptId, scriptOptions); - extension.updateContentScripts(); - return scriptId; }, diff --git a/toolkit/components/extensions/test/mochitest/test_ext_scripting_contentScripts.html b/toolkit/components/extensions/test/mochitest/test_ext_scripting_contentScripts.html index 8c15f1c86442..fdf0ae1cdf18 100644 --- a/toolkit/components/extensions/test/mochitest/test_ext_scripting_contentScripts.html +++ b/toolkit/components/extensions/test/mochitest/test_ext_scripting_contentScripts.html @@ -782,11 +782,16 @@ add_task(async function test_getRegisteredContentScripts_during_a_registration() ]); const scripts = await browser.scripting.getRegisteredContentScripts(); - // This test covers the case where `options` is `undefined` in the - // `getRegisteredContentScripts()` implementation because we call this - // method during a registration that is not yet complete. browser.test.assertEq( - "[]", + JSON.stringify([ + { + id: "a-script", + allFrames: false, + js: ["script.js"], + matches: ["*://test1.example.com/*"], + runAt: "document_idle", + }, + ]), JSON.stringify(scripts), "expected empty result array" ); diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_dynamic_registration.js b/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_dynamic_registration.js new file mode 100644 index 000000000000..026cc2cb2c45 --- /dev/null +++ b/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_dynamic_registration.js @@ -0,0 +1,184 @@ +"use strict"; + +const server = createHttpServer(); +server.registerDirectory("/data/", do_get_file("data")); + +const BASE_URL = `http://localhost:${server.identity.primaryPort}/data`; + +// ExtensionContent.jsm needs to know when it's running from xpcshell, to use +// the right timeout for content scripts executed at document_idle. +ExtensionTestUtils.mockAppInfo(); + +// Do not use preallocated processes. +Services.prefs.setBoolPref("dom.ipc.processPrelaunch.enabled", false); + +const makeExtension = ({ background, manifest }) => { + return ExtensionTestUtils.loadExtension({ + manifest: { + ...manifest, + permissions: + manifest.manifest_version === 3 ? ["scripting"] : ["http://*/*/*.html"], + }, + background, + files: { + "script.js": () => { + browser.test.sendMessage( + `script-ran: ${location.pathname.split("/").pop()}` + ); + }, + "inject_browser.js": () => { + browser.userScripts.onBeforeScript.addListener(script => { + // Inject `browser.test.sendMessage()` so that it can be used in the + // `script.js` defined above when using "user scripts". + script.defineGlobals({ + browser: { + test: { + sendMessage(msg) { + browser.test.sendMessage(msg); + }, + }, + }, + }); + }); + }, + }, + }); +}; + +const verifyRegistrationWithNewProcess = async extension => { + // We override the `broadcast()` method to reliably verify Bug 1756495: when + // a new process is spawned while we register a content script, the script + // should be correctly registered and executed in this new process. Below, + // when we receive the `Extension:RegisterContentScripts`, we open a new tab + // (which is the "new process") and then we invoke the original "broadcast + // logic". The expected result is that the content script registered by the + // extension will run. + const originalBroadcast = Extension.prototype.broadcast; + + let broadcastCalledCount = 0; + let secondContentPage; + + extension.extension.broadcast = async function broadcast(msg, data) { + if (msg !== "Extension:RegisterContentScripts") { + return originalBroadcast.call(this, msg, data); + } + + broadcastCalledCount++; + Assert.equal( + 1, + broadcastCalledCount, + "broadcast override should be called once" + ); + + await originalBroadcast.call(this, msg, data); + + Assert.equal(extension.id, data.id, "got expected extension ID"); + Assert.equal(1, data.scripts.length, "expected 1 script to register"); + Assert.ok( + data.scripts[0].options.jsPaths[0].endsWith("script.js"), + "got expected js file" + ); + + // Collect the existing process IDs. + let existingProcessIDs = ChromeUtils.getAllDOMProcesses().map( + p => p.childID + ); + + secondContentPage = await ExtensionTestUtils.loadContentPage( + `${BASE_URL}/dummy_page.html` + ); + + const { + childID: expectedPID, + } = secondContentPage.browsingContext.currentWindowGlobal.domProcess; + + // We expect to have a new process created for `secondContentPage`. + Assert.ok( + !existingProcessIDs.includes(expectedPID), + `expected PID ${expectedPID} to not be in [${existingProcessIDs.join( + ", " + )}])` + ); + }; + + await extension.startup(); + await extension.awaitMessage("background-done"); + + let contentPage = await ExtensionTestUtils.loadContentPage( + `${BASE_URL}/file_sample.html` + ); + + await Promise.all([ + extension.awaitMessage("script-ran: file_sample.html"), + extension.awaitMessage("script-ran: dummy_page.html"), + ]); + + await contentPage.close(); + await secondContentPage.close(); + await extension.unload(); +}; + +add_task( + { + pref_set: [["extensions.manifestV3.enabled", true]], + }, + async function test_scripting_registerContentScripts() { + let extension = makeExtension({ + manifest: { + manifest_version: 3, + }, + async background() { + const script = { + id: "a-script", + js: ["script.js"], + matches: ["http://*/*/*.html"], + }; + + await browser.scripting.registerContentScripts([script]); + + browser.test.sendMessage("background-done"); + }, + }); + + await verifyRegistrationWithNewProcess(extension); + } +); + +add_task(async function test_contentScripts_register() { + let extension = makeExtension({ + manifest: { + manifest_version: 2, + }, + async background() { + await browser.contentScripts.register({ + js: [{ file: "script.js" }], + matches: ["http://*/*/*.html"], + }); + + browser.test.sendMessage("background-done"); + }, + }); + + await verifyRegistrationWithNewProcess(extension); +}); + +add_task(async function test_userScripts_register() { + let extension = makeExtension({ + manifest: { + manifest_version: 2, + user_scripts: { + api_script: "inject_browser.js", + }, + }, + async background() { + await browser.userScripts.register({ + js: [{ file: "script.js" }], + matches: ["http://*/*/*.html"], + }); + + browser.test.sendMessage("background-done"); + }, + }); + + await verifyRegistrationWithNewProcess(extension); +}); diff --git a/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini b/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini index ed8a0967520b..655a8b821833 100644 --- a/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini +++ b/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini @@ -70,6 +70,7 @@ skip-if = [test_ext_contentscript_create_iframe.js] [test_ext_contentscript_csp.js] [test_ext_contentscript_css.js] +[test_ext_contentscript_dynamic_registration.js] [test_ext_contentscript_exporthelpers.js] [test_ext_contentscript_in_background.js] skip-if = os == "android" # Bug 1700482 From 8adcbead7263eb6d5391b5dbb61c04b44289e9fb Mon Sep 17 00:00:00 2001 From: Norisz Fay Date: Wed, 2 Mar 2022 11:25:53 +0200 Subject: [PATCH 14/26] Backed out changeset 1ea661280632 (bug 1757218) for causing mochitest failures on test_midi_permission_gated.html --- dom/base/nsContentUtils.cpp | 7 ++- dom/base/nsContentUtils.h | 3 +- dom/midi/MIDIPermissionRequest.cpp | 41 ++++++------- dom/midi/tests/file_empty.html | 1 - dom/midi/tests/mochitest.ini | 1 - .../tests/test_midi_permission_gated.html | 57 +++++++------------ 6 files changed, 43 insertions(+), 67 deletions(-) delete mode 100644 dom/midi/tests/file_empty.html diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index 37838e5c5f8d..7fb3a07bd2ec 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -3798,8 +3798,8 @@ bool nsContentUtils::IsExactSitePermDeny(nsIPrincipal* aPrincipal, true); } -bool nsContentUtils::HasSitePerm(nsIPrincipal* aPrincipal, - const nsACString& aType) { +bool nsContentUtils::HasExactSitePerm(nsIPrincipal* aPrincipal, + const nsACString& aType) { if (!aPrincipal) { return false; } @@ -3809,7 +3809,8 @@ bool nsContentUtils::HasSitePerm(nsIPrincipal* aPrincipal, NS_ENSURE_TRUE(permMgr, false); uint32_t perm; - nsresult rv = permMgr->TestPermissionFromPrincipal(aPrincipal, aType, &perm); + nsresult rv = + permMgr->TestExactPermissionFromPrincipal(aPrincipal, aType, &perm); NS_ENSURE_SUCCESS(rv, false); return perm != nsIPermissionManager::UNKNOWN_ACTION; diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h index 486b2a8e6cca..48d8bd4754fd 100644 --- a/dom/base/nsContentUtils.h +++ b/dom/base/nsContentUtils.h @@ -899,7 +899,8 @@ class nsContentUtils { const nsACString& aType); // Returns true if the pref exists and is not UNKNOWN_ACTION. - static bool HasSitePerm(nsIPrincipal* aPrincipal, const nsACString& aType); + static bool HasExactSitePerm(nsIPrincipal* aPrincipal, + const nsACString& aType); // Returns true if aDoc1 and aDoc2 have equal NodePrincipal()s. static bool HaveEqualPrincipals(Document* aDoc1, Document* aDoc2); diff --git a/dom/midi/MIDIPermissionRequest.cpp b/dom/midi/MIDIPermissionRequest.cpp index d5efbd50b280..0f43484baf63 100644 --- a/dom/midi/MIDIPermissionRequest.cpp +++ b/dom/midi/MIDIPermissionRequest.cpp @@ -49,9 +49,9 @@ NS_IMETHODIMP MIDIPermissionRequest::GetTypes(nsIArray** aTypes) { NS_ENSURE_ARG_POINTER(aTypes); nsTArray options; - // NB: We always request midi-sysex, and the base |midi| permission is unused. - // This could be cleaned up at some point. - options.AppendElement(u"sysex"_ns); + if (mNeedsSysex) { + options.AppendElement(u"sysex"_ns); + } return nsContentPermissionUtils::CreatePermissionArray(mType, options, aTypes); } @@ -84,36 +84,27 @@ MIDIPermissionRequest::Run() { return NS_OK; } - // Both the spec and our original implementation of WebMIDI have two - // conceptual permission levels: with and without sysex functionality. - // However, our current implementation just has one level, and requires the - // more-powerful |midi-sysex| permission irrespective of the mode requested in - // requestMIDIAccess. - constexpr auto kPermName = "midi-sysex"_ns; - - // First, check for an explicit allow/deny. Note that we want to support - // granting a permission on the base domain and then using it on a subdomain, - // which is why we use the non-"Exact" variants of these APIs. See bug - // 1757218. - if (nsContentUtils::IsSitePermAllow(mPrincipal, kPermName)) { + // If we already have sysex perms, allow. + if (nsContentUtils::IsExactSitePermAllow(mPrincipal, "midi-sysex"_ns)) { Allow(JS::UndefinedHandleValue); return NS_OK; } - if (nsContentUtils::IsSitePermDeny(mPrincipal, kPermName)) { + // If midi is addon-gated, the we only pass through to ask permission if + // the permission already exists. This allows for the user possibly changing + // the permission to ask, or deny and having the existing UX properly handle + // the outcome. + if ( +#ifndef RELEASE_OR_BETA + StaticPrefs::dom_webmidi_gated() && +#endif + !nsContentUtils::HasExactSitePerm(mPrincipal, "midi"_ns) && + !nsContentUtils::HasExactSitePerm(mPrincipal, "midi-sysex"_ns)) { Cancel(); return NS_OK; } - // If the add-on is not installed, auto-deny. - if (!nsContentUtils::HasSitePerm(mPrincipal, kPermName)) { - Cancel(); - return NS_OK; - } - - // We can only get here if the add-on is installed, but the user has - // subsequently changed the permission from ALLOW to ASK. In that unusual - // case, throw up a prompt. + // If we have no perms, or only have midi and are asking for sysex, pop dialog if (NS_FAILED(nsContentPermissionUtils::AskPermission(this, mWindow))) { Cancel(); return NS_ERROR_FAILURE; diff --git a/dom/midi/tests/file_empty.html b/dom/midi/tests/file_empty.html deleted file mode 100644 index 495c23ec8a81..000000000000 --- a/dom/midi/tests/file_empty.html +++ /dev/null @@ -1 +0,0 @@ - diff --git a/dom/midi/tests/mochitest.ini b/dom/midi/tests/mochitest.ini index 52845b74117d..b3545207fc07 100644 --- a/dom/midi/tests/mochitest.ini +++ b/dom/midi/tests/mochitest.ini @@ -1,7 +1,6 @@ [DEFAULT] support-files = MIDITestUtils.js - file_empty.html scheme = https diff --git a/dom/midi/tests/test_midi_permission_gated.html b/dom/midi/tests/test_midi_permission_gated.html index b4f2d8560dd1..d8e6fee3cfce 100644 --- a/dom/midi/tests/test_midi_permission_gated.html +++ b/dom/midi/tests/test_midi_permission_gated.html @@ -6,7 +6,6 @@ -