From dab9b6216cfbb63ec438b51b612d9a97596eda16 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Wed, 7 Feb 2018 07:27:27 -0500 Subject: [PATCH 01/20] Bug 1383682 - Part 1. Split off imgRequestProxy notification deferrals for validation. r=tnikkel When cache validation is in progress, imgRequestProxy defers its notifications to its listener until the validation is complete. This is because the cache may be discarded, and the current state will change. It attempted to share the same flags with notification deferrals used by ProgressTracker to indicate that there is a pending notification, but this has problematic/confusing. Hence this patch creates dedicated flags for notification deferrals due to cache validation. --- image/ProgressTracker.cpp | 10 +++++ image/imgLoader.cpp | 73 ++++++++++++++++++------------- image/imgLoader.h | 3 +- image/imgRequestProxy.cpp | 92 ++++++++++++++++++++++++++------------- image/imgRequestProxy.h | 14 +++++- 5 files changed, 127 insertions(+), 65 deletions(-) diff --git a/image/ProgressTracker.cpp b/image/ProgressTracker.cpp index 0805d22e92eb..d34e96d6ec5a 100644 --- a/image/ProgressTracker.cpp +++ b/image/ProgressTracker.cpp @@ -171,6 +171,11 @@ ProgressTracker::Notify(IProgressObserver* aObserver) { MOZ_ASSERT(NS_IsMainThread()); + if (aObserver->NotificationsDeferred()) { + // There is a pending notification, or the observer isn't ready yet. + return; + } + if (MOZ_LOG_TEST(gImgLog, LogLevel::Debug)) { RefPtr image = GetImage(); if (image && image->GetURI()) { @@ -241,6 +246,11 @@ ProgressTracker::NotifyCurrentState(IProgressObserver* aObserver) { MOZ_ASSERT(NS_IsMainThread()); + if (aObserver->NotificationsDeferred()) { + // There is a pending notification, or the observer isn't ready yet. + return; + } + if (MOZ_LOG_TEST(gImgLog, LogLevel::Debug)) { RefPtr image = GetImage(); nsAutoCString spec; diff --git a/image/imgLoader.cpp b/image/imgLoader.cpp index d766a6706a63..4632974ef609 100644 --- a/image/imgLoader.cpp +++ b/image/imgLoader.cpp @@ -1768,7 +1768,7 @@ imgLoader::ValidateRequestWithNewChannel(imgRequest* request, // In the mean time, we must defer notifications because we are added to // the imgRequest's proxy list, and we can get extra notifications // resulting from methods such as StartDecoding(). See bug 579122. - proxy->SetNotificationsDeferred(true); + proxy->MarkValidating(); // Attach the proxy without notifying request->GetValidator()->AddProxy(proxy); @@ -1834,7 +1834,7 @@ imgLoader::ValidateRequestWithNewChannel(imgRequest* request, // In the mean time, we must defer notifications because we are added to // the imgRequest's proxy list, and we can get extra notifications // resulting from methods such as StartDecoding(). See bug 579122. - req->SetNotificationsDeferred(true); + req->MarkValidating(); // Add the proxy without notifying hvc->AddProxy(req); @@ -2936,13 +2936,45 @@ imgCacheValidator::AddProxy(imgRequestProxy* aProxy) // the network. aProxy->AddToLoadGroup(); - mProxies.AppendObject(aProxy); + mProxies.AppendElement(aProxy); } void imgCacheValidator::RemoveProxy(imgRequestProxy* aProxy) { - mProxies.RemoveObject(aProxy); + mProxies.RemoveElement(aProxy); +} + +void +imgCacheValidator::UpdateProxies() +{ + // We have finished validating the request, so we can safely take ownership + // of the proxy list. imgRequestProxy::SyncNotifyListener can mutate the list + // if imgRequestProxy::CancelAndForgetObserver is called by its owner. Note + // that any potential notifications should still be suppressed in + // imgRequestProxy::ChangeOwner because we haven't cleared the validating + // flag yet, and thus they will remain deferred. + AutoTArray, 4> proxies(Move(mProxies)); + + for (auto& proxy : proxies) { + // First update the state of all proxies before notifying any of them + // to ensure a consistent state (e.g. in case the notification causes + // other proxies to be touched indirectly.) + MOZ_ASSERT(proxy->IsValidating()); + MOZ_ASSERT(proxy->NotificationsDeferred(), + "Proxies waiting on cache validation should be " + "deferring notifications!"); + if (mNewRequest) { + proxy->ChangeOwner(mNewRequest); + } + proxy->ClearValidating(); + } + + for (auto& proxy : proxies) { + // Notify synchronously, because we're already in OnStartRequest, an + // asynchronously-called function. + proxy->SyncNotifyListener(); + } } /** nsIRequestObserver methods **/ @@ -2982,25 +3014,11 @@ imgCacheValidator::OnStartRequest(nsIRequest* aRequest, nsISupports* ctxt) } if (isFromCache && sameURI) { - uint32_t count = mProxies.Count(); - for (int32_t i = count-1; i>=0; i--) { - imgRequestProxy* proxy = static_cast(mProxies[i]); - - // Proxies waiting on cache validation should be deferring - // notifications. Undefer them. - MOZ_ASSERT(proxy->NotificationsDeferred(), - "Proxies waiting on cache validation should be " - "deferring notifications!"); - proxy->SetNotificationsDeferred(false); - - // Notify synchronously, because we're already in OnStartRequest, an - // asynchronously-called function. - proxy->SyncNotifyListener(); - } - // We don't need to load this any more. aRequest->Cancel(NS_BINDING_ABORTED); + // Clear the validator before updating the proxies. The notifications may + // clone an existing request, and its state could be inconsistent. mRequest->SetLoadId(context); mRequest->SetValidator(nullptr); @@ -3009,6 +3027,7 @@ imgCacheValidator::OnStartRequest(nsIRequest* aRequest, nsISupports* ctxt) mNewRequest = nullptr; mNewEntry = nullptr; + UpdateProxies(); return NS_OK; } } @@ -3035,6 +3054,8 @@ imgCacheValidator::OnStartRequest(nsIRequest* aRequest, nsISupports* ctxt) // Doom the old request's cache entry mRequest->RemoveFromCache(); + // Clear the validator before updating the proxies. The notifications may + // clone an existing request, and its state could be inconsistent. mRequest->SetValidator(nullptr); mRequest = nullptr; @@ -3055,17 +3076,7 @@ imgCacheValidator::OnStartRequest(nsIRequest* aRequest, nsISupports* ctxt) // changes the caching behaviour for imgRequests. mImgLoader->PutIntoCache(mNewRequest->CacheKey(), mNewEntry); - uint32_t count = mProxies.Count(); - for (int32_t i = count-1; i>=0; i--) { - imgRequestProxy* proxy = static_cast(mProxies[i]); - proxy->ChangeOwner(mNewRequest); - - // Notify synchronously, because we're already in OnStartRequest, an - // asynchronously-called function. - proxy->SetNotificationsDeferred(false); - proxy->SyncNotifyListener(); - } - + UpdateProxies(); mNewRequest = nullptr; mNewEntry = nullptr; diff --git a/image/imgLoader.h b/image/imgLoader.h index 24130b25e249..b0b4d84ae46f 100644 --- a/image/imgLoader.h +++ b/image/imgLoader.h @@ -568,6 +568,7 @@ public: NS_DECL_NSIASYNCVERIFYREDIRECTCALLBACK private: + void UpdateProxies(); virtual ~imgCacheValidator(); nsCOMPtr mDestListener; @@ -576,7 +577,7 @@ private: nsCOMPtr mRedirectChannel; RefPtr mRequest; - nsCOMArray mProxies; + AutoTArray, 4> mProxies; RefPtr mNewRequest; RefPtr mNewEntry; diff --git a/image/imgRequestProxy.cpp b/image/imgRequestProxy.cpp index 6edda1d18582..f0c278955872 100644 --- a/image/imgRequestProxy.cpp +++ b/image/imgRequestProxy.cpp @@ -121,6 +121,7 @@ imgRequestProxy::imgRequestProxy() : mListenerIsStrongRef(false), mDecodeRequested(false), mDeferNotifications(false), + mValidating(false), mHadListener(false), mHadDispatch(false) { @@ -163,15 +164,13 @@ imgRequestProxy::~imgRequestProxy() // above assert. NullOutListener(); - if (GetOwner()) { - /* Call RemoveProxy with a successful status. This will keep the - channel, if still downloading data, from being canceled if 'this' is - the last observer. This allows the image to continue to download and - be cached even if no one is using it currently. - */ - mCanceled = true; - GetOwner()->RemoveProxy(this, NS_OK); - } + /* Call RemoveProxy with a successful status. This will keep the + channel, if still downloading data, from being canceled if 'this' is + the last observer. This allows the image to continue to download and + be cached even if no one is using it currently. + */ + mCanceled = true; + RemoveFromOwner(NS_OK); RemoveFromLoadGroup(); LOG_FUNC(gImgLog, "imgRequestProxy::~imgRequestProxy"); @@ -237,6 +236,7 @@ imgRequestProxy::ChangeOwner(imgRequest* aNewOwner) GetOwner()->RemoveProxy(this, NS_OK); mBehaviour->SetOwner(aNewOwner); + MOZ_ASSERT(!GetValidator(), "New owner cannot be validating!"); // If we were locked, apply the locks here for (uint32_t i = 0; i < oldLockCount; i++) { @@ -251,14 +251,29 @@ imgRequestProxy::ChangeOwner(imgRequest* aNewOwner) } AddToOwner(nullptr); + return NS_OK; +} + +void +imgRequestProxy::MarkValidating() +{ + MOZ_ASSERT(GetValidator()); + mValidating = true; +} + +void +imgRequestProxy::ClearValidating() +{ + MOZ_ASSERT(mValidating); + MOZ_ASSERT(!GetValidator()); + mValidating = false; // If we'd previously requested a synchronous decode, request a decode on the // new image. if (mDecodeRequested) { + mDecodeRequested = false; StartDecoding(imgIContainer::FLAG_NONE); } - - return NS_OK; } bool @@ -351,11 +366,28 @@ imgRequestProxy::AddToOwner(nsIDocument* aLoadingDocument) mEventTarget = do_GetMainThread(); } - if (!GetOwner()) { + imgRequest* owner = GetOwner(); + if (!owner) { return; } - GetOwner()->AddProxy(this); + owner->AddProxy(this); +} + +void +imgRequestProxy::RemoveFromOwner(nsresult aStatus) +{ + imgRequest* owner = GetOwner(); + if (owner) { + if (mValidating) { + imgCacheValidator* validator = owner->GetValidator(); + MOZ_ASSERT(validator); + validator->RemoveProxy(this); + mValidating = false; + } + + owner->RemoveProxy(this, aStatus); + } } void @@ -494,10 +526,7 @@ imgRequestProxy::Cancel(nsresult status) void imgRequestProxy::DoCancel(nsresult status) { - if (GetOwner()) { - GetOwner()->RemoveProxy(this, status); - } - + RemoveFromOwner(status); RemoveFromLoadGroup(); NullOutListener(); } @@ -519,17 +548,7 @@ imgRequestProxy::CancelAndForgetObserver(nsresult aStatus) mCanceled = true; mForceDispatchLoadGroup = true; - - imgRequest* owner = GetOwner(); - if (owner) { - imgCacheValidator* validator = owner->GetValidator(); - if (validator) { - validator->RemoveProxy(this); - } - - owner->RemoveProxy(this, aStatus); - } - + RemoveFromOwner(aStatus); RemoveFromLoadGroup(); mForceDispatchLoadGroup = false; @@ -859,15 +878,16 @@ imgRequestProxy::PerformClone(imgINotificationObserver* aObserver, // surprised. NS_ADDREF(*aClone = clone); - if (GetOwner() && GetOwner()->GetValidator()) { + imgCacheValidator* validator = GetValidator(); + if (validator) { // Note that if we have a validator, we don't want to issue notifications at // here because we want to defer until that completes. AddProxy will add us // to the load group; we cannot avoid that in this case, because we don't // know when the validation will complete, and if it will cause us to // discard our cached state anyways. We are probably already blocked by the // original LoadImage(WithChannel) request in any event. - clone->SetNotificationsDeferred(true); - GetOwner()->GetValidator()->AddProxy(clone); + clone->MarkValidating(); + validator->AddProxy(clone); } else { // We only want to add the request to the load group of the owning document // if it is still in progress. Some callers cannot handle a supurious load @@ -1270,6 +1290,16 @@ imgRequestProxy::GetOwner() const return mBehaviour->GetOwner(); } +imgCacheValidator* +imgRequestProxy::GetValidator() const +{ + imgRequest* owner = GetOwner(); + if (!owner) { + return nullptr; + } + return owner->GetValidator(); +} + ////////////////// imgRequestProxyStatic methods class StaticBehaviour : public ProxyBehaviour diff --git a/image/imgRequestProxy.h b/image/imgRequestProxy.h index 5c0d342d1387..287a72239c2b 100644 --- a/image/imgRequestProxy.h +++ b/image/imgRequestProxy.h @@ -30,6 +30,7 @@ {0x8f, 0x65, 0x9c, 0x46, 0x2e, 0xe2, 0xbc, 0x95} \ } +class imgCacheValidator; class imgINotificationObserver; class imgStatusNotifyRunnable; class ProxyBehaviour; @@ -111,15 +112,21 @@ public: virtual void SetHasImage() override; // Whether we want notifications from ProgressTracker to be deferred until - // an event it has scheduled has been fired. + // an event it has scheduled has been fired and/or validation is complete. virtual bool NotificationsDeferred() const override { - return mDeferNotifications; + return IsValidating() || mDeferNotifications; } virtual void SetNotificationsDeferred(bool aDeferNotifications) override { mDeferNotifications = aDeferNotifications; } + bool IsValidating() const + { + return mValidating; + } + void MarkValidating(); + void ClearValidating(); bool IsOnEventTarget() const; already_AddRefed GetEventTarget() const override; @@ -194,6 +201,7 @@ protected: already_AddRefed GetImage() const; bool HasImage() const; imgRequest* GetOwner() const; + imgCacheValidator* GetValidator() const; nsresult PerformClone(imgINotificationObserver* aObserver, nsIDocument* aLoadingDocument, @@ -212,6 +220,7 @@ private: friend class imgCacheValidator; void AddToOwner(nsIDocument* aLoadingDocument); + void RemoveFromOwner(nsresult aStatus); nsresult DispatchWithTargetIfAvailable(already_AddRefed aEvent); void DispatchWithTarget(already_AddRefed aEvent); @@ -242,6 +251,7 @@ private: // Whether we want to defer our notifications by the non-virtual Observer // interfaces as image loads proceed. bool mDeferNotifications : 1; + bool mValidating : 1; bool mHadListener : 1; bool mHadDispatch : 1; }; From b1c05068b88e25aad9f9611c07c7c70effafd324 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Wed, 7 Feb 2018 07:27:27 -0500 Subject: [PATCH 02/20] Bug 1383682 - Part 2. Rename IProgressObserver::SetNotificationsDeferred to make purpose clear. r=tnikkel IProgressObserver::SetNotificationsDeferred is now used just for ProgressTracker to track when there is a pending notification for an observer. It has been renamed to MarkPendingNotify and ClearPendingNotify to make a clear distinction. --- image/IProgressObserver.h | 3 ++- image/MultipartImage.cpp | 17 ++++++++++++----- image/MultipartImage.h | 5 +++-- image/ProgressTracker.cpp | 10 +++++----- image/imgRequestProxy.cpp | 2 +- image/imgRequestProxy.h | 12 ++++++++---- 6 files changed, 31 insertions(+), 18 deletions(-) diff --git a/image/IProgressObserver.h b/image/IProgressObserver.h index 329586a27172..4068c25dccc3 100644 --- a/image/IProgressObserver.h +++ b/image/IProgressObserver.h @@ -43,7 +43,8 @@ public: // Other, internal-only methods: virtual void SetHasImage() = 0; virtual bool NotificationsDeferred() const = 0; - virtual void SetNotificationsDeferred(bool aDeferNotifications) = 0; + virtual void MarkPendingNotify() = 0; + virtual void ClearPendingNotify() = 0; virtual already_AddRefed GetEventTarget() const { diff --git a/image/MultipartImage.cpp b/image/MultipartImage.cpp index d0fdb58a962f..3a9d0ef04f79 100644 --- a/image/MultipartImage.cpp +++ b/image/MultipartImage.cpp @@ -95,7 +95,8 @@ public: // Other notifications are ignored. virtual void SetHasImage() override { } virtual bool NotificationsDeferred() const override { return false; } - virtual void SetNotificationsDeferred(bool) override { } + virtual void MarkPendingNotify() override { } + virtual void ClearPendingNotify() override { } private: virtual ~NextPartObserver() { } @@ -122,7 +123,7 @@ private: MultipartImage::MultipartImage(Image* aFirstPart) : ImageWrapper(aFirstPart) - , mDeferNotifications(false) + , mPendingNotify(false) { mNextPartObserver = new NextPartObserver(this); } @@ -333,13 +334,19 @@ MultipartImage::SetHasImage() bool MultipartImage::NotificationsDeferred() const { - return mDeferNotifications; + return mPendingNotify; } void -MultipartImage::SetNotificationsDeferred(bool aDeferNotifications) +MultipartImage::MarkPendingNotify() { - mDeferNotifications = aDeferNotifications; + mPendingNotify = true; +} + +void +MultipartImage::ClearPendingNotify() +{ + mPendingNotify = false; } } // namespace image diff --git a/image/MultipartImage.h b/image/MultipartImage.h index e26877ccd4e0..d3d0647a46b0 100644 --- a/image/MultipartImage.h +++ b/image/MultipartImage.h @@ -59,7 +59,8 @@ public: virtual void OnLoadComplete(bool aLastPart) override; virtual void SetHasImage() override; virtual bool NotificationsDeferred() const override; - virtual void SetNotificationsDeferred(bool aDeferNotifications) override; + virtual void MarkPendingNotify() override; + virtual void ClearPendingNotify() override; protected: virtual ~MultipartImage(); @@ -76,7 +77,7 @@ private: RefPtr mTracker; RefPtr mNextPartObserver; RefPtr mNextPart; - bool mDeferNotifications : 1; + bool mPendingNotify : 1; }; } // namespace image diff --git a/image/ProgressTracker.cpp b/image/ProgressTracker.cpp index d34e96d6ec5a..d0fc77821387 100644 --- a/image/ProgressTracker.cpp +++ b/image/ProgressTracker.cpp @@ -141,7 +141,7 @@ class AsyncNotifyRunnable : public Runnable MOZ_ASSERT(NS_IsMainThread(), "Should be running on the main thread"); MOZ_ASSERT(mTracker, "mTracker should not be null"); for (uint32_t i = 0; i < mObservers.Length(); ++i) { - mObservers[i]->SetNotificationsDeferred(false); + mObservers[i]->ClearPendingNotify(); mTracker->SyncNotify(mObservers[i]); } @@ -190,7 +190,7 @@ ProgressTracker::Notify(IProgressObserver* aObserver) } } - aObserver->SetNotificationsDeferred(true); + aObserver->MarkPendingNotify(); // If we have an existing runnable that we can use, we just append this // observer to its list of observers to be notified. This ensures we don't @@ -226,7 +226,7 @@ class AsyncNotifyCurrentStateRunnable : public Runnable NS_IMETHOD Run() override { MOZ_ASSERT(NS_IsMainThread(), "Should be running on the main thread"); - mObserver->SetNotificationsDeferred(false); + mObserver->ClearPendingNotify(); mProgressTracker->SyncNotify(mObserver); return NS_OK; @@ -261,7 +261,7 @@ ProgressTracker::NotifyCurrentState(IProgressObserver* aObserver) "ProgressTracker::NotifyCurrentState", "uri", spec.get()); } - aObserver->SetNotificationsDeferred(true); + aObserver->MarkPendingNotify(); nsCOMPtr ev = new AsyncNotifyCurrentStateRunnable(this, aObserver); @@ -526,7 +526,7 @@ ProgressTracker::RemoveObserver(IProgressObserver* aObserver) if (aObserver->NotificationsDeferred() && runnable) { runnable->RemoveObserver(aObserver); - aObserver->SetNotificationsDeferred(false); + aObserver->ClearPendingNotify(); } return removed; diff --git a/image/imgRequestProxy.cpp b/image/imgRequestProxy.cpp index f0c278955872..90a7caeb5b46 100644 --- a/image/imgRequestProxy.cpp +++ b/image/imgRequestProxy.cpp @@ -120,7 +120,7 @@ imgRequestProxy::imgRequestProxy() : mForceDispatchLoadGroup(false), mListenerIsStrongRef(false), mDecodeRequested(false), - mDeferNotifications(false), + mPendingNotify(false), mValidating(false), mHadListener(false), mHadDispatch(false) diff --git a/image/imgRequestProxy.h b/image/imgRequestProxy.h index 287a72239c2b..c7ca46482e57 100644 --- a/image/imgRequestProxy.h +++ b/image/imgRequestProxy.h @@ -115,11 +115,15 @@ public: // an event it has scheduled has been fired and/or validation is complete. virtual bool NotificationsDeferred() const override { - return IsValidating() || mDeferNotifications; + return IsValidating() || mPendingNotify; } - virtual void SetNotificationsDeferred(bool aDeferNotifications) override + virtual void MarkPendingNotify() override { - mDeferNotifications = aDeferNotifications; + mPendingNotify = true; + } + virtual void ClearPendingNotify() override + { + mPendingNotify = false; } bool IsValidating() const { @@ -250,7 +254,7 @@ private: // Whether we want to defer our notifications by the non-virtual Observer // interfaces as image loads proceed. - bool mDeferNotifications : 1; + bool mPendingNotify : 1; bool mValidating : 1; bool mHadListener : 1; bool mHadDispatch : 1; From e68d0fd3d281d2aca8623cb2efc137852ab18cf7 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Wed, 7 Feb 2018 07:27:28 -0500 Subject: [PATCH 03/20] Bug 1383682 - Part 3. Prevent imgRequestProxy from leaking the current state when validating. r=tnikkel There are two other means from which a caller can get the current state which originally ignored validation -- GetImageStatus and StartDecodingWithResult. These methods are used by layout in some circumstances to decide whether or not the image is ready to display. As observed in some web platform tests, in particular css/css-backgrounds-3/background-size-031.html, we may actually validate and purge the cache for images under test. The state given by the aforementioned methods was misleading, because validation changed it. Now they take into account validation, and do not imply any particular state while validation is in progress. --- image/imgRequestProxy.cpp | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/image/imgRequestProxy.cpp b/image/imgRequestProxy.cpp index 90a7caeb5b46..e6db02a95d59 100644 --- a/image/imgRequestProxy.cpp +++ b/image/imgRequestProxy.cpp @@ -560,8 +560,11 @@ imgRequestProxy::CancelAndForgetObserver(nsresult aStatus) NS_IMETHODIMP imgRequestProxy::StartDecoding(uint32_t aFlags) { - // Flag this, so we know to transfer the request if our owner changes - mDecodeRequested = true; + // Flag this, so we know to request after validation if pending. + if (IsValidating()) { + mDecodeRequested = true; + return NS_OK; + } RefPtr image = GetImage(); if (image) { @@ -578,8 +581,11 @@ imgRequestProxy::StartDecoding(uint32_t aFlags) bool imgRequestProxy::StartDecodingWithResult(uint32_t aFlags) { - // Flag this, so we know to transfer the request if our owner changes - mDecodeRequested = true; + // Flag this, so we know to request after validation if pending. + if (IsValidating()) { + mDecodeRequested = true; + return false; + } RefPtr image = GetImage(); if (image) { @@ -736,8 +742,16 @@ imgRequestProxy::GetImage(imgIContainer** aImage) NS_IMETHODIMP imgRequestProxy::GetImageStatus(uint32_t* aStatus) { - RefPtr progressTracker = GetProgressTracker(); - *aStatus = progressTracker->GetImageStatus(); + if (IsValidating()) { + // We are currently validating the image, and so our status could revert if + // we discard the cache. We should also be deferring notifications, such + // that the caller will be notified when validation completes. Rather than + // risk misleading the caller, return nothing. + *aStatus = imgIRequest::STATUS_NONE; + } else { + RefPtr progressTracker = GetProgressTracker(); + *aStatus = progressTracker->GetImageStatus(); + } return NS_OK; } From 3b58cdf796087eefafbb42294d6465235ba333f7 Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Wed, 7 Feb 2018 07:27:28 -0500 Subject: [PATCH 04/20] Bug 1408636 - Ensure accessibility tests pass regardless of image caching affecting whitespace. r=yzen --- accessible/tests/mochitest/elm/test_HTMLSpec.html | 9 +++++---- accessible/tests/mochitest/name/markuprules.xml | 12 ++++++++++++ accessible/tests/mochitest/tree/test_txtcntr.html | 6 ++++-- .../tests/mochitest/treeupdate/test_whitespace.html | 7 +++++-- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/accessible/tests/mochitest/elm/test_HTMLSpec.html b/accessible/tests/mochitest/elm/test_HTMLSpec.html index 86bc18f14e3c..6cc8dc25a58c 100644 --- a/accessible/tests/mochitest/elm/test_HTMLSpec.html +++ b/accessible/tests/mochitest/elm/test_HTMLSpec.html @@ -1510,10 +1510,11 @@ -
- An awesome picture -
Caption for the awesome picture
-
+ +
An awesome picture
Caption for the awesome picture
Some copyright info
diff --git a/accessible/tests/mochitest/name/markuprules.xml b/accessible/tests/mochitest/name/markuprules.xml index 80f8eab77e1d..b8f7d51423d6 100644 --- a/accessible/tests/mochitest/name/markuprules.xml +++ b/accessible/tests/mochitest/name/markuprules.xml @@ -234,6 +234,17 @@ title="no name from title"/> + + + diff --git a/accessible/tests/mochitest/tree/test_txtcntr.html b/accessible/tests/mochitest/tree/test_txtcntr.html index 580cce27abe2..c188f94fa0d7 100644 --- a/accessible/tests/mochitest/tree/test_txtcntr.html +++ b/accessible/tests/mochitest/tree/test_txtcntr.html @@ -227,7 +227,9 @@
This a11y test
This PC is broken
- -
+ +
diff --git a/accessible/tests/mochitest/treeupdate/test_whitespace.html b/accessible/tests/mochitest/treeupdate/test_whitespace.html index 885c5e4207bb..bc6b068e906d 100644 --- a/accessible/tests/mochitest/treeupdate/test_whitespace.html +++ b/accessible/tests/mochitest/treeupdate/test_whitespace.html @@ -170,8 +170,11 @@
   
-
-
+ +
+
From 17da4a7e09d9f210861a4832d83aaeaef880ef6b Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Wed, 7 Feb 2018 13:49:06 +0100 Subject: [PATCH 05/20] Bug 1435209 - Use CMOVcc instead of index masking. r=luke --- js/src/jit/CodeGenerator.cpp | 16 +-- js/src/jit/Lowering.cpp | 10 +- js/src/jit/MacroAssembler.cpp | 116 +----------------- js/src/jit/MacroAssembler.h | 34 ++--- js/src/jit/arm/MacroAssembler-arm-inl.h | 52 ++++++++ js/src/jit/arm64/MacroAssembler-arm64-inl.h | 44 +++++++ js/src/jit/arm64/MacroAssembler-arm64.h | 9 ++ js/src/jit/x86-shared/Assembler-x86-shared.h | 4 + .../MacroAssembler-x86-shared-inl.h | 52 ++++++++ 9 files changed, 189 insertions(+), 148 deletions(-) diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index fe7c709487ba..51141c00f83c 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -8653,24 +8653,14 @@ CodeGenerator::visitSpectreMaskIndex(LSpectreMaskIndex* lir) { MOZ_ASSERT(JitOptions.spectreIndexMasking); - const LAllocation* index = lir->index(); const LAllocation* length = lir->length(); + Register index = ToRegister(lir->index()); Register output = ToRegister(lir->output()); - if (index->isConstant()) { - int32_t idx = ToInt32(index); - if (length->isRegister()) - masm.spectreMaskIndex(idx, ToRegister(length), output); - else - masm.spectreMaskIndex(idx, ToAddress(length), output); - return; - } - - Register indexReg = ToRegister(index); if (length->isRegister()) - masm.spectreMaskIndex(indexReg, ToRegister(length), output); + masm.spectreMaskIndex(index, ToRegister(length), output); else - masm.spectreMaskIndex(indexReg, ToAddress(length), output); + masm.spectreMaskIndex(index, ToAddress(length), output); } class OutOfLineStoreElementHole : public OutOfLineCodeBase diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp index 3f3a531ef491..f9f65c88beee 100644 --- a/js/src/jit/Lowering.cpp +++ b/js/src/jit/Lowering.cpp @@ -3163,16 +3163,8 @@ LIRGenerator::visitSpectreMaskIndex(MSpectreMaskIndex* ins) MOZ_ASSERT(ins->length()->type() == MIRType::Int32); MOZ_ASSERT(ins->type() == MIRType::Int32); - // On 64-bit platforms, the length must be in a register, so - // MacroAssembler::maskIndex can emit more efficient code. -#if JS_BITS_PER_WORD == 64 - LAllocation lengthUse = useRegister(ins->length()); -#else - LAllocation lengthUse = useAny(ins->length()); -#endif - LSpectreMaskIndex* lir = - new(alloc()) LSpectreMaskIndex(useRegisterOrConstant(ins->index()), lengthUse); + new(alloc()) LSpectreMaskIndex(useRegister(ins->index()), useAny(ins->length())); define(lir, ins); } diff --git a/js/src/jit/MacroAssembler.cpp b/js/src/jit/MacroAssembler.cpp index 4e79c5078785..78b719fefed0 100644 --- a/js/src/jit/MacroAssembler.cpp +++ b/js/src/jit/MacroAssembler.cpp @@ -3322,99 +3322,27 @@ MacroAssembler::debugAssertIsObject(const ValueOperand& val) #endif } -template -void -MacroAssembler::computeSpectreIndexMaskGeneric(Register index, const T& length, Register output) -{ - MOZ_ASSERT(JitOptions.spectreIndexMasking); - MOZ_ASSERT(index != output); - - // mask := ((index - length) & ~index) >> 31 - mov(index, output); - sub32(length, output); - not32(index); - and32(index, output); - not32(index); // Restore index register to its original value. - rshift32Arithmetic(Imm32(31), output); -} - -template -void -MacroAssembler::computeSpectreIndexMask(int32_t index, const T& length, Register output) -{ - MOZ_ASSERT(JitOptions.spectreIndexMasking); - - // mask := ((index - length) & ~index) >> 31 - move32(Imm32(index), output); - sub32(length, output); - and32(Imm32(~index), output); - rshift32Arithmetic(Imm32(31), output); -} - -void -MacroAssembler::computeSpectreIndexMask(Register index, Register length, Register output) -{ - MOZ_ASSERT(JitOptions.spectreIndexMasking); - MOZ_ASSERT(length != output); - MOZ_ASSERT(index != output); - -#if JS_BITS_PER_WORD == 64 - // On 64-bit platforms, we can use a faster algorithm: - // - // mask := (uint64_t(index) - uint64_t(length)) >> 32 - // - // mask is 0x11…11 if index < length, 0 otherwise. - move32(index, output); - subPtr(length, output); - rshiftPtr(Imm32(32), output); -#else - computeSpectreIndexMaskGeneric(index, length, output); -#endif -} - -void -MacroAssembler::spectreMaskIndex(int32_t index, Register length, Register output) -{ - MOZ_ASSERT(length != output); - if (index == 0) { - move32(Imm32(index), output); - } else { - computeSpectreIndexMask(index, length, output); - and32(Imm32(index), output); - } -} - -void -MacroAssembler::spectreMaskIndex(int32_t index, const Address& length, Register output) -{ - MOZ_ASSERT(length.base != output); - if (index == 0) { - move32(Imm32(index), output); - } else { - computeSpectreIndexMask(index, length, output); - and32(Imm32(index), output); - } -} - void MacroAssembler::spectreMaskIndex(Register index, Register length, Register output) { + MOZ_ASSERT(JitOptions.spectreIndexMasking); MOZ_ASSERT(length != output); MOZ_ASSERT(index != output); - computeSpectreIndexMask(index, length, output); - and32(index, output); + move32(Imm32(0), output); + cmp32Move32(Assembler::Below, index, length, index, output); } void MacroAssembler::spectreMaskIndex(Register index, const Address& length, Register output) { + MOZ_ASSERT(JitOptions.spectreIndexMasking); MOZ_ASSERT(index != length.base); MOZ_ASSERT(length.base != output); MOZ_ASSERT(index != output); - computeSpectreIndexMaskGeneric(index, length, output); - and32(index, output); + move32(Imm32(0), output); + cmp32Move32(Assembler::Below, index, length, index, output); } void @@ -3429,38 +3357,6 @@ MacroAssembler::boundsCheck32PowerOfTwo(Register index, uint32_t length, Label* and32(Imm32(length - 1), index); } -void -MacroAssembler::boundsCheck32ForLoad(Register index, Register length, Register scratch, - Label* failure) -{ - MOZ_ASSERT(index != length); - MOZ_ASSERT(length != scratch); - MOZ_ASSERT(index != scratch); - - branch32(Assembler::AboveOrEqual, index, length, failure); - - if (JitOptions.spectreIndexMasking) { - computeSpectreIndexMask(index, length, scratch); - and32(scratch, index); - } -} - -void -MacroAssembler::boundsCheck32ForLoad(Register index, const Address& length, Register scratch, - Label* failure) -{ - MOZ_ASSERT(index != length.base); - MOZ_ASSERT(length.base != scratch); - MOZ_ASSERT(index != scratch); - - branch32(Assembler::BelowOrEqual, length, index, failure); - - if (JitOptions.spectreIndexMasking) { - computeSpectreIndexMaskGeneric(index, length, scratch); - and32(scratch, index); - } -} - namespace js { namespace jit { diff --git a/js/src/jit/MacroAssembler.h b/js/src/jit/MacroAssembler.h index dfcc39c0f91d..73c46cb342dc 100644 --- a/js/src/jit/MacroAssembler.h +++ b/js/src/jit/MacroAssembler.h @@ -1359,6 +1359,24 @@ class MacroAssembler : public MacroAssemblerSpecific DEFINED_ON(arm, arm64, x86_shared); public: + + inline void cmp32Move32(Condition cond, Register lhs, Register rhs, Register src, + Register dest) + DEFINED_ON(arm, arm64, x86_shared); + + inline void cmp32Move32(Condition cond, Register lhs, const Address& rhs, Register src, + Register dest) + DEFINED_ON(arm, arm64, x86_shared); + + // Performs a bounds check and zeroes the index register if out-of-bounds + // (to mitigate Spectre). + inline void boundsCheck32ForLoad(Register index, Register length, Register scratch, + Label* failure) + DEFINED_ON(arm, arm64, x86_shared); + inline void boundsCheck32ForLoad(Register index, const Address& length, Register scratch, + Label* failure) + DEFINED_ON(arm, arm64, x86_shared); + // ======================================================================== // Canonicalization primitives. inline void canonicalizeDouble(FloatRegister reg); @@ -2055,18 +2073,6 @@ class MacroAssembler : public MacroAssemblerSpecific store32(Imm32(key.constant()), dest); } - private: - template - void computeSpectreIndexMaskGeneric(Register index, const T& length, Register output); - - void computeSpectreIndexMask(Register index, Register length, Register output); - - template - void computeSpectreIndexMask(int32_t index, const T& length, Register output); - - public: - void spectreMaskIndex(int32_t index, Register length, Register output); - void spectreMaskIndex(int32_t index, const Address& length, Register output); void spectreMaskIndex(Register index, Register length, Register output); void spectreMaskIndex(Register index, const Address& length, Register output); @@ -2074,10 +2080,6 @@ class MacroAssembler : public MacroAssemblerSpecific // masking. void boundsCheck32PowerOfTwo(Register index, uint32_t length, Label* failure); - // Performs a bounds check and Spectre index masking. - void boundsCheck32ForLoad(Register index, Register length, Register scratch, Label* failure); - void boundsCheck32ForLoad(Register index, const Address& length, Register scratch, Label* failure); - template void guardedCallPreBarrier(const T& address, MIRType type) { Label done; diff --git a/js/src/jit/arm/MacroAssembler-arm-inl.h b/js/src/jit/arm/MacroAssembler-arm-inl.h index 0d33d6e17e74..a818fcf05d3b 100644 --- a/js/src/jit/arm/MacroAssembler-arm-inl.h +++ b/js/src/jit/arm/MacroAssembler-arm-inl.h @@ -2136,6 +2136,58 @@ MacroAssembler::branchToComputedAddress(const BaseIndex& addr) breakpoint(); } +void +MacroAssembler::cmp32Move32(Condition cond, Register lhs, Register rhs, Register src, + Register dest) +{ + cmp32(lhs, rhs); + ma_mov(src, dest, LeaveCC, cond); +} + +void +MacroAssembler::cmp32Move32(Condition cond, Register lhs, const Address& rhs, Register src, + Register dest) +{ + ScratchRegisterScope scratch(*this); + SecondScratchRegisterScope scratch2(*this); + ma_ldr(rhs, scratch, scratch2); + cmp32Move32(cond, lhs, scratch, src, dest); +} + +void +MacroAssembler::boundsCheck32ForLoad(Register index, Register length, Register scratch, + Label* failure) +{ + MOZ_ASSERT(index != length); + MOZ_ASSERT(length != scratch); + MOZ_ASSERT(index != scratch); + + if (JitOptions.spectreIndexMasking) + move32(Imm32(0), scratch); + + branch32(Assembler::BelowOrEqual, length, index, failure); + + if (JitOptions.spectreIndexMasking) + ma_mov(scratch, index, LeaveCC, Assembler::BelowOrEqual); +} + +void +MacroAssembler::boundsCheck32ForLoad(Register index, const Address& length, Register scratch, + Label* failure) +{ + MOZ_ASSERT(index != length.base); + MOZ_ASSERT(length.base != scratch); + MOZ_ASSERT(index != scratch); + + if (JitOptions.spectreIndexMasking) + move32(Imm32(0), scratch); + + branch32(Assembler::BelowOrEqual, length, index, failure); + + if (JitOptions.spectreIndexMasking) + ma_mov(scratch, index, LeaveCC, Assembler::BelowOrEqual); +} + // ======================================================================== // Memory access primitives. void diff --git a/js/src/jit/arm64/MacroAssembler-arm64-inl.h b/js/src/jit/arm64/MacroAssembler-arm64-inl.h index da35d2187fe9..fe9b6879fcfe 100644 --- a/js/src/jit/arm64/MacroAssembler-arm64-inl.h +++ b/js/src/jit/arm64/MacroAssembler-arm64-inl.h @@ -1710,6 +1710,50 @@ MacroAssembler::branchToComputedAddress(const BaseIndex& addr) MOZ_CRASH("branchToComputedAddress"); } +void +MacroAssembler::cmp32Move32(Condition cond, Register lhs, Register rhs, Register src, + Register dest) +{ + cmp32(lhs, rhs); + Csel(ARMRegister(dest, 32), ARMRegister(src, 32), ARMRegister(dest, 32), cond); +} + +void +MacroAssembler::cmp32Move32(Condition cond, Register lhs, const Address& rhs, Register src, + Register dest) +{ + cmp32(lhs, rhs); + Csel(ARMRegister(dest, 32), ARMRegister(src, 32), ARMRegister(dest, 32), cond); +} + +void +MacroAssembler::boundsCheck32ForLoad(Register index, Register length, Register scratch, + Label* failure) +{ + MOZ_ASSERT(index != length); + MOZ_ASSERT(length != scratch); + MOZ_ASSERT(index != scratch); + + branch32(Assembler::BelowOrEqual, length, index, failure); + + if (JitOptions.spectreIndexMasking) + Csel(ARMRegister(index, 32), ARMRegister(index, 32), vixl::wzr, Assembler::Above); +} + +void +MacroAssembler::boundsCheck32ForLoad(Register index, const Address& length, Register scratch, + Label* failure) +{ + MOZ_ASSERT(index != length.base); + MOZ_ASSERT(length.base != scratch); + MOZ_ASSERT(index != scratch); + + branch32(Assembler::BelowOrEqual, length, index, failure); + + if (JitOptions.spectreIndexMasking) + Csel(ARMRegister(index, 32), ARMRegister(index, 32), vixl::wzr, Assembler::Above); +} + // ======================================================================== // Memory access primitives. void diff --git a/js/src/jit/arm64/MacroAssembler-arm64.h b/js/src/jit/arm64/MacroAssembler-arm64.h index 0b59b1c74aef..3f0d4c4c73af 100644 --- a/js/src/jit/arm64/MacroAssembler-arm64.h +++ b/js/src/jit/arm64/MacroAssembler-arm64.h @@ -1022,6 +1022,9 @@ class MacroAssemblerCompat : public vixl::MacroAssembler void cmp32(const Address& lhs, Register rhs) { cmp32(Operand(lhs.base, lhs.offset), rhs); } + void cmp32(Register lhs, const Address& rhs) { + cmp32(lhs, Operand(rhs.base, rhs.offset)); + } void cmp32(const Operand& lhs, Imm32 rhs) { vixl::UseScratchRegisterScope temps(this); const ARMRegister scratch32 = temps.AcquireW(); @@ -1034,6 +1037,12 @@ class MacroAssemblerCompat : public vixl::MacroAssembler Mov(scratch32, lhs); Cmp(scratch32, Operand(ARMRegister(rhs, 32))); } + void cmp32(Register lhs, const Operand& rhs) { + vixl::UseScratchRegisterScope temps(this); + const ARMRegister scratch32 = temps.AcquireW(); + Mov(scratch32, rhs); + Cmp(scratch32, Operand(ARMRegister(lhs, 32))); + } void cmpPtr(Register lhs, Imm32 rhs) { Cmp(ARMRegister(lhs, 64), Operand(rhs.value)); diff --git a/js/src/jit/x86-shared/Assembler-x86-shared.h b/js/src/jit/x86-shared/Assembler-x86-shared.h index d9e4ccb4bb59..092bdfb1870c 100644 --- a/js/src/jit/x86-shared/Assembler-x86-shared.h +++ b/js/src/jit/x86-shared/Assembler-x86-shared.h @@ -469,6 +469,10 @@ class AssemblerX86Shared : public AssemblerShared MOZ_CRASH("unexpected operand kind"); } } + void cmovCCl(Condition cond, Register src, Register dest) { + X86Encoding::Condition cc = static_cast(cond); + masm.cmovCCl_rr(cc, src.encoding(), dest.encoding()); + } void cmovzl(const Operand& src, Register dest) { cmovCCl(Condition::Zero, src, dest); } diff --git a/js/src/jit/x86-shared/MacroAssembler-x86-shared-inl.h b/js/src/jit/x86-shared/MacroAssembler-x86-shared-inl.h index 49583c7d34ce..ebf97a93d890 100644 --- a/js/src/jit/x86-shared/MacroAssembler-x86-shared-inl.h +++ b/js/src/jit/x86-shared/MacroAssembler-x86-shared-inl.h @@ -1094,6 +1094,58 @@ MacroAssembler::branchTestMagicImpl(Condition cond, const T& t, L label) j(cond, label); } +void +MacroAssembler::cmp32Move32(Condition cond, Register lhs, Register rhs, Register src, + Register dest) +{ + cmp32(lhs, rhs); + cmovCCl(cond, src, dest); +} + +void +MacroAssembler::cmp32Move32(Condition cond, Register lhs, const Address& rhs, Register src, + Register dest) +{ + cmp32(lhs, Operand(rhs)); + cmovCCl(cond, src, dest); +} + +void +MacroAssembler::boundsCheck32ForLoad(Register index, Register length, Register scratch, + Label* failure) +{ + MOZ_ASSERT(index != length); + MOZ_ASSERT(length != scratch); + MOZ_ASSERT(index != scratch); + + if (JitOptions.spectreIndexMasking) + move32(Imm32(0), scratch); + + cmp32(index, length); + j(Assembler::AboveOrEqual, failure); + + if (JitOptions.spectreIndexMasking) + cmovCCl(Assembler::AboveOrEqual, scratch, index); +} + +void +MacroAssembler::boundsCheck32ForLoad(Register index, const Address& length, Register scratch, + Label* failure) +{ + MOZ_ASSERT(index != length.base); + MOZ_ASSERT(length.base != scratch); + MOZ_ASSERT(index != scratch); + + if (JitOptions.spectreIndexMasking) + move32(Imm32(0), scratch); + + cmp32(index, Operand(length)); + j(Assembler::AboveOrEqual, failure); + + if (JitOptions.spectreIndexMasking) + cmovCCl(Assembler::AboveOrEqual, scratch, index); +} + // ======================================================================== // Canonicalization primitives. void From 89aba048416b4b3bab0c1a60d60b641d174818c2 Mon Sep 17 00:00:00 2001 From: sotaro Date: Wed, 7 Feb 2018 22:21:35 +0900 Subject: [PATCH 06/20] Bug 1435995 - Disable DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL usage r=nical --- gfx/webrender_bindings/RenderCompositorANGLE.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/gfx/webrender_bindings/RenderCompositorANGLE.cpp b/gfx/webrender_bindings/RenderCompositorANGLE.cpp index f92362dcab59..822a3ffce642 100644 --- a/gfx/webrender_bindings/RenderCompositorANGLE.cpp +++ b/gfx/webrender_bindings/RenderCompositorANGLE.cpp @@ -85,8 +85,11 @@ RenderCompositorANGLE::Initialize() desc.SampleDesc.Count = 1; desc.SampleDesc.Quality = 0; desc.BufferUsage = DXGI_USAGE_RENDER_TARGET_OUTPUT; - desc.BufferCount = 2; - desc.SwapEffect = DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL; + // Do not use DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL, since it makes HWND unreusable. + //desc.BufferCount = 2; + //desc.SwapEffect = DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL; + desc.BufferCount = 1; + desc.SwapEffect = DXGI_SWAP_EFFECT_SEQUENTIAL; desc.Scaling = DXGI_SCALING_NONE; desc.Flags = 0; From 6e6de3f7ac672670af75c6f0c3e8f47cdee45cf6 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Wed, 7 Feb 2018 14:38:00 +0100 Subject: [PATCH 07/20] Bug 1436065 - Add JS_NewLatin1String API to create Latin1 strings without copying. r=luke --- js/src/jsapi.cpp | 8 ++++++++ js/src/jsapi.h | 3 +++ 2 files changed, 11 insertions(+) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index c283bbe903a5..716028def036 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -5843,6 +5843,14 @@ JS_AtomizeAndPinStringN(JSContext* cx, const char* s, size_t length) return atom; } +JS_PUBLIC_API(JSString*) +JS_NewLatin1String(JSContext* cx, JS::Latin1Char* chars, size_t length) +{ + AssertHeapIsIdle(); + CHECK_REQUEST(cx); + return NewString(cx, chars, length); +} + JS_PUBLIC_API(JSString*) JS_NewUCString(JSContext* cx, char16_t* chars, size_t length) { diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 16538dca1d28..b2d2bee38506 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -4722,6 +4722,9 @@ JS_AtomizeAndPinStringN(JSContext* cx, const char* s, size_t length); extern JS_PUBLIC_API(JSString*) JS_AtomizeAndPinString(JSContext* cx, const char* s); +extern JS_PUBLIC_API(JSString*) +JS_NewLatin1String(JSContext* cx, JS::Latin1Char* chars, size_t length); + extern JS_PUBLIC_API(JSString*) JS_NewUCString(JSContext* cx, char16_t* chars, size_t length); From 081a738eb4b6e4713c97cbf542e6b3717606f6cd Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Wed, 7 Feb 2018 14:39:11 +0100 Subject: [PATCH 08/20] Bug 1435796 - Fix JSString::dumpRepresentationHeader to use printf instead of put. r=sfink --- js/src/jit-test/tests/basic/dumpStringRepresentation.js | 3 +++ js/src/vm/String.cpp | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/js/src/jit-test/tests/basic/dumpStringRepresentation.js b/js/src/jit-test/tests/basic/dumpStringRepresentation.js index 5a11e5b84b8d..7a63d3c2bda1 100644 --- a/js/src/jit-test/tests/basic/dumpStringRepresentation.js +++ b/js/src/jit-test/tests/basic/dumpStringRepresentation.js @@ -10,6 +10,9 @@ dumpStringRepresentation(""); print("\nResult of coercion to string:"); dumpStringRepresentation(); +print("\nString with an index value:"); +dumpStringRepresentation((12345).toString()); + print("\ns = Simple short atom:"); var s = "xxxxxxxx"; dumpStringRepresentation(s); diff --git a/js/src/vm/String.cpp b/js/src/vm/String.cpp index 87c87bf3b3a3..ae4a16c09c3a 100644 --- a/js/src/vm/String.cpp +++ b/js/src/vm/String.cpp @@ -211,7 +211,7 @@ JSString::dumpRepresentationHeader(js::GenericPrinter& out, int indent, const ch if (flags & ATOM_BIT) out.put(" ATOM"); if (isPermanentAtom()) out.put(" PERMANENT"); if (flags & LATIN1_CHARS_BIT) out.put(" LATIN1"); - if (flags & INDEX_VALUE_BIT) out.put(" INDEX_VALUE(%u)", getIndexValue()); + if (flags & INDEX_VALUE_BIT) out.printf(" INDEX_VALUE(%u)", getIndexValue()); out.putChar('\n'); } From 1d2ba2eb2b02a9a2c30d0515b6db1be83a9914ea Mon Sep 17 00:00:00 2001 From: Tim Taubert Date: Wed, 7 Feb 2018 12:01:00 +0100 Subject: [PATCH 09/20] Bug 1436308 - Generate a ToJSON() method for WebIDL types with records r=bz We currently don't generate a ::ToJSON() method for WebIDL types with record members. These types should be safe to convert to JSON, as long as type V itself is. Per spec, type K is always a DOMString, USVString, or ByteString. --- dom/bindings/Codegen.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index b13078a7f452..c387a5ce9fb1 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -13823,6 +13823,11 @@ class CGDictionary(CGThing): # they're not unrestricted float/double. return not type.isFloat() or not type.isUnrestricted() + if type.isRecord(): + # Records are okay, as long as the value type is. + # Per spec, only strings are allowed as keys. + return CGDictionary.typeSafeToJSONify(type.inner) + return False @staticmethod From 34935cb2fffce999ef28365b3a296d1f7414d92f Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Wed, 7 Feb 2018 09:33:12 -0500 Subject: [PATCH 10/20] Bug 1388020. r=nical --- gfx/layers/client/TextureClient.h | 2 +- gfx/layers/composite/TextureHost.cpp | 65 +++++++++++++++---- gfx/layers/composite/X11TextureHost.cpp | 13 ++-- gfx/layers/d3d11/TextureD3D11.cpp | 6 +- .../opengl/MacIOSurfaceTextureHostOGL.cpp | 8 +++ gfx/layers/opengl/TextureHostOGL.cpp | 25 ++----- gfx/tests/gtest/TestLayers.h | 16 +++++ gfx/tests/gtest/TestTextureCompatibility.cpp | 12 +++- gfx/tests/gtest/TestTextures.cpp | 7 +- gfx/tests/gtest/TextureHelper.h | 3 +- 10 files changed, 108 insertions(+), 49 deletions(-) diff --git a/gfx/layers/client/TextureClient.h b/gfx/layers/client/TextureClient.h index e56ea21841ad..81be2867ca7a 100644 --- a/gfx/layers/client/TextureClient.h +++ b/gfx/layers/client/TextureClient.h @@ -761,7 +761,7 @@ protected: friend void TestTextureClientSurface(TextureClient*, gfxImageSurface*); friend void TestTextureClientYCbCr(TextureClient*, PlanarYCbCrData&); friend already_AddRefed CreateTextureHostWithBackend( - TextureClient*, LayersBackend&); + TextureClient*, ISurfaceAllocator*, LayersBackend&); #ifdef GFX_DEBUG_TRACK_CLIENTS_IN_POOL public: diff --git a/gfx/layers/composite/TextureHost.cpp b/gfx/layers/composite/TextureHost.cpp index 338734b57bd8..3ee8a9c5edcb 100644 --- a/gfx/layers/composite/TextureHost.cpp +++ b/gfx/layers/composite/TextureHost.cpp @@ -116,15 +116,9 @@ TextureHost::CreateIPDLActor(HostIPCAllocator* aAllocator, uint64_t aSerial, const wr::MaybeExternalImageId& aExternalImageId) { - if (aSharedData.type() == SurfaceDescriptor::TSurfaceDescriptorBuffer && - aSharedData.get_SurfaceDescriptorBuffer().data().type() == MemoryOrShmem::Tuintptr_t && - !aAllocator->IsSameProcess()) - { - NS_ERROR("A client process is trying to peek at our address space using a MemoryTexture!"); - return nullptr; - } TextureParent* actor = new TextureParent(aAllocator, aSerial, aExternalImageId); if (!actor->Init(aSharedData, aLayersBackend, aFlags)) { + actor->ActorDestroy(ipc::IProtocol::ActorDestroyReason::FailedConstructor); delete actor; return nullptr; } @@ -232,6 +226,11 @@ TextureHost::Create(const SurfaceDescriptor& aDesc, #ifdef MOZ_X11 case SurfaceDescriptor::TSurfaceDescriptorX11: { + if (!aDeallocator->IsSameProcess()) { + NS_ERROR("A client process is trying to peek at our address space using a X11Texture!"); + return nullptr; + } + const SurfaceDescriptorX11& desc = aDesc.get_SurfaceDescriptorX11(); result = MakeAndAddRef(aFlags, desc); break; @@ -248,7 +247,7 @@ TextureHost::Create(const SurfaceDescriptor& aDesc, MOZ_CRASH("GFX: Unsupported Surface type host"); } - if (WrapWithWebRenderTextureHost(aDeallocator, aBackend, aFlags)) { + if (result && WrapWithWebRenderTextureHost(aDeallocator, aBackend, aFlags)) { MOZ_ASSERT(aExternalImageId.isSome()); result = new WebRenderTextureHost(aDesc, aFlags, result, aExternalImageId.ref()); } @@ -269,13 +268,50 @@ CreateBackendIndependentTextureHost(const SurfaceDescriptor& aDesc, const MemoryOrShmem& data = bufferDesc.data(); switch (data.type()) { case MemoryOrShmem::TShmem: { - result = new ShmemTextureHost(data.get_Shmem(), - bufferDesc.desc(), - aDeallocator, - aFlags); + const ipc::Shmem& shmem = data.get_Shmem(); + const BufferDescriptor& desc = bufferDesc.desc(); + if (!shmem.IsReadable()) { + // We failed to map the shmem so we can't verify its size. This + // should not be a fatal error, so just create the texture with + // nothing backing it. + result = new ShmemTextureHost(shmem, desc, aDeallocator, aFlags); + break; + } + + size_t bufSize = shmem.Size(); + size_t reqSize = SIZE_MAX; + switch (desc.type()) { + case BufferDescriptor::TYCbCrDescriptor: { + const YCbCrDescriptor& ycbcr = desc.get_YCbCrDescriptor(); + reqSize = + ImageDataSerializer::ComputeYCbCrBufferSize(ycbcr.ySize(), ycbcr.yStride(), + ycbcr.cbCrSize(), ycbcr.cbCrStride()); + break; + } + case BufferDescriptor::TRGBDescriptor: { + const RGBDescriptor& rgb = desc.get_RGBDescriptor(); + reqSize = ImageDataSerializer::ComputeRGBBufferSize(rgb.size(), rgb.format()); + break; + } + default: + gfxCriticalError() << "Bad buffer host descriptor " << (int)desc.type(); + MOZ_CRASH("GFX: Bad descriptor"); + } + + if (bufSize < reqSize) { + NS_ERROR("A client process gave a shmem too small to fit for its descriptor!"); + return nullptr; + } + + result = new ShmemTextureHost(shmem, desc, aDeallocator, aFlags); break; } case MemoryOrShmem::Tuintptr_t: { + if (!aDeallocator->IsSameProcess()) { + NS_ERROR("A client process is trying to peek at our address space using a MemoryTexture!"); + return nullptr; + } + result = new MemoryTextureHost(reinterpret_cast(data.get_uintptr_t()), bufferDesc.desc(), aFlags); @@ -293,6 +329,11 @@ CreateBackendIndependentTextureHost(const SurfaceDescriptor& aDesc, } #ifdef XP_WIN case SurfaceDescriptor::TSurfaceDescriptorDIB: { + if (!aDeallocator->IsSameProcess()) { + NS_ERROR("A client process is trying to peek at our address space using a DIBTexture!"); + return nullptr; + } + result = new DIBTextureHost(aFlags, aDesc); break; } diff --git a/gfx/layers/composite/X11TextureHost.cpp b/gfx/layers/composite/X11TextureHost.cpp index e2251f0c531a..94cb3f2f9594 100644 --- a/gfx/layers/composite/X11TextureHost.cpp +++ b/gfx/layers/composite/X11TextureHost.cpp @@ -23,10 +23,9 @@ X11TextureHost::X11TextureHost(TextureFlags aFlags, const SurfaceDescriptorX11& aDescriptor) : TextureHost(aFlags) { - RefPtr surface = aDescriptor.OpenForeign(); - mSurface = surface.get(); + mSurface = aDescriptor.OpenForeign(); - if (!(aFlags & TextureFlags::DEALLOCATE_CLIENT)) { + if (mSurface && !(aFlags & TextureFlags::DEALLOCATE_CLIENT)) { mSurface->TakePixmap(); } } @@ -34,7 +33,7 @@ X11TextureHost::X11TextureHost(TextureFlags aFlags, bool X11TextureHost::Lock() { - if (!mCompositor) { + if (!mCompositor || !mSurface) { return false; } @@ -75,6 +74,9 @@ X11TextureHost::SetTextureSourceProvider(TextureSourceProvider* aProvider) SurfaceFormat X11TextureHost::GetFormat() const { + if (!mSurface) { + return SurfaceFormat::UNKNOWN; + } gfxContentType type = mSurface->GetContentType(); #ifdef GL_PROVIDER_GLX if (mCompositor->GetBackendType() == LayersBackend::LAYERS_OPENGL) { @@ -87,6 +89,9 @@ X11TextureHost::GetFormat() const IntSize X11TextureHost::GetSize() const { + if (!mSurface) { + return IntSize(); + } return mSurface->GetSize(); } diff --git a/gfx/layers/d3d11/TextureD3D11.cpp b/gfx/layers/d3d11/TextureD3D11.cpp index 48c3cfe6b668..192556224202 100644 --- a/gfx/layers/d3d11/TextureD3D11.cpp +++ b/gfx/layers/d3d11/TextureD3D11.cpp @@ -763,10 +763,6 @@ CreateTextureHostD3D11(const SurfaceDescriptor& aDesc, { RefPtr result; switch (aDesc.type()) { - case SurfaceDescriptor::TSurfaceDescriptorBuffer: { - result = CreateBackendIndependentTextureHost(aDesc, aDeallocator, aBackend, aFlags); - break; - } case SurfaceDescriptor::TSurfaceDescriptorD3D10: { result = new DXGITextureHostD3D11(aFlags, aDesc.get_SurfaceDescriptorD3D10()); @@ -778,7 +774,7 @@ CreateTextureHostD3D11(const SurfaceDescriptor& aDesc, break; } default: { - NS_WARNING("Unsupported SurfaceDescriptor type"); + MOZ_ASSERT_UNREACHABLE("Unsupported SurfaceDescriptor type"); } } return result.forget(); diff --git a/gfx/layers/opengl/MacIOSurfaceTextureHostOGL.cpp b/gfx/layers/opengl/MacIOSurfaceTextureHostOGL.cpp index f88f5d02b09a..5729479b5bd2 100644 --- a/gfx/layers/opengl/MacIOSurfaceTextureHostOGL.cpp +++ b/gfx/layers/opengl/MacIOSurfaceTextureHostOGL.cpp @@ -32,6 +32,8 @@ MacIOSurfaceTextureHostOGL::~MacIOSurfaceTextureHostOGL() GLTextureSource* MacIOSurfaceTextureHostOGL::CreateTextureSourceForPlane(size_t aPlane) { + MOZ_ASSERT(mSurface); + GLuint textureHandle; gl::GLContext* gl = mProvider->GetGLContext(); gl->fGenTextures(1, &textureHandle); @@ -94,11 +96,17 @@ MacIOSurfaceTextureHostOGL::SetTextureSourceProvider(TextureSourceProvider* aPro gfx::SurfaceFormat MacIOSurfaceTextureHostOGL::GetFormat() const { + if (!mSurface) { + return gfx::SurfaceFormat::UNKNOWN; + } return mSurface->GetFormat(); } gfx::SurfaceFormat MacIOSurfaceTextureHostOGL::GetReadFormat() const { + if (!mSurface) { + return gfx::SurfaceFormat::UNKNOWN; + } return mSurface->GetReadFormat(); } diff --git a/gfx/layers/opengl/TextureHostOGL.cpp b/gfx/layers/opengl/TextureHostOGL.cpp index bdcd8778c10d..2b28c19a126d 100644 --- a/gfx/layers/opengl/TextureHostOGL.cpp +++ b/gfx/layers/opengl/TextureHostOGL.cpp @@ -26,10 +26,6 @@ #include "mozilla/layers/MacIOSurfaceTextureHostOGL.h" #endif -#ifdef GL_PROVIDER_GLX -#include "mozilla/layers/X11TextureHost.h" -#endif - using namespace mozilla::gl; using namespace mozilla::gfx; @@ -46,14 +42,6 @@ CreateTextureHostOGL(const SurfaceDescriptor& aDesc, { RefPtr result; switch (aDesc.type()) { - case SurfaceDescriptor::TSurfaceDescriptorBuffer: { - result = CreateBackendIndependentTextureHost(aDesc, - aDeallocator, - aBackend, - aFlags); - break; - } - #ifdef MOZ_WIDGET_ANDROID case SurfaceDescriptor::TSurfaceTextureDescriptor: { const SurfaceTextureDescriptor& desc = aDesc.get_SurfaceTextureDescriptor(); @@ -88,14 +76,6 @@ CreateTextureHostOGL(const SurfaceDescriptor& aDesc, } #endif -#ifdef GL_PROVIDER_GLX - case SurfaceDescriptor::TSurfaceDescriptorX11: { - const auto& desc = aDesc.get_SurfaceDescriptorX11(); - result = new X11TextureHost(aFlags, desc); - break; - } -#endif - case SurfaceDescriptor::TSurfaceDescriptorSharedGLTexture: { const auto& desc = aDesc.get_SurfaceDescriptorSharedGLTexture(); result = new GLTextureHost(aFlags, desc.texture(), @@ -105,7 +85,10 @@ CreateTextureHostOGL(const SurfaceDescriptor& aDesc, desc.hasAlpha()); break; } - default: return nullptr; + default: { + MOZ_ASSERT_UNREACHABLE("Unsupported SurfaceDescriptor type"); + break; + } } return result.forget(); } diff --git a/gfx/tests/gtest/TestLayers.h b/gfx/tests/gtest/TestLayers.h index 18e351f7718c..fc6b750f1c62 100644 --- a/gfx/tests/gtest/TestLayers.h +++ b/gfx/tests/gtest/TestLayers.h @@ -8,6 +8,22 @@ #include "Layers.h" #include "nsTArray.h" +#include "mozilla/layers/ISurfaceAllocator.h" + +namespace mozilla { +namespace layers { + +class TestSurfaceAllocator final : public ISurfaceAllocator +{ +public: + TestSurfaceAllocator() {} + ~TestSurfaceAllocator() override {} + + bool IsSameProcess() const override { return true; } +}; + +} // layers +} // mozilla /* Create layer tree from a simple layer tree description syntax. * Each index is either the first letter of the layer type or diff --git a/gfx/tests/gtest/TestTextureCompatibility.cpp b/gfx/tests/gtest/TestTextureCompatibility.cpp index 45db4943ed1c..0815823b81ce 100644 --- a/gfx/tests/gtest/TestTextureCompatibility.cpp +++ b/gfx/tests/gtest/TestTextureCompatibility.cpp @@ -14,6 +14,7 @@ #include "mozilla/layers/TextureClient.h" #include "mozilla/layers/TextureHost.h" #include "mozilla/RefPtr.h" +#include "TestLayers.h" #include "TextureHelper.h" using mozilla::gfx::Feature; @@ -22,6 +23,7 @@ using mozilla::layers::BasicCompositor; using mozilla::layers::Compositor; using mozilla::layers::CompositorOptions; using mozilla::layers::LayersBackend; +using mozilla::layers::TestSurfaceAllocator; using mozilla::layers::TextureClient; using mozilla::layers::TextureHost; using mozilla::widget::CompositorWidget; @@ -31,8 +33,9 @@ using mozilla::widget::InProcessCompositorWidget; * This function will create the possible TextureClient and TextureHost pairs * according to the given backend. */ -void +static void CreateTextureWithBackend(LayersBackend& aLayersBackend, + ISurfaceAllocator* aDeallocator, nsTArray>& aTextureClients, nsTArray>& aTextureHosts) { @@ -43,7 +46,8 @@ CreateTextureWithBackend(LayersBackend& aLayersBackend, for (uint32_t i = 0; i < aTextureClients.Length(); i++) { aTextureHosts.AppendElement( - CreateTextureHostWithBackend(aTextureClients[i], aLayersBackend)); + CreateTextureHostWithBackend(aTextureClients[i], aDeallocator, + aLayersBackend)); } } @@ -115,13 +119,15 @@ CheckCompatibilityWithBasicCompositor(LayersBackend aBackends, TEST(Gfx, TestTextureCompatibility) { nsTArray backendHints; + RefPtr deallocator = new TestSurfaceAllocator(); GetPlatformBackends(backendHints); for (uint32_t i = 0; i < backendHints.Length(); i++) { nsTArray> textureClients; nsTArray> textureHosts; - CreateTextureWithBackend(backendHints[i], textureClients, textureHosts); + CreateTextureWithBackend(backendHints[i], deallocator, + textureClients, textureHosts); CheckCompatibilityWithBasicCompositor(backendHints[i], textureHosts); } } diff --git a/gfx/tests/gtest/TestTextures.cpp b/gfx/tests/gtest/TestTextures.cpp index 19b94b867117..291eb55ca7e5 100644 --- a/gfx/tests/gtest/TestTextures.cpp +++ b/gfx/tests/gtest/TestTextures.cpp @@ -5,6 +5,7 @@ #include "gtest/gtest.h" #include "gmock/gmock.h" +#include "TestLayers.h" #include "mozilla/gfx/2D.h" #include "mozilla/gfx/Tools.h" @@ -147,7 +148,8 @@ void TestTextureClientSurface(TextureClient* texture, gfxImageSurface* surface) ASSERT_NE(descriptor.type(), SurfaceDescriptor::Tnull_t); // host deserialization - RefPtr host = CreateBackendIndependentTextureHost(descriptor, nullptr, + RefPtr deallocator = new TestSurfaceAllocator(); + RefPtr host = CreateBackendIndependentTextureHost(descriptor, deallocator, LayersBackend::LAYERS_NONE, texture->GetFlags()); @@ -193,7 +195,8 @@ void TestTextureClientYCbCr(TextureClient* client, PlanarYCbCrData& ycbcrData) { ASSERT_EQ(ycbcrDesc.stereoMode(), ycbcrData.mStereoMode); // host deserialization - RefPtr textureHost = CreateBackendIndependentTextureHost(descriptor, nullptr, + RefPtr deallocator = new TestSurfaceAllocator(); + RefPtr textureHost = CreateBackendIndependentTextureHost(descriptor, deallocator, LayersBackend::LAYERS_NONE, client->GetFlags()); diff --git a/gfx/tests/gtest/TextureHelper.h b/gfx/tests/gtest/TextureHelper.h index 144a237b17d4..770f7464f829 100644 --- a/gfx/tests/gtest/TextureHelper.h +++ b/gfx/tests/gtest/TextureHelper.h @@ -140,6 +140,7 @@ CreateTextureClientWithBackend(LayersBackend aLayersBackend) */ already_AddRefed CreateTextureHostWithBackend(TextureClient* aClient, + ISurfaceAllocator* aDeallocator, LayersBackend& aLayersBackend) { if (!aClient) { @@ -153,7 +154,7 @@ CreateTextureHostWithBackend(TextureClient* aClient, aClient->ToSurfaceDescriptor(descriptor); wr::MaybeExternalImageId id = Nothing(); - return TextureHost::Create(descriptor, nullptr, aLayersBackend, + return TextureHost::Create(descriptor, aDeallocator, aLayersBackend, aClient->GetFlags(), id); } From d68b7067f1b78b6f6ce00bfc53aed171201c16ba Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Wed, 7 Feb 2018 14:54:48 +0100 Subject: [PATCH 11/20] Bug 1436353: Fix and enhance perf support in the jits; r=campbell MozReview-Commit-ID: IKyJf5jRIZu --HG-- extra : rebase_source : 56ba6731e9918b03e47441673d0eb37d8a8a5387 extra : histedit_source : cce41f271385a436d93289b576ccf0f47ade4f9d --- js/src/jit/CodeGenerator.cpp | 3 +- js/src/jit/PerfSpewer.cpp | 141 ++++++++++++++--------------------- js/src/jit/PerfSpewer.h | 14 +++- 3 files changed, 71 insertions(+), 87 deletions(-) diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index 51141c00f83c..5965fc3f3397 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -5505,7 +5505,8 @@ CodeGenerator::generateBody() TrackedOptimizations* last = nullptr; #if defined(JS_ION_PERF) - perfSpewer->startBasicBlock(current->mir(), masm); + if (!perfSpewer->startBasicBlock(current->mir(), masm)) + return false; #endif for (LInstructionIterator iter = current->begin(); iter != current->end(); iter++) { diff --git a/js/src/jit/PerfSpewer.cpp b/js/src/jit/PerfSpewer.cpp index e4592db0ef6d..0c91dd8d4bd6 100644 --- a/js/src/jit/PerfSpewer.cpp +++ b/js/src/jit/PerfSpewer.cpp @@ -129,24 +129,21 @@ js::jit::PerfFuncEnabled() { return PerfMode == PERF_MODE_FUNC; } -static bool -lockPerfMap(void) -{ - if (!PerfEnabled()) - return false; - - PerfMutex->lock(); - - MOZ_ASSERT(PerfFilePtr); - return true; -} - -static void -unlockPerfMap() -{ - MOZ_ASSERT(PerfFilePtr); - fflush(PerfFilePtr); - PerfMutex->unlock(); +namespace { + struct MOZ_RAII AutoLockPerfMap + { + AutoLockPerfMap() { + if (!PerfEnabled()) + return; + PerfMutex->lock(); + MOZ_ASSERT(PerfFilePtr); + } + ~AutoLockPerfMap() { + MOZ_ASSERT(PerfFilePtr); + fflush(PerfFilePtr); + PerfMutex->unlock(); + } + }; } uint32_t PerfSpewer::nextFunctionIndex = 0; @@ -173,24 +170,36 @@ PerfSpewer::startBasicBlock(MBasicBlock* blk, return basicBlocks_.append(r); } -bool +void PerfSpewer::endBasicBlock(MacroAssembler& masm) { if (!PerfBlockEnabled()) - return true; - + return; masm.bind(&basicBlocks_.back().end); - return true; } -bool +void PerfSpewer::noteEndInlineCode(MacroAssembler& masm) { if (!PerfBlockEnabled()) - return true; - + return; masm.bind(&endInlineCode); - return true; +} + +void +PerfSpewer::WriteEntry(const AutoLockPerfMap&, uintptr_t address, size_t size, + const char* fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + + auto result = mozilla::Vsmprintf(fmt, ap); + va_end(ap); + + fprintf(PerfFilePtr, "%" PRIxPTR " %zx %s\n", + address, + size, + result.get()); } void @@ -198,29 +207,19 @@ PerfSpewer::writeProfile(JSScript* script, JitCode* code, MacroAssembler& masm) { + AutoLockPerfMap lock; + if (PerfFuncEnabled()) { - if (!lockPerfMap()) - return; - uint32_t thisFunctionIndex = nextFunctionIndex++; - size_t size = code->instructionsSize(); if (size > 0) { - fprintf(PerfFilePtr, "%p %zx %s:%zu: Func%02d\n", - code->raw(), - size, - script->filename(), - script->lineno(), - thisFunctionIndex); + WriteEntry(lock, reinterpret_cast(code->raw()), size, "%s:%zu: Func%02" PRIu32, + script->filename(), script->lineno(), thisFunctionIndex); } - unlockPerfMap(); return; } if (PerfBlockEnabled() && basicBlocks_.length() > 0) { - if (!lockPerfMap()) - return; - uint32_t thisFunctionIndex = nextFunctionIndex++; uintptr_t funcStart = uintptr_t(code->raw()); uintptr_t funcEndInlineCode = funcStart + endInlineCode.offset(); @@ -230,8 +229,8 @@ PerfSpewer::writeProfile(JSScript* script, size_t prologueSize = basicBlocks_[0].start.offset(); if (prologueSize > 0) { - fprintf(PerfFilePtr, "%zx %zx %s:%zu: Func%02d-Prologue\n", - funcStart, prologueSize, script->filename(), script->lineno(), thisFunctionIndex); + WriteEntry(lock, funcStart, prologueSize, "%s:%zu: Func%02" PRIu32 "-Prologue", + script->filename(), script->lineno(), thisFunctionIndex); } uintptr_t cur = funcStart + prologueSize; @@ -243,41 +242,31 @@ PerfSpewer::writeProfile(JSScript* script, MOZ_ASSERT(cur <= blockStart); if (cur < blockStart) { - fprintf(PerfFilePtr, "%" PRIxPTR " %" PRIxPTR " %s:%zu: Func%02d-Block?\n", - cur, blockStart - cur, - script->filename(), script->lineno(), - thisFunctionIndex); + WriteEntry(lock, cur, blockStart - cur, "%s:%zu: Func%02" PRIu32 "-Block?", + script->filename(), script->lineno(), thisFunctionIndex); } cur = blockEnd; size_t size = blockEnd - blockStart; if (size > 0) { - fprintf(PerfFilePtr, "%" PRIxPTR " %zx %s:%d:%d: Func%02d-Block%d\n", - blockStart, size, - r.filename, r.lineNumber, r.columnNumber, - thisFunctionIndex, r.id); + WriteEntry(lock, blockStart, size, "%s:%u:%u: Func%02" PRIu32 "d-Block%" PRIu32, + r.filename, r.lineNumber, r.columnNumber, thisFunctionIndex, r.id); } } MOZ_ASSERT(cur <= funcEndInlineCode); if (cur < funcEndInlineCode) { - fprintf(PerfFilePtr, "%" PRIxPTR " %" PRIxPTR " %s:%zu: Func%02d-Epilogue\n", - cur, funcEndInlineCode - cur, - script->filename(), script->lineno(), - thisFunctionIndex); + WriteEntry(lock, cur, funcEndInlineCode - cur, "%s:%zu: Func%02" PRIu32 "-Epilogue", + script->filename(), script->lineno(), thisFunctionIndex); } MOZ_ASSERT(funcEndInlineCode <= funcEnd); if (funcEndInlineCode < funcEnd) { - fprintf(PerfFilePtr, "%" PRIxPTR " %" PRIxPTR " %s:%zu: Func%02d-OOL\n", - funcEndInlineCode, funcEnd - funcEndInlineCode, - script->filename(), script->lineno(), - thisFunctionIndex); + WriteEntry(lock, funcEndInlineCode, funcEnd - funcEndInlineCode, + "%s:%zu: Func%02" PRIu32 "-OOL", + script->filename(), script->lineno(), thisFunctionIndex); } - - unlockPerfMap(); - return; } } @@ -287,17 +276,12 @@ js::jit::writePerfSpewerBaselineProfile(JSScript* script, JitCode* code) if (!PerfEnabled()) return; - if (!lockPerfMap()) - return; - size_t size = code->instructionsSize(); if (size > 0) { - fprintf(PerfFilePtr, "%" PRIxPTR " %zx %s:%zu: Baseline\n", - reinterpret_cast(code->raw()), - size, script->filename(), script->lineno()); + AutoLockPerfMap lock; + PerfSpewer::WriteEntry(lock, reinterpret_cast(code->raw()), size, + "%s:%zu: Baseline", script->filename(), script->lineno()); } - - unlockPerfMap(); } void @@ -306,17 +290,13 @@ js::jit::writePerfSpewerJitCodeProfile(JitCode* code, const char* msg) if (!code || !PerfEnabled()) return; - if (!lockPerfMap()) - return; - size_t size = code->instructionsSize(); if (size > 0) { - fprintf(PerfFilePtr, "%" PRIxPTR " %zx %s (%p 0x%zx)\n", - reinterpret_cast(code->raw()), - size, msg, code->raw(), size); + AutoLockPerfMap lock; + PerfSpewer::WriteEntry(lock, reinterpret_cast(code->raw()), size, + "%s (%p 0x%zx)", msg, code->raw(), size); } - unlockPerfMap(); } void @@ -327,13 +307,8 @@ js::jit::writePerfSpewerWasmFunctionMap(uintptr_t base, uintptr_t size, if (!PerfFuncEnabled() || size == 0U) return; - if (!lockPerfMap()) - return; - - fprintf(PerfFilePtr, "%" PRIxPTR " %" PRIxPTR " %s:%u:%u: Function %s\n", - base, size, filename, lineno, colIndex, funcName); - - unlockPerfMap(); + AutoLockPerfMap lock; + PerfSpewer::WriteEntry(lock, base, size, "%s:%u: Function %s", filename, lineno, funcName); } #endif // defined (JS_ION_PERF) diff --git a/js/src/jit/PerfSpewer.h b/js/src/jit/PerfSpewer.h index 4fd44974660b..24632a0a89e2 100644 --- a/js/src/jit/PerfSpewer.h +++ b/js/src/jit/PerfSpewer.h @@ -12,6 +12,10 @@ # include "jit/MacroAssembler.h" #endif +namespace { + struct MOZ_RAII AutoLockPerfMap; +} + namespace js { namespace jit { @@ -67,10 +71,14 @@ class PerfSpewer public: virtual MOZ_MUST_USE bool startBasicBlock(MBasicBlock* blk, MacroAssembler& masm); - virtual MOZ_MUST_USE bool endBasicBlock(MacroAssembler& masm); - MOZ_MUST_USE bool noteEndInlineCode(MacroAssembler& masm); + virtual void endBasicBlock(MacroAssembler& masm); + void noteEndInlineCode(MacroAssembler& masm); void writeProfile(JSScript* script, JitCode* code, MacroAssembler& masm); + + static void WriteEntry(const AutoLockPerfMap&, uintptr_t address, size_t size, + const char* fmt, ...) + MOZ_FORMAT_PRINTF(4, 5); }; void writePerfSpewerBaselineProfile(JSScript* script, JitCode* code); @@ -81,7 +89,7 @@ class WasmPerfSpewer : public PerfSpewer { public: MOZ_MUST_USE bool startBasicBlock(MBasicBlock* blk, MacroAssembler& masm) { return true; } - MOZ_MUST_USE bool endBasicBlock(MacroAssembler& masm) { return true; } + void endBasicBlock(MacroAssembler& masm) { } }; void writePerfSpewerWasmFunctionMap(uintptr_t base, uintptr_t size, const char* filename, From 24d387c6b1d74bd4c694dcb20d140698456eb279 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Wed, 7 Feb 2018 14:55:47 +0100 Subject: [PATCH 12/20] Bug 1436353: add wasm perf support for entries/exits; r=luke MozReview-Commit-ID: 4kZEMpVP0BX --HG-- extra : rebase_source : 2ce8f06de6a03b1cfc3461cd50f60249fe90339e extra : histedit_source : c6dddaea3815d80a029358518f9d57cd45a15da1 --- js/src/jit/PerfSpewer.cpp | 12 +++++++++- js/src/jit/PerfSpewer.h | 4 +++- js/src/wasm/WasmCode.cpp | 47 ++++++++++++++++++++++++++++++--------- js/src/wasm/WasmTypes.h | 6 +++++ 4 files changed, 57 insertions(+), 12 deletions(-) diff --git a/js/src/jit/PerfSpewer.cpp b/js/src/jit/PerfSpewer.cpp index 0c91dd8d4bd6..fb51a957b9bf 100644 --- a/js/src/jit/PerfSpewer.cpp +++ b/js/src/jit/PerfSpewer.cpp @@ -296,12 +296,22 @@ js::jit::writePerfSpewerJitCodeProfile(JitCode* code, const char* msg) PerfSpewer::WriteEntry(lock, reinterpret_cast(code->raw()), size, "%s (%p 0x%zx)", msg, code->raw(), size); } +} +void +js::jit::writePerfSpewerWasmMap(uintptr_t base, uintptr_t size, const char* filename, + const char* annotation) +{ + if (!PerfFuncEnabled() || size == 0U) + return; + + AutoLockPerfMap lock; + PerfSpewer::WriteEntry(lock, base, size, "%s: Function %s", filename, annotation); } void js::jit::writePerfSpewerWasmFunctionMap(uintptr_t base, uintptr_t size, - const char* filename, unsigned lineno, unsigned colIndex, + const char* filename, unsigned lineno, const char* funcName) { if (!PerfFuncEnabled() || size == 0U) diff --git a/js/src/jit/PerfSpewer.h b/js/src/jit/PerfSpewer.h index 24632a0a89e2..1482d557487e 100644 --- a/js/src/jit/PerfSpewer.h +++ b/js/src/jit/PerfSpewer.h @@ -92,8 +92,10 @@ class WasmPerfSpewer : public PerfSpewer void endBasicBlock(MacroAssembler& masm) { } }; +void writePerfSpewerWasmMap(uintptr_t base, uintptr_t size, const char* filename, + const char* annotation); void writePerfSpewerWasmFunctionMap(uintptr_t base, uintptr_t size, const char* filename, - unsigned lineno, unsigned colIndex, const char* funcName); + unsigned lineno, const char* funcName); #endif // JS_ION_PERF diff --git a/js/src/wasm/WasmCode.cpp b/js/src/wasm/WasmCode.cpp index 22ca995c306a..e41b07559c2c 100644 --- a/js/src/wasm/WasmCode.cpp +++ b/js/src/wasm/WasmCode.cpp @@ -139,6 +139,14 @@ StaticallyUnlink(uint8_t* base, const LinkDataTier& linkData) } } +#ifdef JS_ION_PERF +static bool +AppendToString(const char* str, UTF8Bytes* bytes) +{ + return bytes->append(str, strlen(str)) && bytes->append('\0'); +} +#endif + static void SendCodeRangesToProfiler(const CodeSegment& cs, const Bytes& bytecode, const Metadata& metadata) { @@ -153,18 +161,15 @@ SendCodeRangesToProfiler(const CodeSegment& cs, const Bytes& bytecode, const Met return; for (const CodeRange& codeRange : metadata.metadata(cs.tier()).codeRanges) { - if (!codeRange.isFunction()) + if (!codeRange.hasFuncIndex()) continue; uintptr_t start = uintptr_t(cs.base() + codeRange.begin()); - uintptr_t end = uintptr_t(cs.base() + codeRange.end()); - uintptr_t size = end - start; + uintptr_t size = codeRange.end() - codeRange.begin(); UTF8Bytes name; if (!metadata.getFuncName(&bytecode, codeRange.funcIndex(), &name)) return; - if (!name.append('\0')) - return; // Avoid "unused" warnings (void)start; @@ -173,14 +178,36 @@ SendCodeRangesToProfiler(const CodeSegment& cs, const Bytes& bytecode, const Met #ifdef JS_ION_PERF if (PerfFuncEnabled()) { const char* file = metadata.filename.get(); - unsigned line = codeRange.funcLineOrBytecode(); - unsigned column = 0; - writePerfSpewerWasmFunctionMap(start, size, file, line, column, name.begin()); + if (codeRange.isFunction()) { + if (!name.append('\0')) + return; + unsigned line = codeRange.funcLineOrBytecode(); + writePerfSpewerWasmFunctionMap(start, size, file, line, name.begin()); + } else if (codeRange.isInterpEntry()) { + if (!AppendToString(" slow entry", &name)) + return; + writePerfSpewerWasmMap(start, size, file, name.begin()); + } else if (codeRange.isImportInterpExit()) { + if (!AppendToString(" slow exit", &name)) + return; + writePerfSpewerWasmMap(start, size, file, name.begin()); + } else if (codeRange.isImportJitExit()) { + if (!AppendToString(" fast exit", &name)) + return; + writePerfSpewerWasmMap(start, size, file, name.begin()); + } else { + MOZ_CRASH("unhandled perf hasFuncIndex type"); + } } #endif #ifdef MOZ_VTUNE - if (vtune::IsProfilingActive()) - vtune::MarkWasm(vtune::GenerateUniqueMethodID(), name.begin(), (void*)start, size); + if (!vtune::IsProfilingActive()) + continue; + if (!codeRange.isFunction()) + continue; + if (!name.append('\0')) + return; + vtune::MarkWasm(vtune::GenerateUniqueMethodID(), name.begin(), (void*)start, size); #endif } } diff --git a/js/src/wasm/WasmTypes.h b/js/src/wasm/WasmTypes.h index ed31917f7767..bd98c52fcccb 100644 --- a/js/src/wasm/WasmTypes.h +++ b/js/src/wasm/WasmTypes.h @@ -1126,9 +1126,15 @@ class CodeRange bool isFunction() const { return kind() == Function; } + bool isInterpEntry() const { + return kind() == InterpEntry; + } bool isImportExit() const { return kind() == ImportJitExit || kind() == ImportInterpExit || kind() == BuiltinThunk; } + bool isImportInterpExit() const { + return kind() == ImportInterpExit; + } bool isImportJitExit() const { return kind() == ImportJitExit; } From e6536d905f4ba0190e29c2340a2378732e91140b Mon Sep 17 00:00:00 2001 From: Luke Wagner Date: Wed, 7 Feb 2018 10:06:54 -0600 Subject: [PATCH 13/20] Bug 1435525 - Baldr: eagerly reject too-big code section sizes and clamp masm reservation size (r=lth) --HG-- extra : rebase_source : 117620ce71c1afb5d42347f257432798734bdf8a --- js/src/wasm/WasmBinaryConstants.h | 3 ++- js/src/wasm/WasmGenerator.cpp | 8 ++++---- js/src/wasm/WasmJS.cpp | 2 +- js/src/wasm/WasmValidate.cpp | 3 +++ 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/js/src/wasm/WasmBinaryConstants.h b/js/src/wasm/WasmBinaryConstants.h index cab84708e3aa..882551ca30bc 100644 --- a/js/src/wasm/WasmBinaryConstants.h +++ b/js/src/wasm/WasmBinaryConstants.h @@ -587,7 +587,8 @@ static const unsigned MaxParams = 1000; static const unsigned MaxBrTableElems = 1000000; static const unsigned MaxMemoryInitialPages = 16384; static const unsigned MaxMemoryMaximumPages = 65536; -static const unsigned MaxModuleBytes = 1024 * 1024 * 1024; +static const unsigned MaxCodeSectionBytes = 1024 * 1024 * 1024; +static const unsigned MaxModuleBytes = MaxCodeSectionBytes; static const unsigned MaxFunctionBytes = 7654321; // A magic value of the FramePointer to indicate after a return to the entry diff --git a/js/src/wasm/WasmGenerator.cpp b/js/src/wasm/WasmGenerator.cpp index 9666145a0380..336b0f3b1569 100644 --- a/js/src/wasm/WasmGenerator.cpp +++ b/js/src/wasm/WasmGenerator.cpp @@ -211,9 +211,9 @@ ModuleGenerator::init(Metadata* maybeAsmJSMetadata) // extra conservative. Note, podResizeToFit calls at the end will trim off // unneeded capacity. - uint32_t codeSectionSize = env_->codeSection ? env_->codeSection->size : 0; - - if (!masm_.reserve(size_t(1.2 * EstimateCompiledCodeSize(tier(), codeSectionSize)))) + size_t codeSectionSize = env_->codeSection ? env_->codeSection->size : 0; + size_t estimatedCodeSize = 1.2 * EstimateCompiledCodeSize(tier(), codeSectionSize); + if (!masm_.reserve(Min(estimatedCodeSize, MaxCodeBytesPerProcess))) return false; if (!metadataTier_->codeRanges.reserve(2 * env_->numFuncDefs())) @@ -782,7 +782,7 @@ ModuleGenerator::compileFuncDef(uint32_t funcIndex, uint32_t lineOrBytecode, } batchedBytecode_ += funcBytecodeLength; - MOZ_ASSERT(batchedBytecode_ <= MaxModuleBytes); + MOZ_ASSERT(batchedBytecode_ <= MaxCodeSectionBytes); return batchedBytecode_ <= threshold || launchBatchCompile(); } diff --git a/js/src/wasm/WasmJS.cpp b/js/src/wasm/WasmJS.cpp index b93bb400bd82..bb60df872d17 100644 --- a/js/src/wasm/WasmJS.cpp +++ b/js/src/wasm/WasmJS.cpp @@ -2518,7 +2518,7 @@ class CompileStreamTask : public PromiseHelperTask, public JS::StreamConsumer if (extraBytes) envBytes_.shrinkTo(codeSection_.start); - if (codeSection_.size > MaxModuleBytes) + if (codeSection_.size > MaxCodeSectionBytes) return rejectAndDestroyBeforeHelperThreadStarted(JSMSG_OUT_OF_MEMORY); if (!codeBytes_.resize(codeSection_.size)) diff --git a/js/src/wasm/WasmValidate.cpp b/js/src/wasm/WasmValidate.cpp index 9aab571b3771..d31596dbe544 100644 --- a/js/src/wasm/WasmValidate.cpp +++ b/js/src/wasm/WasmValidate.cpp @@ -1722,6 +1722,9 @@ wasm::DecodeModuleEnvironment(Decoder& d, ModuleEnvironment* env) if (!d.startSection(SectionId::Code, env, &env->codeSection, "code")) return false; + if (env->codeSection && env->codeSection->size > MaxCodeSectionBytes) + return d.fail("code section too big"); + return true; } From a9b8d6c731f42d5e877cd26b4c8842bdb4579060 Mon Sep 17 00:00:00 2001 From: Luke Wagner Date: Wed, 7 Feb 2018 10:16:44 -0600 Subject: [PATCH 14/20] Bug 1436353: fix non-unified build bustage (rs=me) --- js/src/jit/PerfSpewer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/jit/PerfSpewer.h b/js/src/jit/PerfSpewer.h index 1482d557487e..1ff18b6a42e3 100644 --- a/js/src/jit/PerfSpewer.h +++ b/js/src/jit/PerfSpewer.h @@ -13,7 +13,7 @@ #endif namespace { - struct MOZ_RAII AutoLockPerfMap; + struct AutoLockPerfMap; } namespace js { From 4aade813b46f3f2c0181cb514f90a58b990d80fe Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Wed, 7 Feb 2018 17:30:13 +0100 Subject: [PATCH 15/20] Bug 1435772 - File extensions are stripped out from saved web page source, leading to 404 errors r=me Backs out the part of part of changeset a20fbbe7b948 (Bug 1432602) that uses nsIURIMutator instead of calling nsIFileURL::SetFile() MozReview-Commit-ID: 9mC3fv85pUl --- dom/webbrowserpersist/nsWebBrowserPersist.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/dom/webbrowserpersist/nsWebBrowserPersist.cpp b/dom/webbrowserpersist/nsWebBrowserPersist.cpp index 3a06c3f76db5..c388c97a7131 100644 --- a/dom/webbrowserpersist/nsWebBrowserPersist.cpp +++ b/dom/webbrowserpersist/nsWebBrowserPersist.cpp @@ -2113,10 +2113,10 @@ nsWebBrowserPersist::CalculateUniqueFilename(nsIURI *aURI, nsCOMPtr& aOu localFile->SetLeafName(filenameAsUnichar); // Resync the URI with the file after the extension has been appended - return NS_MutateURI(aURI) - .Apply(&nsIFileURLMutator::SetFile, - localFile) - .Finalize(aOutURI); + nsresult rv; + nsCOMPtr fileURL = do_QueryInterface(aURI, &rv); + NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE); + fileURL->SetFile(localFile); // this should recalculate uri } else { @@ -2292,10 +2292,9 @@ nsWebBrowserPersist::CalculateAndAppendFileExt(nsIURI *aURI, localFile->SetLeafName(NS_ConvertUTF8toUTF16(newFileName)); // Resync the URI with the file after the extension has been appended - return NS_MutateURI(aURI) - .Apply(&nsIFileURLMutator::SetFile, - localFile) - .Finalize(aOutURI); + nsCOMPtr fileURL = do_QueryInterface(aURI, &rv); + NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE); + fileURL->SetFile(localFile); // this should recalculate uri } else { From ad0fd7d75a74247ffee28626a9c3b67b092e7133 Mon Sep 17 00:00:00 2001 From: Tim Taubert Date: Wed, 7 Feb 2018 16:37:54 +0100 Subject: [PATCH 16/20] Bug 1406458 - Add WebAuthn extension types r=jcj,baku Summary: This only adds the new WebIDL types but doesn't do any plumbing yet. Bug 1406471 seems to be better suited for that. Reviewers: jcj Reviewed By: jcj Bug #: 1406458 Differential Revision: https://phabricator.services.mozilla.com/D555 --- dom/webidl/WebAuthentication.webidl | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/dom/webidl/WebAuthentication.webidl b/dom/webidl/WebAuthentication.webidl index bfe56e6f063b..ff714d80d86b 100644 --- a/dom/webidl/WebAuthentication.webidl +++ b/dom/webidl/WebAuthentication.webidl @@ -55,8 +55,7 @@ dictionary MakePublicKeyCredentialOptions { sequence excludeCredentials = []; AuthenticatorSelectionCriteria authenticatorSelection; AttestationConveyancePreference attestation = "none"; - // Extensions are not supported yet. - // AuthenticationExtensions extensions; // Add in Bug 1406458 + AuthenticationExtensionsClientInputs extensions; }; dictionary PublicKeyCredentialEntity { @@ -102,11 +101,16 @@ dictionary PublicKeyCredentialRequestOptions { USVString rpId; sequence allowCredentials = []; UserVerificationRequirement userVerification = "preferred"; - // Extensions are not supported yet. - // AuthenticationExtensions extensions; // Add in Bug 1406458 + AuthenticationExtensionsClientInputs extensions; }; -typedef record AuthenticationExtensions; +dictionary AuthenticationExtensionsClientInputs { +}; + +dictionary AuthenticationExtensionsClientOutputs { +}; + +typedef record AuthenticationExtensionsAuthenticatorInputs; dictionary CollectedClientData { required DOMString type; @@ -114,9 +118,8 @@ dictionary CollectedClientData { required DOMString origin; required DOMString hashAlgorithm; DOMString tokenBindingId; - // Extensions are not supported yet. - // AuthenticationExtensions clientExtensions; // Add in Bug 1406458 - // AuthenticationExtensions authenticatorExtensions; // Add in Bug 1406458 + AuthenticationExtensionsClientInputs clientExtensions; + AuthenticationExtensionsAuthenticatorInputs authenticatorExtensions; }; enum PublicKeyCredentialType { From 7283c849b7472450fdb2f871eed7ec0f06e0de2c Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Wed, 7 Feb 2018 17:56:34 +0100 Subject: [PATCH 17/20] Bug 1435266 - Enable Spectre index masking by default. r=luke --- js/src/jit/JitOptions.cpp | 2 +- modules/libpref/init/all.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/js/src/jit/JitOptions.cpp b/js/src/jit/JitOptions.cpp index b1039c81d81b..0ec5ad33b23d 100644 --- a/js/src/jit/JitOptions.cpp +++ b/js/src/jit/JitOptions.cpp @@ -232,7 +232,7 @@ DefaultJitOptions::DefaultJitOptions() Warn(forcedRegisterAllocatorEnv, env); } - SET_DEFAULT(spectreIndexMasking, false); + SET_DEFAULT(spectreIndexMasking, true); // Toggles whether unboxed plain objects can be created by the VM. SET_DEFAULT(disableUnboxedObjects, false); diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index ff2dc62adb41..953b729a3447 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -1549,7 +1549,7 @@ pref("javascript.options.throw_on_debuggee_would_run", false); pref("javascript.options.dump_stack_on_debuggee_would_run", false); // Spectre security vulnerability mitigations. -pref("javascript.options.spectre.index_masking", false); +pref("javascript.options.spectre.index_masking", true); // Streams API pref("javascript.options.streams", false); From 605269a01f930b50212f48ce2bd91800da14f3ae Mon Sep 17 00:00:00 2001 From: Jason Laster Date: Wed, 7 Feb 2018 18:04:07 +0100 Subject: [PATCH 18/20] Bug 1436210 - Update Debugger Frontend v14. r=jdescottes. MozReview-Commit-ID: 7hHRQLx5DaZ --HG-- extra : rebase_source : 7984fae7323c975b3d5586af5c988cf94f47057c --- devtools/client/debugger/new/README.mozilla | 6 +- devtools/client/debugger/new/debugger.css | 95 +- devtools/client/debugger/new/debugger.js | 899 ++++++++++++------ devtools/client/debugger/new/parser-worker.js | 164 +++- .../browser_dbg-sourcemaps-reloading.js | 5 +- .../test/mochitest/browser_dbg-sourcemaps.js | 4 +- .../new/test/mochitest/browser_dbg-sources.js | 6 +- .../debugger/new/test/mochitest/head.js | 30 +- .../client/locales/en-US/debugger.properties | 34 +- devtools/client/preferences/debugger.js | 2 +- 10 files changed, 850 insertions(+), 395 deletions(-) diff --git a/devtools/client/debugger/new/README.mozilla b/devtools/client/debugger/new/README.mozilla index 32ea52b16533..72c7fcd0f3d6 100644 --- a/devtools/client/debugger/new/README.mozilla +++ b/devtools/client/debugger/new/README.mozilla @@ -1,9 +1,9 @@ This is the debugger.html project output. See https://github.com/devtools-html/debugger.html -Version 13.0 -Comparison: https://github.com/devtools-html/debugger.html/compare/release-12...release-13 -Commit: https://github.com/devtools-html/debugger.html/commit/3c10bd43b5 +Version 14.0 +Comparison: https://github.com/devtools-html/debugger.html/compare/release-13...release-14 +Commit: https://github.com/devtools-html/debugger.html/commit/5805cf467 Packages: - babel-plugin-transform-es2015-modules-commonjs @6.26.0 diff --git a/devtools/client/debugger/new/debugger.css b/devtools/client/debugger/new/debugger.css index 54641f2f571a..2f9bab833fc6 100644 --- a/devtools/client/debugger/new/debugger.css +++ b/devtools/client/debugger/new/debugger.css @@ -956,7 +956,7 @@ menuseparator { .file, .folder, .domain { - background-color: var(--theme-splitter-color); + background-color: var(--theme-comment); } .worker, @@ -1022,7 +1022,7 @@ img.arrow { width: 9px; height: 9px; padding-top: 9px; - background: var(--theme-splitter-color); + background: var(--disclosure-arrow); mask-size: 100%; display: inline-block; margin-bottom: 1px; @@ -1067,7 +1067,8 @@ html .arrow.expanded svg { fill: var(--theme-body-color); } -.angular svg, .source-icon svg { +.angular svg, +.source-icon svg { width: 15px; height: 15px; margin-right: 5px; @@ -1095,6 +1096,11 @@ html .arrow.expanded svg { white-space: nowrap; overflow: auto; min-width: 100%; + + display: grid; + grid-template-columns: 1fr; + align-content: start; + } .managed-tree .tree button { @@ -1233,7 +1239,7 @@ html[dir="rtl"] .arrow svg, .close-btn .close { mask: url("chrome://devtools/skin/images/debugger/close.svg") no-repeat; mask-size: 100%; - background-color: var(--theme-comment-alt); + background-color: var(--theme-comment); width: 8px; height: 8px; transition: all 0.15s ease-in-out; @@ -1317,11 +1323,11 @@ html[dir="rtl"] .arrow svg, .search-field .magnifying-glass path, .search-field .magnifying-glass ellipse { - stroke: var(--theme-splitter-color); + stroke: var(--theme-comment); } .search-field input::placeholder { - color: var(--theme-body-color-inactive); + color: var(--theme-toolbar-color); } .search-field input:focus { @@ -1399,6 +1405,7 @@ html[dir="rtl"] .arrow svg, padding: 4px 0 4px 30px; line-height: 16px; font-size: 10px; + width: 100%; } .project-text-search .matches-summary { @@ -1514,8 +1521,7 @@ html[dir="rtl"] .arrow svg, } .sources-list .managed-tree .tree .node { - padding: 0px 0px 0px 3px; - height: 21px; + padding: 0 10px 0 3px; width: 100%; } @@ -1531,10 +1537,6 @@ html[dir="rtl"] .arrow svg, transform: rotate(0deg); } -.theme-dark .sources-list .tree .node:not(.focused) img.arrow { - background: var(--theme-content-color3); -} - .theme-dark .source-list .tree .node.focused { background-color: var(--theme-tab-toolbar-background); } @@ -1669,7 +1671,7 @@ html[dir="rtl"] .arrow svg, .outline-list { list-style-type: none; - width: 100%; + flex: 1 0 100%; padding: 10px 0px; margin: 0; } @@ -1688,19 +1690,18 @@ html[dir="rtl"] .arrow svg, } .outline-list__element { - padding-bottom: 0.5rem; - padding-right: 0.5rem; - padding-top: 0.2rem; + padding: 3px 10px 3px 10px; cursor: default; white-space: nowrap; } -.outline-list__element:hover { - background: var(--theme-toolbar-background-hover); +.outline-list__element-icon { + padding-right: 0.4rem; + padding-left: 1rem; } -.outline-list__element .function { - padding-left: 10px; +.outline-list__element:hover { + background: var(--theme-toolbar-background-hover); } /* 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 @@ -1965,7 +1966,7 @@ html .toggle-button.end.vertical svg { } .search-bottom-bar .search-modifiers button svg { - fill: var(--theme-comment-alt); + fill: var(--theme-comment); height: 16px; width: 16px; } @@ -1997,7 +1998,7 @@ html .toggle-button.end.vertical svg { margin: 0 0 0 6px; border: none; background: transparent; - color: var(--theme-comment-alt); + color: var(--theme-comment); } .search-bottom-bar .search-type-toggles .search-type-btn:active { @@ -2989,8 +2990,7 @@ html .breakpoints-list .breakpoint.paused { .input-expression::placeholder { text-align: center; font-style: italic; - color: var(--theme-comment-alt); - opacity: 1; + color: var(--theme-comment); } .input-expression:focus { @@ -3309,10 +3309,12 @@ html .breakpoints-list .breakpoint.paused { :root { --accordion-header-background: var(--theme-toolbar-background); + --disclosure-arrow: #b2b2b2; } :root.theme-dark { --accordion-header-background: #222225; + --disclosure-arrow: #7f7f81; } .accordion { @@ -3383,6 +3385,10 @@ html .breakpoints-list .breakpoint.paused { .accordion .header-buttons button::-moz-focus-inner { border: none; } + +.accordion .arrow svg { + fill: var(--disclosure-arrow); +} /* 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 . */ @@ -3408,7 +3414,11 @@ img.pause, img.stepOver, img.stepIn, img.stepOut, -img.resume { +img.resume, +img.rewind, +img.reverseStepOver, +img.reverseStepIn, +img.reverseStepOut { background-color: var(--theme-body-color); } @@ -3432,6 +3442,26 @@ img.resume { mask: url("chrome://devtools/skin/images/debugger/resume.svg") no-repeat; } +.command-bar img.rewind { + mask: url("chrome://devtools/skin/images/debugger/resume.svg") no-repeat; + transform: scaleX(-1); +} + +.command-bar img.reverseStepOver { + mask: url("chrome://devtools/skin/images/debugger/stepOver.svg") no-repeat; + transform: scaleX(-1); +} + +.command-bar img.reverseStepIn { + mask: url("chrome://devtools/skin/images/debugger/stepIn.svg") no-repeat; + transform: scaleX(-1); +} + +.command-bar img.reverseStepOut { + mask: url("chrome://devtools/skin/images/debugger/stepOut.svg") no-repeat; + transform: scaleX(-1); +} + .command-bar > .pause-exceptions.uncaught.enabled > img.pause-exceptions { background-color: var(--theme-highlight-purple); } @@ -3697,10 +3727,6 @@ img.ignore-exceptions { cursor: default; } -.theme-dark .secondary-panes .accordion .arrow svg { - fill: var(--theme-content-color3); -} - .secondary-panes .breakpoints-buttons { display: flex; } @@ -3734,7 +3760,6 @@ img.ignore-exceptions { padding: 50px 0 0 0; text-align: center; font-size: 1.25em; - color: var(--theme-comment-alt); background-color: var(--theme-toolbar-background); font-weight: lighter; z-index: 10; @@ -3777,6 +3802,7 @@ img.ignore-exceptions { text-align: left; float: left; margin-left: 25px; + color: var(--theme-comment); } html .welcomebox .toggle-button-end.collapsed { @@ -4033,15 +4059,16 @@ html .welcomebox .toggle-button-end.collapsed { .theme-dark .result-list { background-color: var(--theme-body-background); } -.result-item .title .fuzzy-match { +.result-item .title .highlight { font-weight: bold; } -.result-item .subtitle .fuzzy-match { +.result-item .subtitle .highlight { color: var(--grey-90); + font-weight: 500; } -.theme-dark .result-item .title .fuzzy-match, -.theme-dark .result-item .subtitle .fuzzy-match { +.theme-dark .result-item .title .highlight, +.theme-dark .result-item .subtitle .highlight { color: white; } diff --git a/devtools/client/debugger/new/debugger.js b/devtools/client/debugger/new/debugger.js index 8b9e5ecbdeba..8cc90a8138d5 100644 --- a/devtools/client/debugger/new/debugger.js +++ b/devtools/client/debugger/new/debugger.js @@ -10138,7 +10138,7 @@ if (isDevelopment()) { pref("devtools.debugger.features.root", true); pref("devtools.debugger.features.column-breakpoints", false); pref("devtools.debugger.features.chrome-scopes", false); - pref("devtools.debugger.features.map-scopes", false); + pref("devtools.debugger.features.map-scopes", true); pref("devtools.debugger.features.breakpoints-dropdown", true); pref("devtools.debugger.features.remove-command-bar-options", true); pref("devtools.debugger.features.code-coverage", false); @@ -13925,7 +13925,7 @@ module.exports = "Group 2Created with Sketch." - -/***/ }), +/* 1787 */, /* 1788 */ /***/ (function(module, exports) { @@ -47523,7 +47802,16 @@ function getScope(scope, selectedFrame, frameScopes, why, scopeIndex) { if (isLocalScope) { vars = vars.concat((0, _utils.getFramePopVariables)(why, key)); - const this_ = (0, _utils.getThisVariable)(selectedFrame, key); + let thisDesc_ = selectedFrame.this; + + if ("this" in bindings) { + // The presence of "this" means we're rendering a "this" binding + // generated from mapScopes and this can override the binding + // provided by the current frame. + thisDesc_ = bindings.this ? bindings.this.value : null; + } + + const this_ = (0, _utils.getThisVariable)(thisDesc_, key); if (this_) { vars.push(this_); @@ -48477,7 +48765,7 @@ function showContextMenu(props) { const otherEnabledBreakpoints = breakpoints.filter(b => !b.disabled && b !== breakpoint); const otherDisabledBreakpoints = breakpoints.filter(b => b.disabled && b !== breakpoint); - const deleteSelf = { + const deleteSelfItem = { id: "node-menu-delete-self", label: deleteSelfLabel, accesskey: deleteSelfKey, @@ -48485,7 +48773,7 @@ function showContextMenu(props) { click: () => removeBreakpoint(breakpoint.location) }; - const deleteAll = { + const deleteAllItem = { id: "node-menu-delete-all", label: deleteAllLabel, accesskey: deleteAllKey, @@ -48493,7 +48781,7 @@ function showContextMenu(props) { click: () => removeAllBreakpoints() }; - const deleteOthers = { + const deleteOthersItem = { id: "node-menu-delete-other", label: deleteOthersLabel, accesskey: deleteOthersKey, @@ -48501,7 +48789,7 @@ function showContextMenu(props) { click: () => removeBreakpoints(otherBreakpoints) }; - const enableSelf = { + const enableSelfItem = { id: "node-menu-enable-self", label: enableSelfLabel, accesskey: enableSelfKey, @@ -48509,7 +48797,7 @@ function showContextMenu(props) { click: () => toggleDisabledBreakpoint(breakpoint.location.line) }; - const enableAll = { + const enableAllItem = { id: "node-menu-enable-all", label: enableAllLabel, accesskey: enableAllKey, @@ -48517,7 +48805,7 @@ function showContextMenu(props) { click: () => toggleAllBreakpoints(false) }; - const enableOthers = { + const enableOthersItem = { id: "node-menu-enable-others", label: enableOthersLabel, accesskey: enableOthersKey, @@ -48525,7 +48813,7 @@ function showContextMenu(props) { click: () => toggleBreakpoints(false, otherDisabledBreakpoints) }; - const disableSelf = { + const disableSelfItem = { id: "node-menu-disable-self", label: disableSelfLabel, accesskey: disableSelfKey, @@ -48533,7 +48821,7 @@ function showContextMenu(props) { click: () => toggleDisabledBreakpoint(breakpoint.location.line) }; - const disableAll = { + const disableAllItem = { id: "node-menu-disable-all", label: disableAllLabel, accesskey: disableAllKey, @@ -48541,14 +48829,14 @@ function showContextMenu(props) { click: () => toggleAllBreakpoints(true) }; - const disableOthers = { + const disableOthersItem = { id: "node-menu-disable-others", label: disableOthersLabel, accesskey: disableOthersKey, click: () => toggleBreakpoints(true, otherEnabledBreakpoints) }; - const removeCondition = { + const removeConditionItem = { id: "node-menu-remove-condition", label: removeConditionLabel, accesskey: removeConditionKey, @@ -48556,7 +48844,7 @@ function showContextMenu(props) { click: () => setBreakpointCondition(breakpoint.location) }; - const addCondition = { + const addConditionItem = { id: "node-menu-add-condition", label: addConditionLabel, accesskey: addConditionKey, @@ -48566,7 +48854,7 @@ function showContextMenu(props) { } }; - const editCondition = { + const editConditionItem = { id: "node-menu-edit-condition", label: editConditionLabel, accesskey: editConditionKey, @@ -48576,29 +48864,29 @@ function showContextMenu(props) { } }; - const hideEnableSelf = !breakpoint.disabled; - const hideEnableAll = disabledBreakpoints.size === 0; - const hideEnableOthers = otherDisabledBreakpoints.size === 0; - const hideDisableAll = enabledBreakpoints.size === 0; - const hideDisableOthers = otherEnabledBreakpoints.size === 0; - const hideDisableSelf = breakpoint.disabled; + const hideEnableSelfItem = !breakpoint.disabled; + const hideEnableAllItem = disabledBreakpoints.size === 0; + const hideEnableOthersItem = otherDisabledBreakpoints.size === 0; + const hideDisableAllItem = enabledBreakpoints.size === 0; + const hideDisableOthersItem = otherEnabledBreakpoints.size === 0; + const hideDisableSelfItem = breakpoint.disabled; - const items = [{ item: enableSelf, hidden: () => hideEnableSelf }, { item: enableAll, hidden: () => hideEnableAll }, { item: enableOthers, hidden: () => hideEnableOthers }, { + const items = [{ item: enableSelfItem, hidden: () => hideEnableSelfItem }, { item: enableAllItem, hidden: () => hideEnableAllItem }, { item: enableOthersItem, hidden: () => hideEnableOthersItem }, { item: { type: "separator" }, - hidden: () => hideEnableSelf && hideEnableAll && hideEnableOthers - }, { item: deleteSelf }, { item: deleteAll }, { item: deleteOthers, hidden: () => breakpoints.size === 1 }, { + hidden: () => hideEnableSelfItem && hideEnableAllItem && hideEnableOthersItem + }, { item: deleteSelfItem }, { item: deleteAllItem }, { item: deleteOthersItem, hidden: () => breakpoints.size === 1 }, { item: { type: "separator" }, - hidden: () => hideDisableSelf && hideDisableAll && hideDisableOthers - }, { item: disableSelf, hidden: () => hideDisableSelf }, { item: disableAll, hidden: () => hideDisableAll }, { item: disableOthers, hidden: () => hideDisableOthers }, { + hidden: () => hideDisableSelfItem && hideDisableAllItem && hideDisableOthersItem + }, { item: disableSelfItem, hidden: () => hideDisableSelfItem }, { item: disableAllItem, hidden: () => hideDisableAllItem }, { item: disableOthersItem, hidden: () => hideDisableOthersItem }, { item: { type: "separator" } }, { - item: addCondition, + item: addConditionItem, hidden: () => breakpoint.condition }, { - item: editCondition, + item: editConditionItem, hidden: () => !breakpoint.condition }, { - item: removeCondition, + item: removeConditionItem, hidden: () => !breakpoint.condition }]; @@ -49628,9 +49916,12 @@ function getSourceAnnotation(source, getMetaData) { const sourceId = source.get("id"); const sourceMetaData = getMetaData(sourceId); - if (sourceMetaData && sourceMetaData.isReactComponent) { - return _react2.default.createElement("img", { className: "react" }); + const framework = sourceMetaData && sourceMetaData.framework ? sourceMetaData.framework : false; + + if (framework) { + return _react2.default.createElement("img", { className: framework.toLowerCase() }); } + if ((0, _source.isPretty)(source)) { return _react2.default.createElement("img", { className: "prettyPrint" }); } diff --git a/devtools/client/debugger/new/parser-worker.js b/devtools/client/debugger/new/parser-worker.js index 3ea53a90b511..ee79be9fb3ac 100644 --- a/devtools/client/debugger/new/parser-worker.js +++ b/devtools/client/debugger/new/parser-worker.js @@ -33263,7 +33263,24 @@ let symbolDeclarations = new Map(); function getFunctionParameterNames(path) { if (path.node.params != null) { - return path.node.params.map(param => param.name); + return path.node.params.map(param => { + if (param.type !== "AssignmentPattern") { + return param.name; + } + + // Parameter with default value + if (param.left.type === "Identifier" && param.right.type === "Identifier") { + return `${param.left.name} = ${param.right.name}`; + } else if (param.left.type === "Identifier" && param.right.type === "StringLiteral") { + return `${param.left.name} = ${param.right.value}`; + } else if (param.left.type === "Identifier" && param.right.type === "ObjectExpression") { + return `${param.left.name} = {}`; + } else if (param.left.type === "Identifier" && param.right.type === "ArrayExpression") { + return `${param.left.name} = []`; + } else if (param.left.type === "Identifier" && param.right.type === "NullLiteral") { + return `${param.left.name} = null`; + } + }); } return []; } @@ -33861,7 +33878,7 @@ self.onmessage = workerHandler({ getNextStep: _steps.getNextStep, getEmptyLines: _getEmptyLines2.default, hasSyntaxError: _validate.hasSyntaxError, - isReactComponent: _frameworks.isReactComponent + getFramework: _frameworks.getFramework }); /***/ }), @@ -34598,7 +34615,7 @@ module.exports = findLastIndex; Object.defineProperty(exports, "__esModule", { value: true }); -exports.isReactComponent = isReactComponent; +exports.getFramework = getFramework; var _getSymbols = __webpack_require__(1457); @@ -34606,13 +34623,19 @@ var _getSymbols2 = _interopRequireDefault(_getSymbols); function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; } -function isReactComponent(sourceId) { - const { imports, classes, callExpressions } = (0, _getSymbols2.default)(sourceId); - return (importsReact(imports) || requiresReact(callExpressions)) && extendsComponent(classes); +function getFramework(sourceId) { + if (isReactComponent(sourceId)) { + return "React"; + } } /* 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 . */ +function isReactComponent(sourceId) { + const { imports, classes, callExpressions } = (0, _getSymbols2.default)(sourceId); + return (importsReact(imports) || requiresReact(callExpressions)) && extendsComponent(classes); +} + function importsReact(imports) { return imports.some(importObj => importObj.source === "react" && importObj.specifiers.some(specifier => specifier === "React")); } @@ -35266,6 +35289,20 @@ var _getFunctionName2 = _interopRequireDefault(_getFunctionName); function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; } +/** + * "implicit" + * Variables added automaticly like "this" and "arguments" + * + * "var" + * Variables declared with "var" or non-block function declarations + * + * "let" + * Variables declared with "let". + * + * "const" + * Variables declared with "const", imported bindings, or added as const + * bindings like inner function expressions and inner class names. + */ /* 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/. */ @@ -35289,7 +35326,7 @@ function isNode(node, type) { return node ? node.type === type : false; } -function getFunctionScope(scope) { +function getVarScope(scope) { let s = scope; while (s.type !== "function" && s.type !== "module") { if (!s.parent) { @@ -35361,29 +35398,22 @@ function toParsedScopes(children, sourceId) { return undefined; } return children.map(scope => { - // Removing unneed information from TempScope such as parent reference and - // name types. We also need to convert BabelLocation to the Location type. + // Removing unneed information from TempScope such as parent reference. + // We also need to convert BabelLocation to the Location type. const bindings = Object.keys(scope.names).reduce((_bindings, n) => { const nameRefs = scope.names[n]; - switch (nameRefs.type) { - case "var": - case "let": - case "const": - case "param": - case "fn": - case "import": - _bindings[n] = { - declarations: nameRefs.declarations.map(({ start, end }) => ({ - start: fromBabelLocation(start, sourceId), - end: fromBabelLocation(end, sourceId) - })), - refs: nameRefs.refs.map(({ start, end }) => ({ - start: fromBabelLocation(start, sourceId), - end: fromBabelLocation(end, sourceId) - })) - }; - break; - } + + _bindings[n] = { + type: nameRefs.type, + declarations: nameRefs.declarations.map(({ start, end }) => ({ + start: fromBabelLocation(start, sourceId), + end: fromBabelLocation(end, sourceId) + })), + refs: nameRefs.refs.map(({ start, end }) => ({ + start: fromBabelLocation(start, sourceId), + end: fromBabelLocation(end, sourceId) + })) + }; return _bindings; }, Object.create(null)); return { @@ -35391,7 +35421,7 @@ function toParsedScopes(children, sourceId) { end: fromBabelLocation(scope.loc.end, sourceId), type: scope.type === "module" ? "block" : scope.type, displayName: scope.displayName, - bindings, + bindings: bindings, children: toParsedScopes(scope.children, sourceId) }; }); @@ -35449,6 +35479,11 @@ function createParseJSScopeVisitor(sourceId) { parent = createTempScope("block", "Lexical Global", parent, location); parent = createTempScope("module", "Module", parent, location); + parent.names.this = { + type: "implicit", + declarations: [], + refs: [] + }; return; } if (path.isFunction()) { @@ -35470,18 +35505,53 @@ function createParseJSScopeVisitor(sourceId) { end: location.end }); if (path.isFunctionDeclaration() && isNode(tree.id, "Identifier")) { - const functionName = { - type: "fn", + // This ignores Annex B function declaration hoisting, which + // is probably a fine assumption. + const fnScope = getVarScope(parent); + scope.names[tree.id.name] = { + type: fnScope === scope ? "var" : "let", declarations: [tree.id.loc], refs: [] }; - getFunctionScope(parent).names[tree.id.name] = functionName; - scope.names[tree.id.name] = functionName; } - tree.params.forEach(param => parseDeclarator(param, scope, "param")); + tree.params.forEach(param => parseDeclarator(param, scope, "var")); + + if (!path.isArrowFunctionExpression()) { + scope.names.this = { + type: "implicit", + declarations: [], + refs: [] + }; + scope.names.arguments = { + type: "implicit", + declarations: [], + refs: [] + }; + } + parent = scope; return; } + if (path.isClass()) { + if (path.isClassDeclaration() && path.get("id").isIdentifier()) { + parent.names[tree.id.name] = { + type: "let", + declarations: [tree.id.loc], + refs: [] + }; + } + + if (path.get("id").isIdentifier()) { + savedParents.set(path, parent); + parent = createTempScope("block", "Class", parent, location); + + parent.names[tree.id.name] = { + type: "const", + declarations: [tree.id.loc], + refs: [] + }; + } + } if (path.isForXStatement() || path.isForStatement()) { const init = tree.init || tree.left; if (isNode(init, "VariableDeclaration") && isLetOrConst(init)) { @@ -35499,7 +35569,7 @@ function createParseJSScopeVisitor(sourceId) { if (path.isCatchClause()) { savedParents.set(path, parent); parent = createTempScope("block", "Catch", parent, location); - parseDeclarator(tree.param, parent, "param"); + parseDeclarator(tree.param, parent, "var"); return; } if (path.isBlockStatement()) { @@ -35510,9 +35580,11 @@ function createParseJSScopeVisitor(sourceId) { } return; } - if (path.isVariableDeclaration()) { + if (path.isVariableDeclaration() && (path.node.kind === "var" || + // Lexical declarations in for statements are handled above. + !path.parentPath.isForStatement({ init: tree }) || !path.parentPath.isXStatement({ left: tree }))) { // Finds right lexical environment - const hoistAt = !isLetOrConst(tree) ? getFunctionScope(parent) : parent; + const hoistAt = !isLetOrConst(tree) ? getVarScope(parent) : parent; tree.declarations.forEach(declarator => { parseDeclarator(declarator.id, hoistAt, tree.kind); }); @@ -35523,7 +35595,7 @@ function createParseJSScopeVisitor(sourceId) { path.get("specifiers").forEach(spec => { parent.names[spec.node.local.name] = { - type: "import", + type: "const", declarations: [spec.node.local.loc], refs: [] }; @@ -35542,10 +35614,26 @@ function createParseJSScopeVisitor(sourceId) { } return; } + if (path.isThisExpression()) { + const scope = findIdentifierInScopes(parent, "this"); + if (scope) { + scope.names.this.refs.push(tree.loc); + } + } if (path.parentPath.isClassProperty({ value: tree })) { savedParents.set(path, parent); parent = createTempScope("block", "Class Field", parent, location); + parent.names.this = { + type: "implicit", + declarations: [], + refs: [] + }; + parent.names.arguments = { + type: "implicit", + declarations: [], + refs: [] + }; return; } diff --git a/devtools/client/debugger/new/test/mochitest/browser_dbg-sourcemaps-reloading.js b/devtools/client/debugger/new/test/mochitest/browser_dbg-sourcemaps-reloading.js index 9edbe90c72c8..306913b75651 100644 --- a/devtools/client/debugger/new/test/mochitest/browser_dbg-sourcemaps-reloading.js +++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-sourcemaps-reloading.js @@ -2,7 +2,10 @@ * http://creativecommons.org/publicdomain/zero/1.0/ */ async function waitForBreakpointCount(dbg, count) { - return waitForState(dbg, state => dbg.selectors.getBreakpoints(state).size === count) + return waitForState( + dbg, + state => dbg.selectors.getBreakpoints(state).size === count + ); } add_task(async function() { diff --git a/devtools/client/debugger/new/test/mochitest/browser_dbg-sourcemaps.js b/devtools/client/debugger/new/test/mochitest/browser_dbg-sourcemaps.js index 195ff5c7ef3b..f14935f50cce 100644 --- a/devtools/client/debugger/new/test/mochitest/browser_dbg-sourcemaps.js +++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-sourcemaps.js @@ -80,12 +80,12 @@ add_task(async function() { await stepIn(dbg); assertPausedLocation(dbg); - await dbg.actions.jumpToMappedSelectedLocation() + await dbg.actions.jumpToMappedSelectedLocation(); await stepOver(dbg); assertPausedLocation(dbg); assertDebugLine(dbg, 71); - await dbg.actions.jumpToMappedSelectedLocation() + await dbg.actions.jumpToMappedSelectedLocation(); await stepOut(dbg); await stepOut(dbg); assertPausedLocation(dbg); diff --git a/devtools/client/debugger/new/test/mochitest/browser_dbg-sources.js b/devtools/client/debugger/new/test/mochitest/browser_dbg-sources.js index 0a9b35d61420..a368e43ba63e 100644 --- a/devtools/client/debugger/new/test/mochitest/browser_dbg-sources.js +++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-sources.js @@ -29,13 +29,13 @@ add_task(async function() { await waitForSources(dbg, "simple1", "simple2", "nested-source", "long.js"); // Expand nodes and make sure more sources appear. - assertSourceCount(dbg, 2); + await assertSourceCount(dbg, 2); await clickElement(dbg, "sourceArrow", 2); - assertSourceCount(dbg, 7); + await assertSourceCount(dbg, 7); await clickElement(dbg, "sourceArrow", 3); - assertSourceCount(dbg, 8); + await assertSourceCount(dbg, 8); // Select a source. ok( diff --git a/devtools/client/debugger/new/test/mochitest/head.js b/devtools/client/debugger/new/test/mochitest/head.js index 99c9eed69bea..72bf6ffdcc70 100644 --- a/devtools/client/debugger/new/test/mochitest/head.js +++ b/devtools/client/debugger/new/test/mochitest/head.js @@ -285,9 +285,15 @@ function assertNotPaused(dbg) { * @static */ function assertPausedLocation(dbg) { - const { selectors: { getSelectedSource, getVisibleSelectedFrame }, getState } = dbg; + const { + selectors: { getSelectedSource, getVisibleSelectedFrame }, + getState + } = dbg; - ok(isSelectedFrameSelected(dbg, getState()), "top frame's source is selected"); + ok( + isSelectedFrameSelected(dbg, getState()), + "top frame's source is selected" + ); // Check the pause location const frame = getVisibleSelectedFrame(getState()); @@ -447,7 +453,7 @@ async function waitForMappedScopes(dbg) { await waitForState( dbg, state => { - const scopes = dbg.selectors.getScopes(state); + const scopes = dbg.selectors.getSelectedScope(state); return scopes && scopes.sourceBindings; }, "mapped scopes" @@ -949,14 +955,14 @@ const selectors = { sourceMapLink: ".source-footer .mapped-source", sourcesFooter: ".sources-panel .source-footer", editorFooter: ".editor-pane .source-footer", - sourceNode: i => `.sources-list .tree-node:nth-child(${i})`, + sourceNode: i => `.sources-list .tree-node:nth-child(${i}) .node`, sourceNodes: ".sources-list .tree-node", sourceArrow: i => `.sources-list .tree-node:nth-child(${i}) .arrow`, resultItems: ".result-list .result-item", fileMatch: ".managed-tree .result", popup: ".popover", tooltip: ".tooltip", - outlineItem: i => `.outline-list__element:nth-child(${i})`, + outlineItem: i => `.outline-list__element:nth-child(${i}) .function-signature`, outlineItems: ".outline-list__element" }; @@ -1082,3 +1088,17 @@ function getCM(dbg) { const el = dbg.win.document.querySelector(".CodeMirror"); return el.CodeMirror; } + +// NOTE: still experimental, the screenshots might not be exactly correct +async function takeScreenshot(dbg) { + let canvas = dbg.win.document.createElementNS( + "http://www.w3.org/1999/xhtml", + "html:canvas" + ); + let context = canvas.getContext("2d"); + canvas.width = dbg.win.innerWidth; + canvas.height = dbg.win.innerHeight; + context.drawWindow(dbg.win, 0, 0, canvas.width, canvas.height, "white"); + await waitForTime(1000); + dump(`[SCREENSHOT] ${canvas.toDataURL()}\n`); +} diff --git a/devtools/client/locales/en-US/debugger.properties b/devtools/client/locales/en-US/debugger.properties index 484d269d46f6..7828636d7a08 100644 --- a/devtools/client/locales/en-US/debugger.properties +++ b/devtools/client/locales/en-US/debugger.properties @@ -164,26 +164,38 @@ blackboxCheckboxTooltip2=Toggle blackboxing # LOCALIZATION NOTE (sources.search.key2): Key shortcut to open the search for # searching all the source files the debugger has seen. +# Do not localize "CmdOrCtrl+P", or change the format of the string. These are +# key identifiers, not messages displayed to the user. sources.search.key2=CmdOrCtrl+P # LOCALIZATION NOTE (sources.search.alt.key): A second key shortcut to open the # search for searching all the source files the debugger has seen. +# Do not localize "CmdOrCtrl+O", or change the format of the string. These are +# key identifiers, not messages displayed to the user. sources.search.alt.key=CmdOrCtrl+O # LOCALIZATION NOTE (projectTextSearch.key): A key shortcut to open the # full project text search for searching all the files the debugger has seen. +# Do not localize "CmdOrCtrl+Shift+F", or change the format of the string. These are +# key identifiers, not messages displayed to the user. projectTextSearch.key=CmdOrCtrl+Shift+F # LOCALIZATION NOTE (functionSearch.key): A key shortcut to open the # modal for searching functions in a file. +# Do not localize "CmdOrCtrl+Shift+O", or change the format of the string. These are +# key identifiers, not messages displayed to the user. functionSearch.key=CmdOrCtrl+Shift+O # LOCALIZATION NOTE (toggleBreakpoint.key): A key shortcut to toggle # breakpoints. +# Do not localize "CmdOrCtrl+B", or change the format of the string. These are +# key identifiers, not messages displayed to the user. toggleBreakpoint.key=CmdOrCtrl+B # LOCALIZATION NOTE (toggleCondPanel.key): A key shortcut to toggle # the conditional breakpoint panel. +# Do not localize "CmdOrCtrl+Shift+B", or change the format of the string. These are +# key identifiers, not messages displayed to the user. toggleCondPanel.key=CmdOrCtrl+Shift+B # LOCALIZATION NOTE (stepOut.key): A key shortcut to @@ -216,6 +228,8 @@ sources.noSourcesAvailable=This page has no sources # LOCALIZATION NOTE (sourceSearch.search.key2): Key shortcut to open the search # for searching within a the currently opened files in the editor +# Do not localize "CmdOrCtrl+F", or change the format of the string. These are +# key identifiers, not messages displayed to the user. sourceSearch.search.key2=CmdOrCtrl+F # LOCALIZATION NOTE (sourceSearch.search.placeholder): placeholder text in @@ -224,10 +238,14 @@ sourceSearch.search.placeholder=Search in file… # LOCALIZATION NOTE (sourceSearch.search.again.key2): Key shortcut to highlight # the next occurrence of the last search triggered from a source search +# Do not localize "CmdOrCtrl+G", or change the format of the string. These are +# key identifiers, not messages displayed to the user. sourceSearch.search.again.key2=CmdOrCtrl+G # LOCALIZATION NOTE (sourceSearch.search.againPrev.key2): Key shortcut to highlight # the previous occurrence of the last search triggered from a source search +# Do not localize "CmdOrCtrl+Shift+G", or change the format of the string. These are +# key identifiers, not messages displayed to the user. sourceSearch.search.againPrev.key2=CmdOrCtrl+Shift+G # LOCALIZATION NOTE (sourceSearch.resultsSummary1): Shows a summary of @@ -550,16 +568,16 @@ watchExpressions.refreshButton=Refresh # LOCALIZATION NOTE (welcome.search): The center pane welcome panel's # search prompt. e.g. cmd+p to search for files. On windows, it's ctrl, on # a mac we use the unicode character. -welcome.search=%S to search for sources +welcome.search=%S To search for sources # LOCALIZATION NOTE (welcome.findInFiles): The center pane welcome panel's # search prompt. e.g. cmd+f to search for files. On windows, it's ctrl+shift+f, on # a mac we use the unicode character. -welcome.findInFiles=%S to find in files +welcome.findInFiles=%S To find in files # LOCALIZATION NOTE (welcome.searchFunction): Label displayed in the welcome # panel. %S is replaced by the keyboard shortcut to search for functions. -welcome.searchFunction=%S to search for functions in file +welcome.searchFunction=%S To search for functions in file # LOCALIZATION NOTE (sourceSearch.search): The center pane Source Search # prompt for searching for files. @@ -687,8 +705,10 @@ functionSearchSeparatorLabel=← # LOCALIZATION NOTE(gotoLineModal.placeholder): The placeholder # text displayed when the user searches for specific lines in a file +# Do not localize "CmdOrCtrl+;", or change the format of the string. These are +# key identifiers, not messages displayed to the user. gotoLineModal.placeholder=Go to line… -gotoLineModal.key=CmdOrCtrl+Shift+; +gotoLineModal.key=CmdOrCtrl+; gotoLineModal.title=Go to a line number in a file # LOCALIZATION NOTE(symbolSearch.search.functionsPlaceholder): The placeholder @@ -703,6 +723,8 @@ symbolSearch.search.variablesPlaceholder.title=Search for a variable in a file # LOCALIZATION NOTE(symbolSearch.search.key2): The Key Shortcut for # searching for a function or variable +# Do not localize "CmdOrCtrl+Shift+O", or change the format of the string. These are +# key identifiers, not messages displayed to the user. symbolSearch.search.key2=CmdOrCtrl+Shift+O # LOCALIZATION NOTE(symbolSearch.searchModifier.modifiersLabel): A label @@ -825,6 +847,10 @@ shortcuts.stepOut=Step Out # keyboard shortcut action for source file search shortcuts.fileSearch=Source File Search +# LOCALIZATION NOTE (shortcuts.gotoLine): text describing +# keyboard shortcut for jumping to a specific line +shortcuts.gotoLine=Go to line + # LOCALIZATION NOTE (shortcuts.searchAgain): text describing # keyboard shortcut action for searching again shortcuts.searchAgain=Search Again diff --git a/devtools/client/preferences/debugger.js b/devtools/client/preferences/debugger.js index e52f40205fa2..a0eeb2524874 100644 --- a/devtools/client/preferences/debugger.js +++ b/devtools/client/preferences/debugger.js @@ -45,7 +45,7 @@ pref("devtools.debugger.project-directory-root", ""); pref("devtools.debugger.features.wasm", true); pref("devtools.debugger.features.shortcuts", true); -pref("devtools.debugger.features.root", false); +pref("devtools.debugger.features.root", true); pref("devtools.debugger.features.column-breakpoints", false); pref("devtools.debugger.features.chrome-scopes", false); pref("devtools.debugger.features.map-scopes", false); From 843da70a1a04dd726e84f5069a7d56bdfed44d10 Mon Sep 17 00:00:00 2001 From: Jonathan Watt Date: Fri, 26 Jan 2018 10:39:11 +0000 Subject: [PATCH 19/20] Bug 1436438 part 1 - Remove the tests that test SVG path data DOM interfaces. r=longsonr MozReview-Commit-ID: 78yzAb6Khf1 --- dom/svg/crashtests/367357-1.xhtml | 20 --- dom/svg/crashtests/369249-1.svg | 20 --- dom/svg/crashtests/372046-1.svg | 17 --- dom/svg/crashtests/372046-2.svg | 17 --- dom/svg/crashtests/435209-1.svg | 11 -- dom/svg/crashtests/crashtests.list | 5 - dom/svg/test/mochitest.ini | 4 +- dom/svg/test/test_SVGPathSegList.xhtml | 128 ---------------- dom/svg/test/test_SVGxxxList.xhtml | 57 ------- dom/svg/test/test_SVGxxxListIndexing.xhtml | 7 - dom/svg/test/test_pathLength.html | 58 ------- dom/svg/test/test_pathSeg.xhtml | 142 ------------------ .../mochitest/general/test_interfaces.js | 40 ----- layout/reftests/svg/path-03.svg | 29 ---- layout/reftests/svg/reftest.list | 1 - layout/svg/crashtests/327709-1.svg | 17 --- layout/svg/crashtests/crashtests.list | 1 - .../web-platform/meta/svg/historical.html.ini | 3 - 18 files changed, 1 insertion(+), 576 deletions(-) delete mode 100644 dom/svg/crashtests/367357-1.xhtml delete mode 100644 dom/svg/crashtests/369249-1.svg delete mode 100644 dom/svg/crashtests/372046-1.svg delete mode 100644 dom/svg/crashtests/372046-2.svg delete mode 100644 dom/svg/crashtests/435209-1.svg delete mode 100644 dom/svg/test/test_SVGPathSegList.xhtml delete mode 100644 dom/svg/test/test_pathLength.html delete mode 100644 dom/svg/test/test_pathSeg.xhtml delete mode 100644 layout/reftests/svg/path-03.svg delete mode 100644 layout/svg/crashtests/327709-1.svg diff --git a/dom/svg/crashtests/367357-1.xhtml b/dom/svg/crashtests/367357-1.xhtml deleted file mode 100644 index 207e2daee32a..000000000000 --- a/dom/svg/crashtests/367357-1.xhtml +++ /dev/null @@ -1,20 +0,0 @@ - - - - - - - - - - - - - - - diff --git a/dom/svg/crashtests/369249-1.svg b/dom/svg/crashtests/369249-1.svg deleted file mode 100644 index 64d55869b1fe..000000000000 --- a/dom/svg/crashtests/369249-1.svg +++ /dev/null @@ -1,20 +0,0 @@ - - - - -function boom() -{ - try { - document.getElementById("path").pathSegList.getItem(-10000000); - } catch(e) { - } -} - - - - - - diff --git a/dom/svg/crashtests/372046-1.svg b/dom/svg/crashtests/372046-1.svg deleted file mode 100644 index e18660c24c58..000000000000 --- a/dom/svg/crashtests/372046-1.svg +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - - diff --git a/dom/svg/crashtests/372046-2.svg b/dom/svg/crashtests/372046-2.svg deleted file mode 100644 index e18660c24c58..000000000000 --- a/dom/svg/crashtests/372046-2.svg +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - - diff --git a/dom/svg/crashtests/435209-1.svg b/dom/svg/crashtests/435209-1.svg deleted file mode 100644 index d674d50ed637..000000000000 --- a/dom/svg/crashtests/435209-1.svg +++ /dev/null @@ -1,11 +0,0 @@ - - - - - \ No newline at end of file diff --git a/dom/svg/crashtests/crashtests.list b/dom/svg/crashtests/crashtests.list index 10f9654995be..667704058b21 100644 --- a/dom/svg/crashtests/crashtests.list +++ b/dom/svg/crashtests/crashtests.list @@ -4,14 +4,10 @@ load 336994-1.html load 344888-1.svg load 345445-1.svg load 360836-1.svg -load 367357-1.xhtml load 369051-1.svg -load 369249-1.svg load 369291-1.svg load 369291-2.svg load 369568-1.svg -load 372046-1.svg -load 372046-2.svg load 374882-1.svg load 380101-1.svg load 381777-1.svg @@ -41,7 +37,6 @@ load 414188-1.svg load 427325-1.svg load 428228-1.svg load 428841-1.svg -load 435209-1.svg load 436418-mpathRoot-1.svg load 448244-1.svg load 466576-1.xhtml diff --git a/dom/svg/test/mochitest.ini b/dom/svg/test/mochitest.ini index a5885bdbaf14..e257087af5db 100644 --- a/dom/svg/test/mochitest.ini +++ b/dom/svg/test/mochitest.ini @@ -55,8 +55,7 @@ skip-if = true # disabled-for-intermittent-failures--bug-701060 [test_object-delayed-intrinsic-size.html] [test_onerror.xhtml] [test_pathAnimInterpolation.xhtml] -[test_pathLength.html] -[test_pathSeg.xhtml] +skip-if = true # We need to polyfill the SVG DOM for path data [test_pointAtLength.xhtml] [test_pointer-events-1a.xhtml] [test_pointer-events-1b.xhtml] @@ -77,7 +76,6 @@ skip-if = android_version == '18' # bug 1147994 [test_SVGMatrix.xhtml] [test_SVG_namespace_ids.html] [test_SVGNumberList.xhtml] -[test_SVGPathSegList.xhtml] [test_SVGPointList.xhtml] [test_SVGStringList.xhtml] [test_SVGStyleElement.xhtml] diff --git a/dom/svg/test/test_SVGPathSegList.xhtml b/dom/svg/test/test_SVGPathSegList.xhtml deleted file mode 100644 index d6f060f33f99..000000000000 --- a/dom/svg/test/test_SVGPathSegList.xhtml +++ /dev/null @@ -1,128 +0,0 @@ - - - - Generic tests for SVG animated length lists - - - - - -Mozilla Bug 611138 -

- -
-
-
- - diff --git a/dom/svg/test/test_SVGxxxList.xhtml b/dom/svg/test/test_SVGxxxList.xhtml index 6c40c8ebdb54..a59595b4a749 100644 --- a/dom/svg/test/test_SVGxxxList.xhtml +++ b/dom/svg/test/test_SVGxxxList.xhtml @@ -222,61 +222,6 @@ var tests = [ is(itemA.y, itemB.y, message); } }, - { - // SVGPathSegList test: - target_element_id: 'path', - attr_name: 'd', - prop_name: null, // SVGAnimatedPathData is an inherited interface! - bv_name: 'pathSegList', - av_name: 'animatedPathSegList', - el_type: 'SVGPathElement', - prop_type: null, - list_type: 'SVGPathSegList', - item_type: 'SVGPathSeg', - attr_val_3a: 'M 10,10 L 50,50 L 90,10', - attr_val_3b: 'M 10,50 L 50,10 L 90,50', - attr_val_4 : 'M 10,10 L 50,50 L 90,10 M 200,100', - attr_val_5a: 'M 10,10 L 50,50 L 90,10 L 130,50 L 170,10', - attr_val_5b: 'M 50,10 L 50,10 L 90,50 L 130,10 L 170,50', - attr_val_5b_firstItem_x3_constructor: function(constructor) { - var expected = constructor(); - is(expected.pathSegTypeAsLetter, "M", - "test error -- expected constructor to generate a segment of type M"); - expected.x = 150; - expected.y = 30; - return expected; - }, - item_constructor: function() { - // XXX return different values each time - return document.getElementById('path').createSVGPathSegMovetoAbs(1, 1); - }, - item_is: function(itemA, itemB, message) { - ok(typeof(itemA.pathSegTypeAsLetter) != 'undefined' && - typeof(itemB.pathSegTypeAsLetter) != 'undefined', - 'expecting pathSegTypeAsLetter property'); - - // First: are we dealing with the same type of segment? - is(itemA.pathSegTypeAsLetter, itemB.pathSegTypeAsLetter, message); - if (itemA.pathSegTypeAsLetter != itemB.pathSegTypeAsLetter) - return; // The rest of this function is nonsense if types don't match. - - // Make sure property-counts match (so we can iterate across itemA's - // properties and not worry about itemB having extra properties that - // we might be skipping over). - is(keys(itemA).length, keys(itemB).length, - 'expecting same property-count when comparing path segs of same type.'); - - // Compare the properties, skipping the constant properties inherited - // from 'SVGPathSeg', and skipping the pathSegTypeAsLetter field since we - // already checked that above. - for (var prop in itemA) { - if (!SVGPathSeg.hasOwnProperty(prop) && - prop != 'pathSegTypeAsLetter') { - is(itemA[prop], itemB[prop], message); - } - } - } - }, { // SVGStringList test: target_element_id: 'g', @@ -1170,14 +1115,12 @@ function run_animation_timeline_tests() 'in the '+t.list_type+' for '+t.bv_path+' to 5.'); // TODO - if (t.list_type != 'SVGPathSegList') { ok(t.animVal.getItem(3) === t.old_animVal_items[3], 'When affected by SMIL animation, list items in the '+t.list_type+ ' for '+t.bv_path+' that are at indexes that existed prior to the '+ 'start of the animation should be the exact same objects as the '+ 'objects that were at those indexes prior to the start of the '+ 'animation.'); - } t.old_animVal_items = get_array_of_list_items(t.animVal); diff --git a/dom/svg/test/test_SVGxxxListIndexing.xhtml b/dom/svg/test/test_SVGxxxListIndexing.xhtml index 4ba2e737ae98..e0b87bacb1e0 100644 --- a/dom/svg/test/test_SVGxxxListIndexing.xhtml +++ b/dom/svg/test/test_SVGxxxListIndexing.xhtml @@ -48,13 +48,6 @@ var tests = [ subtests: [ { values: null, length: 3 }, { values: "10", length: 1 }, { values: "1 2 3 4 5", length: 5 } ] }, - { element: path, - attribute: "d", - listProperty: "pathSegList", - type: "SVGPathSegList", - subtests: [ { values: null, length: 2 }, - { values: "M50,50", length: 1 }, - { values: "M0,0 h10 v20 h30 v40", length: 5 } ] }, { element: poly, attribute: "points", listProperty: "animatedPoints", diff --git a/dom/svg/test/test_pathLength.html b/dom/svg/test/test_pathLength.html deleted file mode 100644 index 169e7dca54d8..000000000000 --- a/dom/svg/test/test_pathLength.html +++ /dev/null @@ -1,58 +0,0 @@ - - - - - - Test path length changes when manipulated - - - - -Mozilla Bug 1024926 -

- -
-
-
- - diff --git a/dom/svg/test/test_pathSeg.xhtml b/dom/svg/test/test_pathSeg.xhtml deleted file mode 100644 index d7fbc49fad35..000000000000 --- a/dom/svg/test/test_pathSeg.xhtml +++ /dev/null @@ -1,142 +0,0 @@ - - - - - Test for Bug 459953 - - - - -Mozilla Bug 459953 -

- -
-
-
- - diff --git a/dom/tests/mochitest/general/test_interfaces.js b/dom/tests/mochitest/general/test_interfaces.js index fd652782cfa5..bdd6f803b46f 100644 --- a/dom/tests/mochitest/general/test_interfaces.js +++ b/dom/tests/mochitest/general/test_interfaces.js @@ -1026,48 +1026,8 @@ var interfaceNamesInGlobalScope = {name: "SVGNumberList", insecureContext: true}, // IMPORTANT: Do not change this list without review from a DOM peer! {name: "SVGPathElement", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSeg", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegArcAbs", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegArcRel", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegClosePath", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegCurvetoCubicAbs", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegCurvetoCubicRel", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegCurvetoCubicSmoothAbs", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegCurvetoCubicSmoothRel", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegCurvetoQuadraticAbs", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegCurvetoQuadraticRel", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegCurvetoQuadraticSmoothAbs", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegCurvetoQuadraticSmoothRel", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegLinetoAbs", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegLinetoHorizontalAbs", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegLinetoHorizontalRel", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegLinetoRel", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegLinetoVerticalAbs", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegLinetoVerticalRel", insecureContext: true}, // IMPORTANT: Do not change this list without review from a DOM peer! {name: "SVGPathSegList", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegMovetoAbs", insecureContext: true}, -// IMPORTANT: Do not change this list without review from a DOM peer! - {name: "SVGPathSegMovetoRel", insecureContext: true}, // IMPORTANT: Do not change this list without review from a DOM peer! {name: "SVGPatternElement", insecureContext: true}, // IMPORTANT: Do not change this list without review from a DOM peer! diff --git a/layout/reftests/svg/path-03.svg b/layout/reftests/svg/path-03.svg deleted file mode 100644 index f3f8c57df088..000000000000 --- a/layout/reftests/svg/path-03.svg +++ /dev/null @@ -1,29 +0,0 @@ - - - - Testcase for invalid path - - - - - - - - - - - - - diff --git a/layout/reftests/svg/reftest.list b/layout/reftests/svg/reftest.list index f8939cedc72a..f212ad5a5e50 100644 --- a/layout/reftests/svg/reftest.list +++ b/layout/reftests/svg/reftest.list @@ -340,7 +340,6 @@ pref(svg.paint-order.enabled,true) == paint-order-03.svg paint-order-03-ref.svg #fuzzy(23,60) fails-if(d2d) == path-01.svg path-01-ref.svg == path-02.svg pass.svg -== path-03.svg pass.svg == path-04.svg pass.svg == path-05.svg pass.svg fuzzy-if(skiaContent,1,400) == path-06.svg path-06-ref.svg diff --git a/layout/svg/crashtests/327709-1.svg b/layout/svg/crashtests/327709-1.svg deleted file mode 100644 index ce0711427275..000000000000 --- a/layout/svg/crashtests/327709-1.svg +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - - - - - diff --git a/layout/svg/crashtests/crashtests.list b/layout/svg/crashtests/crashtests.list index 892505482e34..dd059b263241 100644 --- a/layout/svg/crashtests/crashtests.list +++ b/layout/svg/crashtests/crashtests.list @@ -15,7 +15,6 @@ load 325427-1.svg load 326495-1.svg load 326974-1.svg load 327706-1.svg -load 327709-1.svg load 327711-1.svg load 328137-1.svg load 329848-1.svg diff --git a/testing/web-platform/meta/svg/historical.html.ini b/testing/web-platform/meta/svg/historical.html.ini index d8718814e53a..6d66f5575efc 100644 --- a/testing/web-platform/meta/svg/historical.html.ini +++ b/testing/web-platform/meta/svg/historical.html.ini @@ -1,7 +1,4 @@ [historical.html] - [SVGPathSeg interface must be removed] - expected: FAIL - [SVGUnitTypes mixin interface must not be exposed] expected: FAIL From 4f06ebe51b966133db471fe1d4acf5641a90ee3e Mon Sep 17 00:00:00 2001 From: Jonathan Watt Date: Fri, 26 Jan 2018 21:16:49 +0000 Subject: [PATCH 20/20] Bug 1436438 part 2 - Remove the WebIDL methods for creating and mutating SVG path data. r=longsonr,baku MozReview-Commit-ID: Ey7fROPCaSS --- dom/webidl/SVGPathElement.webidl | 38 -------------------------------- dom/webidl/SVGPathSeg.webidl | 20 +++++++++++++++++ dom/webidl/SVGPathSegList.webidl | 12 ---------- 3 files changed, 20 insertions(+), 50 deletions(-) diff --git a/dom/webidl/SVGPathElement.webidl b/dom/webidl/SVGPathElement.webidl index 9732d3cfe8ae..a811a8ff20d0 100644 --- a/dom/webidl/SVGPathElement.webidl +++ b/dom/webidl/SVGPathElement.webidl @@ -12,44 +12,6 @@ interface SVGPathElement : SVGGeometryElement { unsigned long getPathSegAtLength(float distance); - [NewObject] - SVGPathSegClosePath createSVGPathSegClosePath(); - [NewObject] - SVGPathSegMovetoAbs createSVGPathSegMovetoAbs(float x, float y); - [NewObject] - SVGPathSegMovetoRel createSVGPathSegMovetoRel(float x, float y); - [NewObject] - SVGPathSegLinetoAbs createSVGPathSegLinetoAbs(float x, float y); - [NewObject] - SVGPathSegLinetoRel createSVGPathSegLinetoRel(float x, float y); - [NewObject] - SVGPathSegCurvetoCubicAbs createSVGPathSegCurvetoCubicAbs(float x, float y, float x1, float y1, float x2, float y2); - [NewObject] - SVGPathSegCurvetoCubicRel createSVGPathSegCurvetoCubicRel(float x, float y, float x1, float y1, float x2, float y2); - [NewObject] - SVGPathSegCurvetoQuadraticAbs createSVGPathSegCurvetoQuadraticAbs(float x, float y, float x1, float y1); - [NewObject] - SVGPathSegCurvetoQuadraticRel createSVGPathSegCurvetoQuadraticRel(float x, float y, float x1, float y1); - [NewObject] - SVGPathSegArcAbs createSVGPathSegArcAbs(float x, float y, float r1, float r2, float angle, boolean largeArcFlag, boolean sweepFlag); - [NewObject] - SVGPathSegArcRel createSVGPathSegArcRel(float x, float y, float r1, float r2, float angle, boolean largeArcFlag, boolean sweepFlag); - [NewObject] - SVGPathSegLinetoHorizontalAbs createSVGPathSegLinetoHorizontalAbs(float x); - [NewObject] - SVGPathSegLinetoHorizontalRel createSVGPathSegLinetoHorizontalRel(float x); - [NewObject] - SVGPathSegLinetoVerticalAbs createSVGPathSegLinetoVerticalAbs(float y); - [NewObject] - SVGPathSegLinetoVerticalRel createSVGPathSegLinetoVerticalRel(float y); - [NewObject] - SVGPathSegCurvetoCubicSmoothAbs createSVGPathSegCurvetoCubicSmoothAbs(float x, float y, float x2, float y2); - [NewObject] - SVGPathSegCurvetoCubicSmoothRel createSVGPathSegCurvetoCubicSmoothRel(float x, float y, float x2, float y2); - [NewObject] - SVGPathSegCurvetoQuadraticSmoothAbs createSVGPathSegCurvetoQuadraticSmoothAbs(float x, float y); - [NewObject] - SVGPathSegCurvetoQuadraticSmoothRel createSVGPathSegCurvetoQuadraticSmoothRel(float x, float y); }; SVGPathElement implements SVGAnimatedPathData; diff --git a/dom/webidl/SVGPathSeg.webidl b/dom/webidl/SVGPathSeg.webidl index b5ff1abd6099..5895a8c963f6 100644 --- a/dom/webidl/SVGPathSeg.webidl +++ b/dom/webidl/SVGPathSeg.webidl @@ -10,6 +10,7 @@ * liability, trademark and document use rules apply. */ +[NoInterfaceObject] interface SVGPathSeg { // Path Segment Types @@ -40,9 +41,11 @@ interface SVGPathSeg { readonly attribute DOMString pathSegTypeAsLetter; }; +[NoInterfaceObject] interface SVGPathSegClosePath : SVGPathSeg { }; +[NoInterfaceObject] interface SVGPathSegMovetoAbs : SVGPathSeg { [SetterThrows] attribute float x; @@ -50,6 +53,7 @@ interface SVGPathSegMovetoAbs : SVGPathSeg { attribute float y; }; +[NoInterfaceObject] interface SVGPathSegMovetoRel : SVGPathSeg { [SetterThrows] attribute float x; @@ -57,6 +61,7 @@ interface SVGPathSegMovetoRel : SVGPathSeg { attribute float y; }; +[NoInterfaceObject] interface SVGPathSegLinetoAbs : SVGPathSeg { [SetterThrows] attribute float x; @@ -64,6 +69,7 @@ interface SVGPathSegLinetoAbs : SVGPathSeg { attribute float y; }; +[NoInterfaceObject] interface SVGPathSegLinetoRel : SVGPathSeg { [SetterThrows] attribute float x; @@ -71,6 +77,7 @@ interface SVGPathSegLinetoRel : SVGPathSeg { attribute float y; }; +[NoInterfaceObject] interface SVGPathSegCurvetoCubicAbs : SVGPathSeg { [SetterThrows] attribute float x; @@ -86,6 +93,7 @@ interface SVGPathSegCurvetoCubicAbs : SVGPathSeg { attribute float y2; }; +[NoInterfaceObject] interface SVGPathSegCurvetoCubicRel : SVGPathSeg { [SetterThrows] attribute float x; @@ -101,6 +109,7 @@ interface SVGPathSegCurvetoCubicRel : SVGPathSeg { attribute float y2; }; +[NoInterfaceObject] interface SVGPathSegCurvetoQuadraticAbs : SVGPathSeg { [SetterThrows] attribute float x; @@ -112,6 +121,7 @@ interface SVGPathSegCurvetoQuadraticAbs : SVGPathSeg { attribute float y1; }; +[NoInterfaceObject] interface SVGPathSegCurvetoQuadraticRel : SVGPathSeg { [SetterThrows] attribute float x; @@ -123,6 +133,7 @@ interface SVGPathSegCurvetoQuadraticRel : SVGPathSeg { attribute float y1; }; +[NoInterfaceObject] interface SVGPathSegArcAbs : SVGPathSeg { [SetterThrows] attribute float x; @@ -140,6 +151,7 @@ interface SVGPathSegArcAbs : SVGPathSeg { attribute boolean sweepFlag; }; +[NoInterfaceObject] interface SVGPathSegArcRel : SVGPathSeg { [SetterThrows] attribute float x; @@ -157,26 +169,31 @@ interface SVGPathSegArcRel : SVGPathSeg { attribute boolean sweepFlag; }; +[NoInterfaceObject] interface SVGPathSegLinetoHorizontalAbs : SVGPathSeg { [SetterThrows] attribute float x; }; +[NoInterfaceObject] interface SVGPathSegLinetoHorizontalRel : SVGPathSeg { [SetterThrows] attribute float x; }; +[NoInterfaceObject] interface SVGPathSegLinetoVerticalAbs : SVGPathSeg { [SetterThrows] attribute float y; }; +[NoInterfaceObject] interface SVGPathSegLinetoVerticalRel : SVGPathSeg { [SetterThrows] attribute float y; }; +[NoInterfaceObject] interface SVGPathSegCurvetoCubicSmoothAbs : SVGPathSeg { [SetterThrows] attribute float x; @@ -188,6 +205,7 @@ interface SVGPathSegCurvetoCubicSmoothAbs : SVGPathSeg { attribute float y2; }; +[NoInterfaceObject] interface SVGPathSegCurvetoCubicSmoothRel : SVGPathSeg { [SetterThrows] attribute float x; @@ -199,6 +217,7 @@ interface SVGPathSegCurvetoCubicSmoothRel : SVGPathSeg { attribute float y2; }; +[NoInterfaceObject] interface SVGPathSegCurvetoQuadraticSmoothAbs : SVGPathSeg { [SetterThrows] attribute float x; @@ -206,6 +225,7 @@ interface SVGPathSegCurvetoQuadraticSmoothAbs : SVGPathSeg { attribute float y; }; +[NoInterfaceObject] interface SVGPathSegCurvetoQuadraticSmoothRel : SVGPathSeg { [SetterThrows] attribute float x; diff --git a/dom/webidl/SVGPathSegList.webidl b/dom/webidl/SVGPathSegList.webidl index 49d22e5af2e0..dc8c9a436fa1 100644 --- a/dom/webidl/SVGPathSegList.webidl +++ b/dom/webidl/SVGPathSegList.webidl @@ -13,19 +13,7 @@ interface SVGPathSegList { readonly attribute unsigned long numberOfItems; [Throws] - void clear(); - [Throws] - SVGPathSeg initialize(SVGPathSeg newItem); - [Throws] getter SVGPathSeg getItem(unsigned long index); - [Throws] - SVGPathSeg insertItemBefore(SVGPathSeg newItem, unsigned long index); - [Throws] - SVGPathSeg replaceItem(SVGPathSeg newItem, unsigned long index); - [Throws] - SVGPathSeg removeItem(unsigned long index); - [Throws] - SVGPathSeg appendItem(SVGPathSeg newItem); // Mozilla-specific stuff readonly attribute unsigned long length; // synonym for numberOfItems