From ee984e1e490bd58e517b4eb99076c09c7f87319a Mon Sep 17 00:00:00 2001 From: Dylan Roeh Date: Tue, 21 Aug 2018 12:52:39 -0500 Subject: [PATCH 01/45] Bug 1478171 - [1.0] Forward channel redirect to nsILoadURIDelegate to allow external handling. r=smaug,snorp --- netwerk/base/nsIChannelEventSink.idl | 7 ++ uriloader/base/nsDocLoader.cpp | 99 ++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/netwerk/base/nsIChannelEventSink.idl b/netwerk/base/nsIChannelEventSink.idl index 64c5b9eec316..3f6edc04da92 100644 --- a/netwerk/base/nsIChannelEventSink.idl +++ b/netwerk/base/nsIChannelEventSink.idl @@ -52,6 +52,13 @@ interface nsIChannelEventSink : nsISupports */ const unsigned long REDIRECT_STS_UPGRADE = 1 << 3; + /** + * This redirect has already been presented to the nsILoadURIDelegate + * for possible handling; if this flag is set we may safely skip checking + * if the nsILoadURIDelegate will handle the redirect. + */ + const unsigned long REDIRECT_DELEGATES_CHECKED = 1 << 4; + /** * Called when a redirect occurs. This may happen due to an HTTP 3xx status * code. The purpose of this method is to notify the sink that a redirect diff --git a/uriloader/base/nsDocLoader.cpp b/uriloader/base/nsDocLoader.cpp index 524681ad8e61..598fa6dbea8d 100644 --- a/uriloader/base/nsDocLoader.cpp +++ b/uriloader/base/nsDocLoader.cpp @@ -6,6 +6,8 @@ #include "nspr.h" #include "mozilla/Logging.h" #include "mozilla/IntegerPrintfMacros.h" +#include "mozilla/dom/Promise.h" +#include "mozilla/dom/PromiseNativeHandler.h" #include "nsDocLoader.h" #include "nsCURILoader.h" @@ -34,6 +36,8 @@ #include "nsIDocument.h" #include "nsPresContext.h" #include "nsIAsyncVerifyRedirectCallback.h" +#include "nsILoadURIDelegate.h" +#include "nsIBrowserDOMWindow.h" using mozilla::DebugOnly; using mozilla::LogLevel; @@ -1421,11 +1425,106 @@ int64_t nsDocLoader::CalculateMaxProgress() return max; } +class LoadURIDelegateRedirectHandler final : public mozilla::dom::PromiseNativeHandler +{ +public: + NS_DECL_CYCLE_COLLECTING_ISUPPORTS + NS_DECL_CYCLE_COLLECTION_CLASS(LoadURIDelegateRedirectHandler) + + LoadURIDelegateRedirectHandler(nsDocLoader* aDocLoader, + nsIChannel* aOldChannel, + nsIChannel* aNewChannel, + uint32_t aFlags, + nsIAsyncVerifyRedirectCallback* aCallback) + : mDocLoader(aDocLoader) + , mOldChannel(aOldChannel) + , mNewChannel(aNewChannel) + , mFlags(aFlags) + , mCallback(aCallback) + {} + + void + ResolvedCallback(JSContext* aCx, JS::Handle aValue) override + { + if (aValue.isBoolean() && aValue.toBoolean()) { + // The app handled the redirect, notify the callback + mCallback->OnRedirectVerifyCallback(NS_ERROR_ABORT); + } else { + UnhandledCallback(); + } + } + + void + RejectedCallback(JSContext* aCx, JS::Handle aValue) override + { + UnhandledCallback(); + } + +private: + ~LoadURIDelegateRedirectHandler() + {} + + void UnhandledCallback() + { + // If the redirect wasn't handled by the nsILoadURIDelegate, let Gecko + // handle it. + mFlags |= nsIChannelEventSink::REDIRECT_DELEGATES_CHECKED; + mDocLoader->AsyncOnChannelRedirect(mOldChannel, mNewChannel, mFlags, + mCallback); + } + + RefPtr mDocLoader; + nsCOMPtr mOldChannel; + nsCOMPtr mNewChannel; + uint32_t mFlags; + nsCOMPtr mCallback; +}; + +NS_IMPL_CYCLE_COLLECTION(LoadURIDelegateRedirectHandler, mDocLoader, + mOldChannel, mNewChannel, mCallback) + +NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(LoadURIDelegateRedirectHandler) + NS_INTERFACE_MAP_ENTRY(nsISupports) +NS_INTERFACE_MAP_END + +NS_IMPL_CYCLE_COLLECTING_ADDREF(LoadURIDelegateRedirectHandler) +NS_IMPL_CYCLE_COLLECTING_RELEASE(LoadURIDelegateRedirectHandler) + NS_IMETHODIMP nsDocLoader::AsyncOnChannelRedirect(nsIChannel *aOldChannel, nsIChannel *aNewChannel, uint32_t aFlags, nsIAsyncVerifyRedirectCallback *cb) { + if ((aFlags & + (nsIChannelEventSink::REDIRECT_TEMPORARY | + nsIChannelEventSink::REDIRECT_PERMANENT)) && + !(aFlags & nsIChannelEventSink::REDIRECT_DELEGATES_CHECKED)) { + nsCOMPtr docShell = + do_QueryInterface(static_cast(this)); + + nsCOMPtr delegate; + docShell->GetLoadURIDelegate(getter_AddRefs(delegate)); + + nsCOMPtr newURI; + aNewChannel->GetURI(getter_AddRefs(newURI)); + + if (newURI && delegate) { + RefPtr promise; + const int where = nsIBrowserDOMWindow::OPEN_CURRENTWINDOW; + nsresult rv = delegate->LoadURI(newURI, where, /* flags */ 0, + /* triggering principal */ nullptr, + getter_AddRefs(promise)); + if (NS_SUCCEEDED(rv) && promise) { + RefPtr handler = + new LoadURIDelegateRedirectHandler(this, aOldChannel, aNewChannel, + aFlags, cb); + + promise->AppendNativeHandler(handler); + return NS_OK; + } + } + } + if (aOldChannel) { nsLoadFlags loadFlags = 0; From 68079d3759fba13ebb0c33d3dbeb14d6262ac399 Mon Sep 17 00:00:00 2001 From: Cosmin Sabou Date: Wed, 22 Aug 2018 17:15:34 +0300 Subject: [PATCH 02/45] Bug 1455602 - Disable browser/components/sessionstore/test/browser_restore_reversed_z_order.js for frequent failures. r=jmaher --HG-- extra : rebase_source : 56ca0cbf76c9e4701e9b0448474dad141e58ec14 --- browser/components/sessionstore/test/browser.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/browser/components/sessionstore/test/browser.ini b/browser/components/sessionstore/test/browser.ini index 3de967c23e2b..62ab7afbd48c 100644 --- a/browser/components/sessionstore/test/browser.ini +++ b/browser/components/sessionstore/test/browser.ini @@ -286,3 +286,4 @@ skip-if = !crashreporter || !e10s # Tabs can't crash without e10s [browser_speculative_connect.js] [browser_1446343-windowsize.js] [browser_restore_reversed_z_order.js] +skip-if = true #Bug 1455602 From 37fb58dac28ead811669fb54f41c5814294b6096 Mon Sep 17 00:00:00 2001 From: Cosmin Sabou Date: Wed, 22 Aug 2018 17:36:57 +0300 Subject: [PATCH 03/45] Bug 1430530 - Disable gfx/tests/mochitest/test_acceleration.html on windows for frequent failures. r=jmaher --- gfx/tests/mochitest/mochitest.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gfx/tests/mochitest/mochitest.ini b/gfx/tests/mochitest/mochitest.ini index 2f344b35f9bc..335a164ce1b6 100644 --- a/gfx/tests/mochitest/mochitest.ini +++ b/gfx/tests/mochitest/mochitest.ini @@ -1,7 +1,7 @@ [DEFAULT] [test_acceleration.html] -skip-if = (os == 'win' && os_version == '10.0' && !debug) # Bug 1430530 +skip-if = (os == 'win') # Bug 1430530 subsuite = gpu [test_bug509244.html] [test_bug513439.html] From 5b4c63c8a126a12c8f8702814bb19757bc977c84 Mon Sep 17 00:00:00 2001 From: Lee Salzman Date: Wed, 22 Aug 2018 11:12:33 -0400 Subject: [PATCH 04/45] Bug 1485358 - fix purging of missing glyph WebRender users. r=me --- gfx/thebes/gfxFontMissingGlyphs.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/gfx/thebes/gfxFontMissingGlyphs.cpp b/gfx/thebes/gfxFontMissingGlyphs.cpp index d1a323b8bab7..7fb157afac74 100644 --- a/gfx/thebes/gfxFontMissingGlyphs.cpp +++ b/gfx/thebes/gfxFontMissingGlyphs.cpp @@ -157,7 +157,6 @@ public: void Remove() { - remove(); mManager->RemoveUserData(&sWRUserDataKey); } @@ -232,8 +231,11 @@ PurgeWRGlyphAtlas() } } } - // Remove the layer manager's destroy notification. - user->Remove(); + } + // Remove the layer managers' destroy notifications only after processing + // so as not to mess up gWRUsers iteration. + while (!gWRUsers.isEmpty()) { + gWRUsers.popFirst()->Remove(); } // Finally, clear out the atlases. for (size_t i = 0; i < 8; i++) { From f7be15f4f48ddb17ff26ba62169cbe404353a3be Mon Sep 17 00:00:00 2001 From: Jean-Yves Avenard Date: Fri, 13 Jul 2018 12:42:18 +0200 Subject: [PATCH 05/45] Bug 1245400 - P1. Make ImageComposite::mImages a private member. r=nical We will use the characteristic of which TimedImage is returned to keep track on how many frames were dropped because they were too old. As such, we must make sure the retrieval of the current image is serialised. This allows to reduce duplicated code between WebRenderImageHost and ImageHost classes. Additionally, make RenderInfo::img member const as really, we never want to modify that one. A future change will enforce that RenderInfo.img never survives longer than the ChooseImage()'s caller to clarify the lifetime of the TimedImage. Differential Revision: https://phabricator.services.mozilla.com/D2175 --- gfx/layers/composite/ImageComposite.cpp | 78 ++++++++++++++++++------- gfx/layers/composite/ImageComposite.h | 20 ++++--- gfx/layers/composite/ImageHost.cpp | 68 ++++++++++----------- gfx/layers/composite/ImageHost.h | 2 +- gfx/layers/wr/WebRenderImageHost.cpp | 57 ++++++++---------- 5 files changed, 122 insertions(+), 103 deletions(-) diff --git a/gfx/layers/composite/ImageComposite.cpp b/gfx/layers/composite/ImageComposite.cpp index 612e15a3c70a..084cc561c36d 100644 --- a/gfx/layers/composite/ImageComposite.cpp +++ b/gfx/layers/composite/ImageComposite.cpp @@ -24,10 +24,10 @@ ImageComposite::~ImageComposite() { } -/* static */ TimeStamp -ImageComposite::GetBiasedTime(const TimeStamp& aInput, ImageComposite::Bias aBias) +TimeStamp +ImageComposite::GetBiasedTime(const TimeStamp& aInput) const { - switch (aBias) { + switch (mBias) { case ImageComposite::BIAS_NEGATIVE: return aInput - TimeDuration::FromMilliseconds(BIAS_TIME_MS); case ImageComposite::BIAS_POSITIVE: @@ -37,18 +37,24 @@ ImageComposite::GetBiasedTime(const TimeStamp& aInput, ImageComposite::Bias aBia } } -/* static */ ImageComposite::Bias -ImageComposite::UpdateBias(const TimeStamp& aCompositionTime, - const TimeStamp& aCompositedImageTime, - const TimeStamp& aNextImageTime, // may be null - ImageComposite::Bias aBias) +void +ImageComposite::UpdateBias(size_t aImageIndex) { - if (aCompositedImageTime.IsNull()) { - return ImageComposite::BIAS_NONE; + MOZ_ASSERT(aImageIndex < ImagesCount()); + + TimeStamp compositionTime = GetCompositionTime(); + TimeStamp compositedImageTime = mImages[aImageIndex].mTimeStamp; + TimeStamp nextImageTime = aImageIndex + 1 < ImagesCount() + ? mImages[aImageIndex + 1].mTimeStamp + : TimeStamp(); + + if (compositedImageTime.IsNull()) { + mBias = ImageComposite::BIAS_NONE; + return; } TimeDuration threshold = TimeDuration::FromMilliseconds(1.0); - if (aCompositionTime - aCompositedImageTime < threshold && - aCompositionTime - aCompositedImageTime > -threshold) { + if (compositionTime - compositedImageTime < threshold && + compositionTime - compositedImageTime > -threshold) { // The chosen frame's time is very close to the composition time (probably // just before the current composition time, but due to previously set // negative bias, it could be just after the current composition time too). @@ -59,11 +65,12 @@ ImageComposite::UpdateBias(const TimeStamp& aCompositionTime, // Try to prevent that by adding a negative bias to the frame times during // the next composite; that should ensure the next frame's time is treated // as falling just before a composite time. - return ImageComposite::BIAS_NEGATIVE; + mBias = ImageComposite::BIAS_NEGATIVE; + return; } - if (!aNextImageTime.IsNull() && - aNextImageTime - aCompositionTime < threshold && - aNextImageTime - aCompositionTime > -threshold) { + if (!nextImageTime.IsNull() && + nextImageTime - compositionTime < threshold && + nextImageTime - compositionTime > -threshold) { // The next frame's time is very close to our composition time (probably // just after the current composition time, but due to previously set // positive bias, it could be just before the current composition time too). @@ -73,9 +80,10 @@ ImageComposite::UpdateBias(const TimeStamp& aCompositionTime, // Try to prevent that by adding a negative bias to the frame times during // the next composite; that should ensure the next frame's time is treated // as falling just before a composite time. - return ImageComposite::BIAS_POSITIVE; + mBias = ImageComposite::BIAS_POSITIVE; + return; } - return ImageComposite::BIAS_NONE; + mBias = ImageComposite::BIAS_NONE; } int @@ -100,7 +108,7 @@ ImageComposite::ChooseImageIndex() const uint32_t result = 0; while (result + 1 < mImages.Length() && - GetBiasedTime(mImages[result + 1].mTimeStamp, mBias) <= now) { + GetBiasedTime(mImages[result + 1].mTimeStamp) <= now) { ++result; } return result; @@ -112,10 +120,36 @@ const ImageComposite::TimedImage* ImageComposite::ChooseImage() const return index >= 0 ? &mImages[index] : nullptr; } -ImageComposite::TimedImage* ImageComposite::ChooseImage() +void +ImageComposite::RemoveImagesWithTextureHost(TextureHost* aTexture) { - int index = ChooseImageIndex(); - return index >= 0 ? &mImages[index] : nullptr; + for (int32_t i = mImages.Length() - 1; i >= 0; --i) { + if (mImages[i].mTextureHost == aTexture) { + aTexture->UnbindTextureSource(); + mImages.RemoveElementAt(i); + } + } +} + +void +ImageComposite::ClearImages() +{ + mImages.Clear(); +} + +void +ImageComposite::SetImages(nsTArray&& aNewImages) +{ + mImages = std::move(aNewImages); +} + +const ImageComposite::TimedImage* +ImageComposite::GetImage(size_t aIndex) const +{ + if (aIndex >= mImages.Length()) { + return nullptr; + } + return &mImages[aIndex]; } } // namespace layers diff --git a/gfx/layers/composite/ImageComposite.h b/gfx/layers/composite/ImageComposite.h index 362f9538f6a1..e590da2d202c 100644 --- a/gfx/layers/composite/ImageComposite.h +++ b/gfx/layers/composite/ImageComposite.h @@ -50,13 +50,8 @@ public: BIAS_POSITIVE, }; - static TimeStamp GetBiasedTime(const TimeStamp& aInput, ImageComposite::Bias aBias); - protected: - static Bias UpdateBias(const TimeStamp& aCompositionTime, - const TimeStamp& aCompositedImageTime, - const TimeStamp& aNextImageTime, // may be null - ImageComposite::Bias aBias); + void UpdateBias(size_t aImageIndex); virtual TimeStamp GetCompositionTime() const = 0; @@ -75,12 +70,21 @@ protected: * mBias is updated at the end of Composite(). */ const TimedImage* ChooseImage() const; - TimedImage* ChooseImage(); int ChooseImageIndex() const; + const TimedImage* GetImage(size_t aIndex) const; + size_t ImagesCount() const { return mImages.Length(); } + const nsTArray& Images() const { return mImages; } + + void RemoveImagesWithTextureHost(TextureHost* aTexture); + void ClearImages(); + void SetImages(nsTArray&& aNewImages); - nsTArray mImages; int32_t mLastFrameID; int32_t mLastProducerID; + +private: + nsTArray mImages; + TimeStamp GetBiasedTime(const TimeStamp& aInput) const; /** * Bias to apply to the next frame. */ diff --git a/gfx/layers/composite/ImageHost.cpp b/gfx/layers/composite/ImageHost.cpp index 85a9cbdacd09..6a96feb9c6cc 100644 --- a/gfx/layers/composite/ImageHost.cpp +++ b/gfx/layers/composite/ImageHost.cpp @@ -9,6 +9,7 @@ #include "LayersLogging.h" // for AppendToString #include "composite/CompositableHost.h" // for CompositableHost, etc #include "ipc/IPCMessageUtils.h" // for null_t +#include "mozilla/Move.h" #include "mozilla/layers/Compositor.h" // for Compositor #include "mozilla/layers/Effects.h" // for TexturedEffect, Effect, etc #include "mozilla/layers/LayerManagerComposite.h" // for TexturedEffect, Effect, etc @@ -66,13 +67,12 @@ ImageHost::UseTextureHost(const nsTArray& aTextures) img.mTextureHost->Updated(); } - mImages.SwapElements(newImages); - newImages.Clear(); + SetImages(std::move(newImages)); // If we only have one image we can upload it right away, otherwise we'll upload // on-demand during composition after we have picked the proper timestamp. - if (mImages.Length() == 1) { - SetCurrentTextureHost(mImages[0].mTextureHost); + if (ImagesCount() == 1) { + SetCurrentTextureHost(GetImage(0)->mTextureHost); } HostLayerManager* lm = GetLayerManager(); @@ -82,11 +82,11 @@ ImageHost::UseTextureHost(const nsTArray& aTextures) // means that any CompositeUntil() call we made in Composite() may no longer // guarantee that we'll composite until the next frame is ready. Fix that here. if (lm && mLastFrameID >= 0) { - for (size_t i = 0; i < mImages.Length(); ++i) { - bool frameComesAfter = mImages[i].mFrameID > mLastFrameID || - mImages[i].mProducerID != mLastProducerID; - if (frameComesAfter && !mImages[i].mTimeStamp.IsNull()) { - lm->CompositeUntil(mImages[i].mTimeStamp + + for (const auto& img : Images()) { + bool frameComesAfter = + img.mFrameID > mLastFrameID || img.mProducerID != mLastProducerID; + if (frameComesAfter && !img.mTimeStamp.IsNull()) { + lm->CompositeUntil(img.mTimeStamp + TimeDuration::FromMilliseconds(BIAS_TIME_MS)); break; } @@ -142,13 +142,7 @@ ImageHost::RemoveTextureHost(TextureHost* aTexture) MOZ_ASSERT(!mLocked); CompositableHost::RemoveTextureHost(aTexture); - - for (int32_t i = mImages.Length() - 1; i >= 0; --i) { - if (mImages[i].mTextureHost == aTexture) { - aTexture->UnbindTextureSource(); - mImages.RemoveElementAt(i); - } - } + RemoveImagesWithTextureHost(aTexture); } TimeStamp @@ -164,14 +158,15 @@ ImageHost::GetCompositionTime() const TextureHost* ImageHost::GetAsTextureHost(IntRect* aPictureRect) { - TimedImage* img = ChooseImage(); - if (img) { - SetCurrentTextureHost(img->mTextureHost); + const TimedImage* img = ChooseImage(); + if (!img) { + return nullptr; } - if (aPictureRect && img) { + SetCurrentTextureHost(img->mTextureHost); + if (aPictureRect) { *aPictureRect = img->mPictureRect; } - return img ? img->mTextureHost.get() : nullptr; + return img->mTextureHost; } void ImageHost::Attach(Layer* aLayer, @@ -179,7 +174,7 @@ void ImageHost::Attach(Layer* aLayer, AttachFlags aFlags) { CompositableHost::Attach(aLayer, aProvider, aFlags); - for (auto& img : mImages) { + for (const auto& img : Images()) { img.mTextureHost->SetTextureSourceProvider(aProvider); img.mTextureHost->Updated(); } @@ -201,7 +196,7 @@ ImageHost::Composite(Compositor* aCompositor, return; } - TimedImage* img = info.img; + const TimedImage* img = info.img; { AutoLockCompositableHost autoLock(this); @@ -319,11 +314,12 @@ ImageHost::PrepareToRender(TextureSourceProvider* aProvider, RenderInfo* aOutInf return false; } - if (uint32_t(imageIndex) + 1 < mImages.Length()) { - lm->CompositeUntil(mImages[imageIndex + 1].mTimeStamp + TimeDuration::FromMilliseconds(BIAS_TIME_MS)); + if (uint32_t(imageIndex) + 1 < ImagesCount()) { + lm->CompositeUntil(GetImage(imageIndex + 1)->mTimeStamp + + TimeDuration::FromMilliseconds(BIAS_TIME_MS)); } - TimedImage* img = &mImages[imageIndex]; + const TimedImage* img = GetImage(imageIndex); img->mTextureHost->SetTextureSourceProvider(aProvider); SetCurrentTextureHost(img->mTextureHost); @@ -347,7 +343,7 @@ void ImageHost::FinishRendering(const RenderInfo& aInfo) { HostLayerManager* lm = GetLayerManager(); - TimedImage* img = aInfo.img; + const TimedImage* img = aInfo.img; int imageIndex = aInfo.imageIndex; if (mLastFrameID != img->mFrameID || mLastProducerID != img->mProducerID) { @@ -369,18 +365,14 @@ ImageHost::FinishRendering(const RenderInfo& aInfo) // since callers of ChooseImage(Index) assume the same image will be chosen // during a given composition. This must happen after autoLock's // destructor! - mBias = UpdateBias( - lm->GetCompositionTime(), mImages[imageIndex].mTimeStamp, - uint32_t(imageIndex + 1) < mImages.Length() ? - mImages[imageIndex + 1].mTimeStamp : TimeStamp(), - mBias); + UpdateBias(imageIndex); } void ImageHost::SetTextureSourceProvider(TextureSourceProvider* aProvider) { if (mTextureSourceProvider != aProvider) { - for (auto& img : mImages) { + for (const auto& img : Images()) { img.mTextureHost->SetTextureSourceProvider(aProvider); } } @@ -395,7 +387,7 @@ ImageHost::PrintInfo(std::stringstream& aStream, const char* aPrefix) nsAutoCString pfx(aPrefix); pfx += " "; - for (auto& img : mImages) { + for (const auto& img : Images()) { aStream << "\n"; img.mTextureHost->PrintInfo(aStream, pfx.get()); AppendToString(aStream, img.mPictureRect, " [picture-rect=", "]"); @@ -407,7 +399,7 @@ ImageHost::Dump(std::stringstream& aStream, const char* aPrefix, bool aDumpHtml) { - for (auto& img : mImages) { + for (const auto& img : Images()) { aStream << aPrefix; aStream << (aDumpHtml ? "
  • TextureHost: " : "TextureHost: "); @@ -419,7 +411,7 @@ ImageHost::Dump(std::stringstream& aStream, already_AddRefed ImageHost::GetAsSurface() { - TimedImage* img = ChooseImage(); + const TimedImage* img = ChooseImage(); if (img) { return img->mTextureHost->GetAsSurface(); } @@ -430,7 +422,7 @@ bool ImageHost::Lock() { MOZ_ASSERT(!mLocked); - TimedImage* img = ChooseImage(); + const TimedImage* img = ChooseImage(); if (!img) { return false; } @@ -489,7 +481,7 @@ ImageHost::IsOpaque() already_AddRefed ImageHost::GenEffect(const gfx::SamplingFilter aSamplingFilter) { - TimedImage* img = ChooseImage(); + const TimedImage* img = ChooseImage(); if (!img) { return nullptr; } diff --git a/gfx/layers/composite/ImageHost.h b/gfx/layers/composite/ImageHost.h index 25d899b3b6d8..392f8df3c594 100644 --- a/gfx/layers/composite/ImageHost.h +++ b/gfx/layers/composite/ImageHost.h @@ -93,7 +93,7 @@ public: struct RenderInfo { int imageIndex; - TimedImage* img; + const TimedImage* img; RefPtr host; RenderInfo() : imageIndex(-1), img(nullptr) diff --git a/gfx/layers/wr/WebRenderImageHost.cpp b/gfx/layers/wr/WebRenderImageHost.cpp index 471a89cce77a..784112ba79d5 100644 --- a/gfx/layers/wr/WebRenderImageHost.cpp +++ b/gfx/layers/wr/WebRenderImageHost.cpp @@ -7,6 +7,7 @@ #include "WebRenderImageHost.h" #include "LayersLogging.h" +#include "mozilla/Move.h" #include "mozilla/layers/Compositor.h" // for Compositor #include "mozilla/layers/CompositorVsyncScheduler.h" // for CompositorVsyncScheduler #include "mozilla/layers/Effects.h" // for TexturedEffect, Effect, etc @@ -68,8 +69,7 @@ WebRenderImageHost::UseTextureHost(const nsTArray& aTextures) img.mTextureHost->Updated(); } - mImages.SwapElements(newImages); - newImages.Clear(); + SetImages(std::move(newImages)); if (mWrBridge && mWrBridge->CompositorScheduler() && GetAsyncRef()) { // Will check if we will generate frame. @@ -82,12 +82,12 @@ WebRenderImageHost::UseTextureHost(const nsTArray& aTextures) // guarantee that we'll composite until the next frame is ready. Fix that here. if (mWrBridge && mLastFrameID >= 0) { MOZ_ASSERT(mWrBridge->AsyncImageManager()); - for (size_t i = 0; i < mImages.Length(); ++i) { - bool frameComesAfter = mImages[i].mFrameID > mLastFrameID || - mImages[i].mProducerID != mLastProducerID; - if (frameComesAfter && !mImages[i].mTimeStamp.IsNull()) { - mWrBridge->AsyncImageManager()->CompositeUntil(mImages[i].mTimeStamp + - TimeDuration::FromMilliseconds(BIAS_TIME_MS)); + for (const auto& img : Images()) { + bool frameComesAfter = + img.mFrameID > mLastFrameID || img.mProducerID != mLastProducerID; + if (frameComesAfter && !img.mTimeStamp.IsNull()) { + mWrBridge->AsyncImageManager()->CompositeUntil( + img.mTimeStamp + TimeDuration::FromMilliseconds(BIAS_TIME_MS)); break; } } @@ -104,9 +104,7 @@ WebRenderImageHost::UseComponentAlphaTextures(TextureHost* aTextureOnBlack, void WebRenderImageHost::CleanupResources() { - nsTArray newImages; - mImages.SwapElements(newImages); - newImages.Clear(); + ClearImages(); SetCurrentTextureHost(nullptr); } @@ -114,13 +112,7 @@ void WebRenderImageHost::RemoveTextureHost(TextureHost* aTexture) { CompositableHost::RemoveTextureHost(aTexture); - - for (int32_t i = mImages.Length() - 1; i >= 0; --i) { - if (mImages[i].mTextureHost == aTexture) { - aTexture->UnbindTextureSource(); - mImages.RemoveElementAt(i); - } - } + RemoveImagesWithTextureHost(aTexture); } TimeStamp @@ -137,7 +129,7 @@ WebRenderImageHost::GetCompositionTime() const TextureHost* WebRenderImageHost::GetAsTextureHost(IntRect* aPictureRect) { - TimedImage* img = ChooseImage(); + const TimedImage* img = ChooseImage(); if (img) { return img->mTextureHost; } @@ -157,12 +149,14 @@ WebRenderImageHost::GetAsTextureHostForComposite() return nullptr; } - if (uint32_t(imageIndex) + 1 < mImages.Length()) { + if (uint32_t(imageIndex) + 1 < ImagesCount()) { MOZ_ASSERT(mWrBridge->AsyncImageManager()); - mWrBridge->AsyncImageManager()->CompositeUntil(mImages[imageIndex + 1].mTimeStamp + TimeDuration::FromMilliseconds(BIAS_TIME_MS)); + mWrBridge->AsyncImageManager()->CompositeUntil( + GetImage(imageIndex + 1)->mTimeStamp + + TimeDuration::FromMilliseconds(BIAS_TIME_MS)); } - TimedImage* img = &mImages[imageIndex]; + const TimedImage* img = GetImage(imageIndex); if (mLastFrameID != img->mFrameID || mLastProducerID != img->mProducerID) { if (mAsyncRef) { @@ -179,12 +173,7 @@ WebRenderImageHost::GetAsTextureHostForComposite() } SetCurrentTextureHost(img->mTextureHost); - mBias = UpdateBias( - mWrBridge->AsyncImageManager()->GetCompositionTime(), - mImages[imageIndex].mTimeStamp, - uint32_t(imageIndex + 1) < mImages.Length() ? - mImages[imageIndex + 1].mTimeStamp : TimeStamp(), - mBias); + UpdateBias(imageIndex); return mCurrentTextureHost; } @@ -223,7 +212,7 @@ void WebRenderImageHost::SetTextureSourceProvider(TextureSourceProvider* aProvider) { if (mTextureSourceProvider != aProvider) { - for (auto& img : mImages) { + for (const auto& img : Images()) { img.mTextureHost->SetTextureSourceProvider(aProvider); } } @@ -238,7 +227,7 @@ WebRenderImageHost::PrintInfo(std::stringstream& aStream, const char* aPrefix) nsAutoCString pfx(aPrefix); pfx += " "; - for (auto& img : mImages) { + for (const auto& img : Images()) { aStream << "\n"; img.mTextureHost->PrintInfo(aStream, pfx.get()); AppendToString(aStream, img.mPictureRect, " [picture-rect=", "]"); @@ -247,10 +236,10 @@ WebRenderImageHost::PrintInfo(std::stringstream& aStream, const char* aPrefix) void WebRenderImageHost::Dump(std::stringstream& aStream, - const char* aPrefix, - bool aDumpHtml) + const char* aPrefix, + bool aDumpHtml) { - for (auto& img : mImages) { + for (const auto& img : Images()) { aStream << aPrefix; aStream << (aDumpHtml ? "
    • TextureHost: " : "TextureHost: "); @@ -262,7 +251,7 @@ WebRenderImageHost::Dump(std::stringstream& aStream, already_AddRefed WebRenderImageHost::GetAsSurface() { - TimedImage* img = ChooseImage(); + const TimedImage* img = ChooseImage(); if (img) { return img->mTextureHost->GetAsSurface(); } From a0a2549c8213879ed13035de4c98810c9191e7a9 Mon Sep 17 00:00:00 2001 From: Jean-Yves Avenard Date: Mon, 16 Jul 2018 14:52:41 +0200 Subject: [PATCH 06/45] Bug 1245400 - P2. Keep track of frames that should have been painted but didn't. r=nical, r=mattwoodrow We can't rely on the FrameID continuity to determine if a frame has been dropped due to timing or not. The reason being that the VideoSink will not send to the compositor frames it knows as being late already (causing a discontinuity in the frames IDs), and count them as being dropped. If we were to look at discontinuity on the compositor we would account for those frames twice. FramesID will also increase non-linearly if a frame isn't painted because it's not visible (either out of the visible tree or in a hidden tab). What we can measure however, is when a frame should have been painted but didn't because it was too late by looking at the value returned by ImageComposite::ChooseImageIndex() or when a new set of images is being received by the ImageComposite. Any images found in the earlier array but never returned must have been dropped due to timing. Looking at the index continuity greatly simplify the logic as we no longer need to worry if a video is hidden or not, or be part of a layer that is itself hidden as neither SetImages will be called then, nor ChooseImage For now, we only account for those frames dropped, and do not report them yet. Differential Revision: https://phabricator.services.mozilla.com/D2176 --- gfx/layers/composite/CompositableHost.h | 5 +- gfx/layers/composite/ImageComposite.cpp | 80 +++++++++++++++++++++++-- gfx/layers/composite/ImageComposite.h | 12 +++- gfx/layers/composite/ImageHost.cpp | 2 +- gfx/layers/composite/ImageHost.h | 2 +- gfx/layers/wr/WebRenderImageHost.cpp | 2 +- gfx/layers/wr/WebRenderImageHost.h | 2 +- 7 files changed, 93 insertions(+), 12 deletions(-) diff --git a/gfx/layers/composite/CompositableHost.h b/gfx/layers/composite/CompositableHost.h index 5eefc5eb8f43..18c8c8f2c1e9 100644 --- a/gfx/layers/composite/CompositableHost.h +++ b/gfx/layers/composite/CompositableHost.h @@ -127,11 +127,12 @@ public: * *aPictureRect (if non-null, and the returned TextureHost is non-null) * is set to the picture rect. */ - virtual TextureHost* GetAsTextureHost(gfx::IntRect* aPictureRect = nullptr) { + virtual TextureHost* GetAsTextureHost(gfx::IntRect* aPictureRect = nullptr) + { return nullptr; } - virtual gfx::IntSize GetImageSize() const + virtual gfx::IntSize GetImageSize() { MOZ_ASSERT(false, "Should have been overridden"); return gfx::IntSize(); diff --git a/gfx/layers/composite/ImageComposite.cpp b/gfx/layers/composite/ImageComposite.cpp index 084cc561c36d..1a1cacdf8a56 100644 --- a/gfx/layers/composite/ImageComposite.cpp +++ b/gfx/layers/composite/ImageComposite.cpp @@ -18,7 +18,10 @@ ImageComposite::ImageComposite() : mLastFrameID(-1) , mLastProducerID(-1) , mBias(BIAS_NONE) -{} + , mDroppedFrames(0) + , mLastChosenImageIndex(0) +{ +} ImageComposite::~ImageComposite() { @@ -87,8 +90,12 @@ ImageComposite::UpdateBias(size_t aImageIndex) } int -ImageComposite::ChooseImageIndex() const +ImageComposite::ChooseImageIndex() { + // ChooseImageIndex is called for all images in the layer when it is visible. + // Change to this behaviour would break dropped frames counting calculation: + // We rely on this assumption to determine if during successive runs an + // image is returned that isn't the one following immediately the previous one if (mImages.IsEmpty()) { return -1; } @@ -106,15 +113,22 @@ ImageComposite::ChooseImageIndex() const return -1; } - uint32_t result = 0; + uint32_t result = mLastChosenImageIndex; while (result + 1 < mImages.Length() && GetBiasedTime(mImages[result + 1].mTimeStamp) <= now) { ++result; } + if (result - mLastChosenImageIndex > 1) { + // We're not returning the same image as the last call to ChooseImageIndex + // or the immediately next one. We can assume that the frames not returned + // have been dropped as they were too late to be displayed + mDroppedFrames += result - mLastChosenImageIndex - 1; + } + mLastChosenImageIndex = result; return result; } -const ImageComposite::TimedImage* ImageComposite::ChooseImage() const +const ImageComposite::TimedImage* ImageComposite::ChooseImage() { int index = ChooseImageIndex(); return index >= 0 ? &mImages[index] : nullptr; @@ -135,11 +149,69 @@ void ImageComposite::ClearImages() { mImages.Clear(); + mLastChosenImageIndex = 0; +} + +uint32_t +ImageComposite::ScanForLastFrameIndex(const nsTArray& aNewImages) +{ + if (mImages.IsEmpty()) { + return 0; + } + uint32_t i = mLastChosenImageIndex; + uint32_t newIndex = 0; + uint32_t dropped = 0; + // See if the new array of images have any images in common with the + // previous list that we haven't played yet. + uint32_t j = 0; + while (i < mImages.Length() && j < aNewImages.Length()) { + if (mImages[i].mProducerID != aNewImages[j].mProducerID) { + // This is new content, can stop. + newIndex = j; + break; + } + int32_t oldFrameID = mImages[i].mFrameID; + int32_t newFrameID = aNewImages[j].mFrameID; + if (oldFrameID > newFrameID) { + // This is an image we have already returned, we don't need to present + // it again and can start from this index next time. + newIndex = ++j; + continue; + } + if (oldFrameID < mLastFrameID) { + // we have already returned that frame previously, ignore. + i++; + continue; + } + if (oldFrameID < newFrameID) { + // This is a new image, all images prior the new one and not yet + // rendered can be considered as dropped. Those images have a FrameID + // inferior to the new image. + for (++i; i < mImages.Length() && mImages[i].mFrameID < newFrameID && + mImages[i].mProducerID == aNewImages[j].mProducerID; + i++) { + dropped++; + } + break; + } + i++; + j++; + } + if (dropped > 0) { + mDroppedFrames += dropped; + } + if (newIndex >= aNewImages.Length()) { + // Somehow none of those images should be rendered (can this happen?) + // We will always return the last one for now. + newIndex = aNewImages.Length() - 1; + } + return newIndex; } void ImageComposite::SetImages(nsTArray&& aNewImages) { + mLastChosenImageIndex = ScanForLastFrameIndex(aNewImages); mImages = std::move(aNewImages); } diff --git a/gfx/layers/composite/ImageComposite.h b/gfx/layers/composite/ImageComposite.h index e590da2d202c..bb9cee1eeb29 100644 --- a/gfx/layers/composite/ImageComposite.h +++ b/gfx/layers/composite/ImageComposite.h @@ -69,8 +69,8 @@ protected: * it depends only on mImages, mCompositor->GetCompositionTime(), and mBias. * mBias is updated at the end of Composite(). */ - const TimedImage* ChooseImage() const; - int ChooseImageIndex() const; + const TimedImage* ChooseImage(); + int ChooseImageIndex(); const TimedImage* GetImage(size_t aIndex) const; size_t ImagesCount() const { return mImages.Length(); } const nsTArray& Images() const { return mImages; } @@ -85,10 +85,18 @@ protected: private: nsTArray mImages; TimeStamp GetBiasedTime(const TimeStamp& aInput) const; + // Scan new images and look for common ones in the existing mImages array. + // Will determine if an image has been dropped through gaps between images and + // adjust mDroppedFrames accordingly. + // Return the index of what the last returned image would have been. + uint32_t ScanForLastFrameIndex(const nsTArray& aNewImages); + /** * Bias to apply to the next frame. */ Bias mBias; + uint32_t mDroppedFrames; + uint32_t mLastChosenImageIndex; }; } // namespace layers diff --git a/gfx/layers/composite/ImageHost.cpp b/gfx/layers/composite/ImageHost.cpp index 6a96feb9c6cc..c9b8b213e7e1 100644 --- a/gfx/layers/composite/ImageHost.cpp +++ b/gfx/layers/composite/ImageHost.cpp @@ -448,7 +448,7 @@ ImageHost::Unlock() } IntSize -ImageHost::GetImageSize() const +ImageHost::GetImageSize() { const TimedImage* img = ChooseImage(); if (img) { diff --git a/gfx/layers/composite/ImageHost.h b/gfx/layers/composite/ImageHost.h index 392f8df3c594..5359550a023c 100644 --- a/gfx/layers/composite/ImageHost.h +++ b/gfx/layers/composite/ImageHost.h @@ -69,7 +69,7 @@ public: virtual void SetTextureSourceProvider(TextureSourceProvider* aProvider) override; - gfx::IntSize GetImageSize() const override; + gfx::IntSize GetImageSize() override; virtual void PrintInfo(std::stringstream& aStream, const char* aPrefix) override; diff --git a/gfx/layers/wr/WebRenderImageHost.cpp b/gfx/layers/wr/WebRenderImageHost.cpp index 784112ba79d5..71317fb7d2d8 100644 --- a/gfx/layers/wr/WebRenderImageHost.cpp +++ b/gfx/layers/wr/WebRenderImageHost.cpp @@ -272,7 +272,7 @@ WebRenderImageHost::Unlock() } IntSize -WebRenderImageHost::GetImageSize() const +WebRenderImageHost::GetImageSize() { const TimedImage* img = ChooseImage(); if (img) { diff --git a/gfx/layers/wr/WebRenderImageHost.h b/gfx/layers/wr/WebRenderImageHost.h index d1126d3ed579..bd84a3ab3952 100644 --- a/gfx/layers/wr/WebRenderImageHost.h +++ b/gfx/layers/wr/WebRenderImageHost.h @@ -50,7 +50,7 @@ public: virtual void SetTextureSourceProvider(TextureSourceProvider* aProvider) override; - gfx::IntSize GetImageSize() const override; + gfx::IntSize GetImageSize() override; virtual void PrintInfo(std::stringstream& aStream, const char* aPrefix) override; From 70040ff272afee81545c9b7cce982580190da719 Mon Sep 17 00:00:00 2001 From: Jean-Yves Avenard Date: Mon, 16 Jul 2018 23:19:09 +0200 Subject: [PATCH 07/45] Bug 1245400 - P3. Report number of frames dropped by compositor back to VideoSink. r=nical We report the number of frames dropped by the compositor because they were too late through: ImageComposite -> ImageHost -> CompositableTransactionParent -> ImageBridgeParent -> IPDL -> ImageBridgeChild -> ImageContainerListener -> ImageContainer -> VideoSink Differential Revision: https://phabricator.services.mozilla.com/D2177 --- dom/media/mediasink/VideoSink.cpp | 8 ++++ dom/media/mediasink/VideoSink.h | 5 ++- gfx/layers/ImageContainer.cpp | 30 ++++++++----- gfx/layers/ImageContainer.h | 7 +-- gfx/layers/composite/CompositableHost.h | 2 + gfx/layers/composite/ImageComposite.h | 6 +++ gfx/layers/composite/ImageHost.h | 5 +++ .../ipc/CompositableTransactionParent.cpp | 45 +++++++++++-------- .../ipc/CompositableTransactionParent.h | 3 ++ gfx/layers/ipc/ImageBridgeChild.cpp | 32 +++++++++---- gfx/layers/ipc/ImageBridgeChild.h | 6 ++- gfx/layers/ipc/ImageBridgeParent.cpp | 11 ++++- gfx/layers/ipc/PImageBridge.ipdl | 3 ++ 13 files changed, 119 insertions(+), 44 deletions(-) diff --git a/dom/media/mediasink/VideoSink.cpp b/dom/media/mediasink/VideoSink.cpp index f77387f0f741..0f4cec6e005c 100644 --- a/dom/media/mediasink/VideoSink.cpp +++ b/dom/media/mediasink/VideoSink.cpp @@ -47,6 +47,7 @@ VideoSink::VideoSink(AbstractThread* aThread, , mContainer(aContainer) , mProducerID(ImageContainer::AllocateProducerID()) , mFrameStats(aFrameStats) + , mOldDroppedCount(0) , mHasVideo(false) , mUpdateScheduler(aThread) , mVideoQueueSendToCompositorSize(aVQueueSentToCompositerSize) @@ -464,6 +465,13 @@ VideoSink::RenderVideoFrames(int32_t aMaxFrames, if (images.Length() > 0) { mContainer->SetCurrentFrames(frames[0]->mDisplay, images); + uint32_t droppedCount = mContainer->GetDroppedImageCount(); + uint32_t dropped = droppedCount - mOldDroppedCount; + if (dropped > 0) { + mFrameStats.NotifyDecodedFrames({0, 0, dropped}); + mOldDroppedCount = droppedCount; + VSINK_LOG_V("%u video frame discarded by compositor", dropped); + } } } diff --git a/dom/media/mediasink/VideoSink.h b/dom/media/mediasink/VideoSink.h index 2bf74cc12b9b..bb20d2d56a28 100644 --- a/dom/media/mediasink/VideoSink.h +++ b/dom/media/mediasink/VideoSink.h @@ -108,7 +108,8 @@ private: MOZ_ASSERT(mOwnerThread->IsCurrentThreadIn()); } - MediaQueue& VideoQueue() const { + MediaQueue& VideoQueue() const + { return mVideoQueue; } @@ -131,6 +132,8 @@ private: // The presentation end time of the last video frame which has been displayed. TimeUnit mVideoFrameEndTime; + uint32_t mOldDroppedCount; + // Event listeners for VideoQueue MediaEventListener mPushListener; MediaEventListener mFinishListener; diff --git a/gfx/layers/ImageContainer.cpp b/gfx/layers/ImageContainer.cpp index 7a9ce3c28a0e..db79b20794ba 100644 --- a/gfx/layers/ImageContainer.cpp +++ b/gfx/layers/ImageContainer.cpp @@ -121,6 +121,15 @@ ImageContainerListener::NotifyComposite(const ImageCompositeNotification& aNotif } } +void +ImageContainerListener::NotifyDropped(uint32_t aDropped) +{ + MutexAutoLock lock(mLock); + if (mImageContainer) { + mImageContainer->NotifyDropped(aDropped); + } +} + void ImageContainerListener::ClearImageContainer() { @@ -262,7 +271,7 @@ ImageContainer::SetCurrentImageInternal(const nsTArray& aImages) mCurrentProducerID = aImages[0].mProducerID; } else if (!aImages[0].mTimeStamp.IsNull()) { // Check for expired frames - for (auto& img : mCurrentImages) { + for (const auto& img : mCurrentImages) { if (img.mProducerID != aImages[0].mProducerID || img.mTimeStamp.IsNull() || img.mTimeStamp >= aImages[0].mTimeStamp) { @@ -278,7 +287,6 @@ ImageContainer::SetCurrentImageInternal(const nsTArray& aImages) const uint32_t maxFrames = 100; if (mFrameIDsNotYetComposited.Length() > maxFrames) { uint32_t dropFrames = mFrameIDsNotYetComposited.Length() - maxFrames; - mDroppedImageCount += dropFrames; mFrameIDsNotYetComposited.RemoveElementsAt(0, dropFrames); } } @@ -303,7 +311,7 @@ ImageContainer::SetCurrentImageInternal(const nsTArray& aImages) img->mTimeStamp = aImages[i].mTimeStamp; img->mFrameID = aImages[i].mFrameID; img->mProducerID = aImages[i].mProducerID; - for (auto& oldImg : mCurrentImages) { + for (const auto& oldImg : mCurrentImages) { if (oldImg.mFrameID == img->mFrameID && oldImg.mProducerID == img->mProducerID) { img->mComposited = oldImg.mComposited; @@ -441,11 +449,7 @@ ImageContainer::NotifyComposite(const ImageCompositeNotification& aNotification) if (aNotification.producerID() == mCurrentProducerID) { uint32_t i; for (i = 0; i < mFrameIDsNotYetComposited.Length(); ++i) { - if (mFrameIDsNotYetComposited[i] <= aNotification.frameID()) { - if (mFrameIDsNotYetComposited[i] < aNotification.frameID()) { - ++mDroppedImageCount; - } - } else { + if (mFrameIDsNotYetComposited[i] > aNotification.frameID()) { break; } } @@ -458,11 +462,17 @@ ImageContainer::NotifyComposite(const ImageCompositeNotification& aNotification) } if (!aNotification.imageTimeStamp().IsNull()) { - mPaintDelay = aNotification.firstCompositeTimeStamp() - - aNotification.imageTimeStamp(); + mPaintDelay = + aNotification.firstCompositeTimeStamp() - aNotification.imageTimeStamp(); } } +void +ImageContainer::NotifyDropped(uint32_t aDropped) +{ + mDroppedImageCount += aDropped; +} + #ifdef XP_WIN D3D11YCbCrRecycleAllocator* ImageContainer::GetD3D11YCbCrRecycleAllocator(KnowsCompositor* aAllocator) diff --git a/gfx/layers/ImageContainer.h b/gfx/layers/ImageContainer.h index b5e5a7f83b82..c9d221269d92 100644 --- a/gfx/layers/ImageContainer.h +++ b/gfx/layers/ImageContainer.h @@ -356,6 +356,7 @@ public: explicit ImageContainerListener(ImageContainer* aImageContainer); void NotifyComposite(const ImageCompositeNotification& aNotification); + void NotifyDropped(uint32_t aDropped); void ClearImageContainer(); void DropImageClient(); private: @@ -613,11 +614,11 @@ public: */ uint32_t GetDroppedImageCount() { - RecursiveMutexAutoLock lock(mRecursiveMutex); return mDroppedImageCount; } void NotifyComposite(const ImageCompositeNotification& aNotification); + void NotifyDropped(uint32_t aDropped); ImageContainerListener* GetImageContainerListener() { @@ -675,8 +676,8 @@ private: // See GetPaintDelay. Accessed only with mRecursiveMutex held. TimeDuration mPaintDelay; - // See GetDroppedImageCount. Accessed only with mRecursiveMutex held. - uint32_t mDroppedImageCount; + // See GetDroppedImageCount. + mozilla::Atomic mDroppedImageCount; // This is the image factory used by this container, layer managers using // this container can set an alternative image factory that will be used to diff --git a/gfx/layers/composite/CompositableHost.h b/gfx/layers/composite/CompositableHost.h index 18c8c8f2c1e9..5705d0e31d7d 100644 --- a/gfx/layers/composite/CompositableHost.h +++ b/gfx/layers/composite/CompositableHost.h @@ -245,6 +245,8 @@ public: virtual void BindTextureSource() {} + virtual uint32_t GetDroppedFrames() { return 0; } + protected: HostLayerManager* GetLayerManager() const; diff --git a/gfx/layers/composite/ImageComposite.h b/gfx/layers/composite/ImageComposite.h index bb9cee1eeb29..8011ed530f0a 100644 --- a/gfx/layers/composite/ImageComposite.h +++ b/gfx/layers/composite/ImageComposite.h @@ -40,6 +40,12 @@ public: int32_t GetLastFrameID() const { return mLastFrameID; } int32_t GetLastProducerID() const { return mLastProducerID; } + uint32_t GetDroppedFramesAndReset() + { + uint32_t dropped = mDroppedFrames; + mDroppedFrames = 0; + return dropped; + } enum Bias { // Don't apply bias to frame times diff --git a/gfx/layers/composite/ImageHost.h b/gfx/layers/composite/ImageHost.h index 5359550a023c..75513cc3753e 100644 --- a/gfx/layers/composite/ImageHost.h +++ b/gfx/layers/composite/ImageHost.h @@ -91,6 +91,11 @@ public: bool IsOpaque(); + uint32_t GetDroppedFrames() override + { + return GetDroppedFramesAndReset(); + } + struct RenderInfo { int imageIndex; const TimedImage* img; diff --git a/gfx/layers/ipc/CompositableTransactionParent.cpp b/gfx/layers/ipc/CompositableTransactionParent.cpp index 3a5f893799ef..33db232699c4 100644 --- a/gfx/layers/ipc/CompositableTransactionParent.cpp +++ b/gfx/layers/ipc/CompositableTransactionParent.cpp @@ -68,18 +68,26 @@ CompositableParentManager::ReceiveCompositableUpdate(const CompositableOperation if (!compositable) { return false; } - if (TextureSourceProvider* provider = compositable->GetTextureSourceProvider()) { + return ReceiveCompositableUpdate(aEdit.detail(), WrapNotNull(compositable)); +} + +bool +CompositableParentManager::ReceiveCompositableUpdate( + const CompositableOperationDetail& aDetail, + NotNull aCompositable) +{ + if (TextureSourceProvider* provider = aCompositable->GetTextureSourceProvider()) { if (!provider->IsValid()) { return false; } } - switch (aEdit.detail().type()) { + switch (aDetail.type()) { case CompositableOperationDetail::TOpPaintTextureRegion: { MOZ_LAYERS_LOG(("[ParentSide] Paint PaintedLayer")); - const OpPaintTextureRegion& op = aEdit.detail().get_OpPaintTextureRegion(); - Layer* layer = compositable->GetLayer(); + const OpPaintTextureRegion& op = aDetail.get_OpPaintTextureRegion(); + Layer* layer = aCompositable->GetLayer(); if (!layer || layer->GetType() != Layer::TYPE_PAINTED) { return false; } @@ -89,10 +97,9 @@ CompositableParentManager::ReceiveCompositableUpdate(const CompositableOperation RenderTraceInvalidateStart(thebes, "FF00FF", op.updatedRegion().GetBounds()); - if (!compositable->UpdateThebes(bufferData, - op.updatedRegion(), - thebes->GetValidRegion())) - { + if (!aCompositable->UpdateThebes(bufferData, + op.updatedRegion(), + thebes->GetValidRegion())) { return false; } @@ -101,8 +108,8 @@ CompositableParentManager::ReceiveCompositableUpdate(const CompositableOperation } case CompositableOperationDetail::TOpUseTiledLayerBuffer: { MOZ_LAYERS_LOG(("[ParentSide] Paint TiledLayerBuffer")); - const OpUseTiledLayerBuffer& op = aEdit.detail().get_OpUseTiledLayerBuffer(); - TiledContentHost* tiledHost = compositable->AsTiledContentHost(); + const OpUseTiledLayerBuffer& op = aDetail.get_OpUseTiledLayerBuffer(); + TiledContentHost* tiledHost = aCompositable->AsTiledContentHost(); NS_ASSERTION(tiledHost, "The compositable is not tiled"); @@ -140,16 +147,16 @@ CompositableParentManager::ReceiveCompositableUpdate(const CompositableOperation break; } case CompositableOperationDetail::TOpRemoveTexture: { - const OpRemoveTexture& op = aEdit.detail().get_OpRemoveTexture(); + const OpRemoveTexture& op = aDetail.get_OpRemoveTexture(); RefPtr tex = TextureHost::AsTextureHost(op.textureParent()); MOZ_ASSERT(tex.get()); - compositable->RemoveTextureHost(tex); + aCompositable->RemoveTextureHost(tex); break; } case CompositableOperationDetail::TOpUseTexture: { - const OpUseTexture& op = aEdit.detail().get_OpUseTexture(); + const OpUseTexture& op = aDetail.get_OpUseTexture(); AutoTArray textures; for (auto& timedTexture : op.textures()) { @@ -166,7 +173,7 @@ CompositableParentManager::ReceiveCompositableUpdate(const CompositableOperation } } if (textures.Length() > 0) { - compositable->UseTextureHost(textures); + aCompositable->UseTextureHost(textures); for (auto& timedTexture : op.textures()) { RefPtr texture = TextureHost::AsTextureHost(timedTexture.textureParent()); @@ -179,13 +186,13 @@ CompositableParentManager::ReceiveCompositableUpdate(const CompositableOperation } } - if (UsesImageBridge() && compositable->GetLayer()) { - ScheduleComposition(compositable); + if (UsesImageBridge() && aCompositable->GetLayer()) { + ScheduleComposition(aCompositable); } break; } case CompositableOperationDetail::TOpUseComponentAlphaTextures: { - const OpUseComponentAlphaTextures& op = aEdit.detail().get_OpUseComponentAlphaTextures(); + const OpUseComponentAlphaTextures& op = aDetail.get_OpUseComponentAlphaTextures(); RefPtr texOnBlack = TextureHost::AsTextureHost(op.textureOnBlackParent()); RefPtr texOnWhite = TextureHost::AsTextureHost(op.textureOnWhiteParent()); if (op.readLockedBlack()) { @@ -196,7 +203,7 @@ CompositableParentManager::ReceiveCompositableUpdate(const CompositableOperation } MOZ_ASSERT(texOnBlack && texOnWhite); - compositable->UseComponentAlphaTextures(texOnBlack, texOnWhite); + aCompositable->UseComponentAlphaTextures(texOnBlack, texOnWhite); if (texOnBlack) { texOnBlack->SetLastFwdTransactionId(mFwdTransactionId); @@ -213,7 +220,7 @@ CompositableParentManager::ReceiveCompositableUpdate(const CompositableOperation } if (UsesImageBridge()) { - ScheduleComposition(compositable); + ScheduleComposition(aCompositable); } break; } diff --git a/gfx/layers/ipc/CompositableTransactionParent.h b/gfx/layers/ipc/CompositableTransactionParent.h index 3728d44d7185..b7c0ccd2d961 100644 --- a/gfx/layers/ipc/CompositableTransactionParent.h +++ b/gfx/layers/ipc/CompositableTransactionParent.h @@ -9,6 +9,7 @@ #include // for vector #include "mozilla/Attributes.h" // for override +#include "mozilla/NotNull.h" #include "mozilla/layers/ISurfaceAllocator.h" // for ISurfaceAllocator #include "mozilla/layers/LayersMessages.h" // for EditReply, etc #include "mozilla/layers/TextureClient.h" @@ -48,6 +49,8 @@ protected: * Handle the IPDL messages that affect PCompositable actors. */ bool ReceiveCompositableUpdate(const CompositableOperation& aEdit); + bool ReceiveCompositableUpdate(const CompositableOperationDetail& aDetail, + NotNull aCompositable); void ReleaseCompositable(const CompositableHandle& aHandle); diff --git a/gfx/layers/ipc/ImageBridgeChild.cpp b/gfx/layers/ipc/ImageBridgeChild.cpp index a24a0d88dcde..a433aed23899 100644 --- a/gfx/layers/ipc/ImageBridgeChild.cpp +++ b/gfx/layers/ipc/ImageBridgeChild.cpp @@ -997,18 +997,23 @@ ImageBridgeChild::RecvParentAsyncMessages(InfallibleTArray +ImageBridgeChild::FindListener(const CompositableHandle& aHandle) +{ + RefPtr listener; + MutexAutoLock lock(mContainerMapLock); + auto it = mImageContainerListeners.find(aHandle.Value()); + if (it != mImageContainerListeners.end()) { + listener = it->second; + } + return listener; +} + mozilla::ipc::IPCResult ImageBridgeChild::RecvDidComposite(InfallibleTArray&& aNotifications) { for (auto& n : aNotifications) { - RefPtr listener; - { - MutexAutoLock lock(mContainerMapLock); - auto it = mImageContainerListeners.find(n.compositable().Value()); - if (it != mImageContainerListeners.end()) { - listener = it->second; - } - } + RefPtr listener = FindListener(n.compositable()); if (listener) { listener->NotifyComposite(n); } @@ -1016,6 +1021,17 @@ ImageBridgeChild::RecvDidComposite(InfallibleTArray& return IPC_OK(); } +mozilla::ipc::IPCResult +ImageBridgeChild::RecvReportFramesDropped(const CompositableHandle& aHandle, const uint32_t& aFrames) +{ + RefPtr listener = FindListener(aHandle); + if (listener) { + listener->NotifyDropped(aFrames); + } + + return IPC_OK(); +} + PTextureChild* ImageBridgeChild::CreateTexture(const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, diff --git a/gfx/layers/ipc/ImageBridgeChild.h b/gfx/layers/ipc/ImageBridgeChild.h index 84a638ee4cb6..2898a90a2af9 100644 --- a/gfx/layers/ipc/ImageBridgeChild.h +++ b/gfx/layers/ipc/ImageBridgeChild.h @@ -95,7 +95,7 @@ bool InImageBridgeChildThread(); * - (B) Since the ImageContainer does not use ImageBridge, the image data is swaped. * * - During composition: - * - (A) The CompositableHost has an AsyncID, it looks up the ID in the + * - (A) The CompositableHost has an AsyncID, it looks up the ID in the * global table to see if there is an image. If there is no image, nothing is rendered. * - (B) The CompositableHost has image data rather than an ID (meaning it is not * using ImageBridge), then it just composites the image data normally. @@ -194,6 +194,9 @@ public: virtual mozilla::ipc::IPCResult RecvDidComposite(InfallibleTArray&& aNotifications) override; + virtual mozilla::ipc::IPCResult + RecvReportFramesDropped(const CompositableHandle& aHandle, const uint32_t& aFrames) override; + // Create an ImageClient from any thread. RefPtr CreateImageClient( CompositableType aType, @@ -403,6 +406,7 @@ private: */ Mutex mContainerMapLock; std::unordered_map> mImageContainerListeners; + RefPtr FindListener(const CompositableHandle& aHandle); #if defined(XP_WIN) /** diff --git a/gfx/layers/ipc/ImageBridgeParent.cpp b/gfx/layers/ipc/ImageBridgeParent.cpp index 1cd68ad143e6..d15d5537a6ed 100644 --- a/gfx/layers/ipc/ImageBridgeParent.cpp +++ b/gfx/layers/ipc/ImageBridgeParent.cpp @@ -198,10 +198,17 @@ ImageBridgeParent::RecvUpdate(EditArray&& aEdits, OpDestroyArray&& aToDestroy, AutoImageBridgeParentAsyncMessageSender autoAsyncMessageSender(this, &aToDestroy); UpdateFwdTransactionId(aFwdTransactionId); - for (EditArray::index_type i = 0; i < aEdits.Length(); ++i) { - if (!ReceiveCompositableUpdate(aEdits[i])) { + for (const auto& edit : aEdits) { + RefPtr compositable = + FindCompositable(edit.compositable()); + if (!compositable || + !ReceiveCompositableUpdate(edit.detail(), WrapNotNull(compositable))) { return IPC_FAIL_NO_REASON(this); } + uint32_t dropped = compositable->GetDroppedFrames(); + if (dropped) { + Unused << SendReportFramesDropped(edit.compositable(), dropped); + } } if (!IsSameProcess()) { diff --git a/gfx/layers/ipc/PImageBridge.ipdl b/gfx/layers/ipc/PImageBridge.ipdl index e074a7b07e88..704726c8f676 100644 --- a/gfx/layers/ipc/PImageBridge.ipdl +++ b/gfx/layers/ipc/PImageBridge.ipdl @@ -36,6 +36,9 @@ child: async DidComposite(ImageCompositeNotification[] aNotifications); + // Report the number of frames dropped for the given CompositableHost. + async ReportFramesDropped(CompositableHandle aHandle, uint32_t aFrames); + parent: async Update(CompositableOperation[] ops, OpDestroy[] toDestroy, uint64_t fwdTransactionId); From ea40a439aca8f24ef074f9cff2bac0350472531e Mon Sep 17 00:00:00 2001 From: Jean-Yves Avenard Date: Mon, 16 Jul 2018 23:25:12 +0200 Subject: [PATCH 08/45] Bug 1245400 - P4. Remove no longer used class member. r=nical Also speed up compositing videos as there's no longer need to check every single frames twice to determine if they were composited or not. Differential Revision: https://phabricator.services.mozilla.com/D2178 --- gfx/layers/ImageContainer.cpp | 28 ---------------------------- gfx/layers/ImageContainer.h | 4 +--- 2 files changed, 1 insertion(+), 31 deletions(-) diff --git a/gfx/layers/ImageContainer.cpp b/gfx/layers/ImageContainer.cpp index db79b20794ba..4382c24c1dfc 100644 --- a/gfx/layers/ImageContainer.cpp +++ b/gfx/layers/ImageContainer.cpp @@ -267,28 +267,7 @@ ImageContainer::SetCurrentImageInternal(const nsTArray& aImages) mCurrentImages[0].mFrameID <= aImages[0].mFrameID, "frame IDs shouldn't go backwards"); if (aImages[0].mProducerID != mCurrentProducerID) { - mFrameIDsNotYetComposited.Clear(); mCurrentProducerID = aImages[0].mProducerID; - } else if (!aImages[0].mTimeStamp.IsNull()) { - // Check for expired frames - for (const auto& img : mCurrentImages) { - if (img.mProducerID != aImages[0].mProducerID || - img.mTimeStamp.IsNull() || - img.mTimeStamp >= aImages[0].mTimeStamp) { - break; - } - if (!img.mComposited && !img.mTimeStamp.IsNull() && - img.mFrameID != aImages[0].mFrameID) { - mFrameIDsNotYetComposited.AppendElement(img.mFrameID); - } - } - - // Remove really old frames, assuming they'll never be composited. - const uint32_t maxFrames = 100; - if (mFrameIDsNotYetComposited.Length() > maxFrames) { - uint32_t dropFrames = mFrameIDsNotYetComposited.Length() - maxFrames; - mFrameIDsNotYetComposited.RemoveElementsAt(0, dropFrames); - } } } @@ -447,13 +426,6 @@ ImageContainer::NotifyComposite(const ImageCompositeNotification& aNotification) ++mPaintCount; if (aNotification.producerID() == mCurrentProducerID) { - uint32_t i; - for (i = 0; i < mFrameIDsNotYetComposited.Length(); ++i) { - if (mFrameIDsNotYetComposited[i] > aNotification.frameID()) { - break; - } - } - mFrameIDsNotYetComposited.RemoveElementsAt(0, i); for (auto& img : mCurrentImages) { if (img.mFrameID == aNotification.frameID()) { img.mComposited = true; diff --git a/gfx/layers/ImageContainer.h b/gfx/layers/ImageContainer.h index c9d221269d92..39b47a8b53b9 100644 --- a/gfx/layers/ImageContainer.h +++ b/gfx/layers/ImageContainer.h @@ -702,9 +702,7 @@ private: bool mIsAsync; CompositableHandle mAsyncContainerHandle; - nsTArray mFrameIDsNotYetComposited; - // ProducerID for last current image(s), including the frames in - // mFrameIDsNotYetComposited + // ProducerID for last current image(s) ProducerID mCurrentProducerID; RefPtr mNotifyCompositeListener; From efd5ec6e7af7a0ac6b7c7c2870d32700a067bb7e Mon Sep 17 00:00:00 2001 From: Jean-Yves Avenard Date: Tue, 17 Jul 2018 01:12:35 +0200 Subject: [PATCH 09/45] Bug 1245400 - P5. Report frames dropped with WebRender. r=nical Differential Revision: https://phabricator.services.mozilla.com/D2182 --- gfx/layers/wr/WebRenderImageHost.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/gfx/layers/wr/WebRenderImageHost.h b/gfx/layers/wr/WebRenderImageHost.h index bd84a3ab3952..a3bff558caf1 100644 --- a/gfx/layers/wr/WebRenderImageHost.h +++ b/gfx/layers/wr/WebRenderImageHost.h @@ -66,6 +66,11 @@ public: virtual void CleanupResources() override; + uint32_t GetDroppedFrames() override + { + return GetDroppedFramesAndReset(); + } + virtual WebRenderImageHost* AsWebRenderImageHost() override { return this; } TextureHost* GetAsTextureHostForComposite(); From 526114e5e923c91b45feda35ca8746ab6bd918af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A3o=20Gottwald?= Date: Fri, 17 Aug 2018 14:13:00 +0200 Subject: [PATCH 10/45] Bug 1446168 - Load tabbox.css as a document stylesheet. r=bgrins --- browser/base/content/tabbrowser.xml | 3 ++- browser/themes/shared/tabs.inc.css | 4 ++-- toolkit/content/widgets.css | 1 + toolkit/content/widgets/tabbox.xml | 17 ----------------- 4 files changed, 5 insertions(+), 20 deletions(-) diff --git a/browser/base/content/tabbrowser.xml b/browser/base/content/tabbrowser.xml index 44d4a17b68f0..c9bd33a6bfce 100644 --- a/browser/base/content/tabbrowser.xml +++ b/browser/base/content/tabbrowser.xml @@ -553,7 +553,8 @@ for (let i = numPinned - 1; i >= 0; i--) { let tab = this.children[i]; width += layoutData.pinnedTabWidth; - tab.style.marginInlineStart = -(width + layoutData.scrollButtonWidth) + "px"; + tab.style.setProperty("margin-inline-start", + -(width + layoutData.scrollButtonWidth) + "px", "important"); tab._pinnedUnscrollable = true; } this.style.paddingInlineStart = width + "px"; diff --git a/browser/themes/shared/tabs.inc.css b/browser/themes/shared/tabs.inc.css index eb2dd0f4d3b2..5464ce44ceb7 100644 --- a/browser/themes/shared/tabs.inc.css +++ b/browser/themes/shared/tabs.inc.css @@ -70,8 +70,8 @@ background-color: transparent; border-radius: 0; border-width: 0; - margin: 0; - padding: 0; + margin: 0 !important /* override tabbox.css */; + padding: 0 !important /* override tabbox.css */; -moz-box-align: stretch; } diff --git a/toolkit/content/widgets.css b/toolkit/content/widgets.css index 6b664cb4447f..d217013c7cf6 100644 --- a/toolkit/content/widgets.css +++ b/toolkit/content/widgets.css @@ -18,5 +18,6 @@ @import url("chrome://global/skin/progressmeter.css"); @import url("chrome://global/skin/richlistbox.css"); @import url("chrome://global/skin/splitter.css"); +@import url("chrome://global/skin/tabbox.css"); @import url("chrome://global/skin/toolbar.css"); @import url("chrome://global/skin/wizard.css"); diff --git a/toolkit/content/widgets/tabbox.xml b/toolkit/content/widgets/tabbox.xml index 3fe02a14624c..c8e86f619c55 100644 --- a/toolkit/content/widgets/tabbox.xml +++ b/toolkit/content/widgets/tabbox.xml @@ -8,12 +8,7 @@ xmlns="http://www.mozilla.org/xbl" xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" xmlns:xbl="http://www.mozilla.org/xbl"> - - - - - @@ -217,10 +212,6 @@ - - - - @@ -533,10 +524,6 @@ - - - - @@ -649,10 +636,6 @@ - - - - Date: Mon, 20 Aug 2018 06:25:00 +0300 Subject: [PATCH 11/45] Bug 1480278 - Disable border-image-017.xht on Linux. r=jmaher --- .../tests/css/css-backgrounds/border-image-017.xht.ini | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 testing/web-platform/tests/css/css-backgrounds/border-image-017.xht.ini diff --git a/testing/web-platform/tests/css/css-backgrounds/border-image-017.xht.ini b/testing/web-platform/tests/css/css-backgrounds/border-image-017.xht.ini new file mode 100644 index 000000000000..719701537ff6 --- /dev/null +++ b/testing/web-platform/tests/css/css-backgrounds/border-image-017.xht.ini @@ -0,0 +1,3 @@ +[border-image-017] + disabled: + if (os == "linux"): https://bugzilla.mozilla.org/show_bug.cgi?id=1480278 From 3a00b39e7b93fd97dc1fa885313e9bf520891645 Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Wed, 8 Aug 2018 17:30:40 +0200 Subject: [PATCH 12/45] Bug 1481851 - Add kind to RTCRtpStreamStats as alias to mediaType. r=ng,smaug spec change in https://github.com/w3c/webrtc-stats/issues/301 --- .../mochitest/test_peerConnection_stats.html | 32 +++++++++++-------- dom/media/webrtc/WebrtcGlobal.h | 2 ++ dom/webidl/RTCStatsReport.webidl | 1 + .../src/peerconnection/PeerConnectionImpl.cpp | 16 ++++++---- 4 files changed, 32 insertions(+), 19 deletions(-) diff --git a/dom/media/tests/mochitest/test_peerConnection_stats.html b/dom/media/tests/mochitest/test_peerConnection_stats.html index 00fba4eabfc7..8a64e3ba886b 100644 --- a/dom/media/tests/mochitest/test_peerConnection_stats.html +++ b/dom/media/tests/mochitest/test_peerConnection_stats.html @@ -13,7 +13,7 @@ var statsExpectedByType = { "inbound-rtp": { expected: ["id", "timestamp", "type", "ssrc", "isRemote", "mediaType", - "packetsReceived", "packetsLost", "bytesReceived", "jitter",], + "kind", "packetsReceived", "packetsLost", "bytesReceived", "jitter",], optional: ["roundTripTime", "remoteId", "nackCount",], localVideoOnly: ["discardedPackets", "framerateStdDev", "framerateMean", "bitrateMean", "bitrateStdDev", "firCount", "pliCount", "framesDecoded",], @@ -26,7 +26,7 @@ var statsExpectedByType = { }, "outbound-rtp": { expected: ["id", "timestamp", "type", "ssrc", "isRemote", "mediaType", - "packetsSent", "bytesSent", "remoteId",], + "kind", "packetsSent", "bytesSent", "remoteId",], optional: ["remoteId", "nackCount",], localVideoOnly: ["droppedFrames", "bitrateMean", "bitrateStdDev", "framerateMean", "framerateStdDev", "framesEncoded", "firCount", @@ -148,10 +148,16 @@ var pedanticChecks = report => { // isRemote ok(stat.isRemote !== undefined, stat.type + ".isRemote exists."); - // mediaType + // kind + ok(["audio", "video"].includes(stat.kind), + stat.type + ".kind is 'audio' or 'video'"); + + // mediaType, renamed to kind but remains for backward compability. ok(["audio", "video"].includes(stat.mediaType), stat.type + ".mediaType is 'audio' or 'video'"); + ok(stat.kind == stat.mediaType, "kind equals legacy mediaType"); + // remote id if (stat.remoteId) { ok(report.has(stat.remoteId), "remoteId exists in report."); @@ -169,7 +175,7 @@ var pedanticChecks = report => { + ".nackCount is only set when isRemote is false"); } - if (!stat.inner.isRemote && stat.inner.mediaType == "video") { + if (!stat.inner.isRemote && stat.inner.kind == "video") { // firCount ok(stat.firCount >= 0 && stat.firCount < 100, stat.type + ".firCount is a sane number for a short test. value=" @@ -205,7 +211,7 @@ var pedanticChecks = report => { + stat.packetsLost); // This should be much lower for audio, TODO: Bug 1330575 - let expectedJitter = stat.mediaType == "video" ? 0.5 : 1; + let expectedJitter = stat.kind == "video" ? 0.5 : 1; // jitter ok(stat.jitter < expectedJitter, stat.type + ".jitter is sane number for a local only test. value=" @@ -240,20 +246,20 @@ var pedanticChecks = report => { // // Local video only stats // - if (stat.inner.isRemote || stat.inner.mediaType != "video") { + if (stat.inner.isRemote || stat.inner.kind != "video") { expectations.localVideoOnly.forEach(field => { if (stat.inner.isRemote) { ok(stat[field] === undefined, stat.type + " does not have field " + field + " when isRemote is true"); - } else { // mediaType != video + } else { // kind != video ok(stat[field] === undefined, stat.type + " does not have field " - + field + " when mediaType is not 'video'"); + + field + " when kind is not 'video'"); } }); } else { expectations.localVideoOnly.forEach(field => { ok(stat.inner[field] !== undefined, stat.type + " has field " + field - + " when mediaType is video"); + + " when kind is video"); }); // discardedPackets ok(stat.discardedPackets < 100, stat.type @@ -321,20 +327,20 @@ var pedanticChecks = report => { // // Local video only stats // - if (stat.inner.isRemote || stat.inner.mediaType != "video") { + if (stat.inner.isRemote || stat.inner.kind != "video") { expectations.localVideoOnly.forEach(field => { if (stat.inner.isRemote) { ok(stat[field] === undefined, stat.type + " does not have field " + field + " when isRemote is true"); - } else { // mediaType != video + } else { // kind != video ok(stat[field] === undefined, stat.type + " does not have field " - + field + " when mediaType is not 'video'"); + + field + " when kind is not 'video'"); } }); } else { expectations.localVideoOnly.forEach(field => { ok(stat.inner[field] !== undefined, stat.type + " has field " + field - + " when mediaType is video and isRemote is false"); + + " when kind is video and isRemote is false"); }); // bitrateMean diff --git a/dom/media/webrtc/WebrtcGlobal.h b/dom/media/webrtc/WebrtcGlobal.h index d5892777f75f..b580566c6d9f 100644 --- a/dom/media/webrtc/WebrtcGlobal.h +++ b/dom/media/webrtc/WebrtcGlobal.h @@ -292,6 +292,7 @@ static void WriteRTCRtpStreamStats( WriteParam(aMsg, aParam.mIsRemote); WriteParam(aMsg, aParam.mMediaTrackId); WriteParam(aMsg, aParam.mMediaType); + WriteParam(aMsg, aParam.mKind); WriteParam(aMsg, aParam.mRemoteId); WriteParam(aMsg, aParam.mSsrc); WriteParam(aMsg, aParam.mTransportId); @@ -309,6 +310,7 @@ static bool ReadRTCRtpStreamStats( !ReadParam(aMsg, aIter, &(aResult->mIsRemote)) || !ReadParam(aMsg, aIter, &(aResult->mMediaTrackId)) || !ReadParam(aMsg, aIter, &(aResult->mMediaType)) || + !ReadParam(aMsg, aIter, &(aResult->mKind)) || !ReadParam(aMsg, aIter, &(aResult->mRemoteId)) || !ReadParam(aMsg, aIter, &(aResult->mSsrc)) || !ReadParam(aMsg, aIter, &(aResult->mTransportId))) { diff --git a/dom/webidl/RTCStatsReport.webidl b/dom/webidl/RTCStatsReport.webidl index ec54f56f981e..9cc42a078f9d 100644 --- a/dom/webidl/RTCStatsReport.webidl +++ b/dom/webidl/RTCStatsReport.webidl @@ -29,6 +29,7 @@ dictionary RTCStats { dictionary RTCRtpStreamStats : RTCStats { unsigned long ssrc; DOMString mediaType; + DOMString kind; DOMString remoteId; boolean isRemote = false; DOMString mediaTrackId; diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp index 7facb5899d8a..2da3d108866e 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp @@ -3433,9 +3433,9 @@ PeerConnectionImpl::ExecuteStatsQuery_s(RTCStatsQuery *query) { } const MediaPipeline& mp = *query->pipelines[p]; bool isAudio = (mp.Conduit()->type() == MediaSessionConduit::AUDIO); - nsString mediaType = isAudio ? + nsString kind = isAudio ? NS_LITERAL_STRING("audio") : NS_LITERAL_STRING("video"); - nsString idstr = mediaType; + nsString idstr = kind; idstr.AppendLiteral("_"); idstr.AppendInt((uint32_t)p); @@ -3472,7 +3472,8 @@ PeerConnectionImpl::ExecuteStatsQuery_s(RTCStatsQuery *query) { s.mId.Construct(remoteId); s.mType.Construct(RTCStatsType::Inbound_rtp); ssrc.apply([&s](uint32_t aSsrc){s.mSsrc.Construct(aSsrc);}); - s.mMediaType.Construct(mediaType); + s.mMediaType.Construct(kind); // mediaType is the old name for kind. + s.mKind.Construct(kind); s.mJitter.Construct(double(jitterMs)/1000); s.mRemoteId.Construct(localId); s.mIsRemote = true; @@ -3493,7 +3494,8 @@ PeerConnectionImpl::ExecuteStatsQuery_s(RTCStatsQuery *query) { s.mId.Construct(localId); s.mType.Construct(RTCStatsType::Outbound_rtp); ssrc.apply([&s](uint32_t aSsrc){s.mSsrc.Construct(aSsrc);}); - s.mMediaType.Construct(mediaType); + s.mMediaType.Construct(kind); // mediaType is the old name for kind. + s.mKind.Construct(kind); s.mRemoteId.Construct(remoteId); s.mIsRemote = false; s.mPacketsSent.Construct(mp.RtpPacketsSent()); @@ -3558,7 +3560,8 @@ PeerConnectionImpl::ExecuteStatsQuery_s(RTCStatsQuery *query) { s.mId.Construct(remoteId); s.mType.Construct(RTCStatsType::Outbound_rtp); ssrc.apply([&s](uint32_t aSsrc){s.mSsrc.Construct(aSsrc);}); - s.mMediaType.Construct(mediaType); + s.mMediaType.Construct(kind); // mediaType is the old name for kind. + s.mKind.Construct(kind); s.mRemoteId.Construct(localId); s.mIsRemote = true; s.mPacketsSent.Construct(packetsSent); @@ -3573,7 +3576,8 @@ PeerConnectionImpl::ExecuteStatsQuery_s(RTCStatsQuery *query) { s.mId.Construct(localId); s.mType.Construct(RTCStatsType::Inbound_rtp); ssrc.apply([&s](uint32_t aSsrc){s.mSsrc.Construct(aSsrc);}); - s.mMediaType.Construct(mediaType); + s.mMediaType.Construct(kind); // mediaType is the old name for kind. + s.mKind.Construct(kind); unsigned int jitterMs, packetsLost; if (mp.Conduit()->GetRTPStats(&jitterMs, &packetsLost)) { s.mJitter.Construct(double(jitterMs)/1000); From 76325b1cffdb684b66edd27b92eca94bab19a35f Mon Sep 17 00:00:00 2001 From: Ryan Hunt Date: Wed, 22 Aug 2018 09:34:22 -0500 Subject: [PATCH 13/45] Bug 1485371 - Re-enable tiling for OpenBSD. r=nical --HG-- extra : rebase_source : 83a8e6533312cf8db9aee9e70dfdbfa21ab13bb0 extra : histedit_source : 6ec64a8cb17d4e4b10a487c5c676d63edbfa9dc4 --- modules/libpref/init/all.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 223a1b10a34f..ec3dc475658b 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -4894,7 +4894,7 @@ pref("layers.max-active", -1); // 0 -> full-tilt mode: Recomposite even if not transaction occured. pref("layers.offmainthreadcomposition.frame-rate", -1); -#if defined(XP_MACOSX) +#if defined(XP_MACOSX) || defined (OS_OPENBSD) pref("layers.enable-tiles", true); #else pref("layers.enable-tiles", false); From b6429577e3de29a03f9a0a185009527b3c2c9498 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Mon, 20 Aug 2018 19:32:32 -0400 Subject: [PATCH 14/45] Bug 1484876 - Part 1: Refactor the PrivateBrowsingTrackingProtectionWhitelist service on top of the permission manager; r=johannh Session permissions aren't persisted to disk, so we can easily add a new permission type and only store session permissions in the permission manager database, and drop the in-memory allow list that this service maintains. This has the advantage that the permission manager already has the IPC machinery to make this information available in the content processes, so we can also check this allow list in the content process. --- ...ivateBrowsingTrackingProtectionWhitelist.js | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/toolkit/components/privatebrowsing/PrivateBrowsingTrackingProtectionWhitelist.js b/toolkit/components/privatebrowsing/PrivateBrowsingTrackingProtectionWhitelist.js index 9347ac7e74a9..b0b7aeaac7a9 100644 --- a/toolkit/components/privatebrowsing/PrivateBrowsingTrackingProtectionWhitelist.js +++ b/toolkit/components/privatebrowsing/PrivateBrowsingTrackingProtectionWhitelist.js @@ -6,9 +6,6 @@ ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); ChromeUtils.import("resource://gre/modules/Services.jsm"); function PrivateBrowsingTrackingProtectionWhitelist() { - // The list of URIs explicitly excluded from tracking protection. - this._allowlist = []; - Services.obs.addObserver(this, "last-pb-context-exited", true); } @@ -24,9 +21,8 @@ PrivateBrowsingTrackingProtectionWhitelist.prototype = { * The URI to add to the list. */ addToAllowList(uri) { - if (!this._allowlist.includes(uri.spec)) { - this._allowlist.push(uri.spec); - } + Services.perms.add(uri, "trackingprotection-pb", Ci.nsIPermissionManager.ALLOW_ACTION, + Ci.nsIPermissionManager.EXPIRE_SESSION); }, /** @@ -36,10 +32,7 @@ PrivateBrowsingTrackingProtectionWhitelist.prototype = { * The URI to add to the list. */ removeFromAllowList(uri) { - let index = this._allowlist.indexOf(uri.spec); - if (index !== -1) { - this._allowlist.splice(index, 1); - } + Services.perms.remove(uri, "trackingprotection-pb"); }, /** @@ -49,12 +42,13 @@ PrivateBrowsingTrackingProtectionWhitelist.prototype = { * The URI to add to the list. */ existsInAllowList(uri) { - return this._allowlist.includes(uri.spec); + return Services.perms.testPermission(uri, "trackingprotection-pb") == + Ci.nsIPermissionManager.ALLOW_ACTION; }, observe(subject, topic, data) { if (topic == "last-pb-context-exited") { - this._allowlist = []; + Services.perms.removeByType("trackingprotection-pb"); } } }; From 7a3e0c65f069e568f8fed720ef66352e1b999d0d Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Mon, 20 Aug 2018 19:46:58 -0400 Subject: [PATCH 15/45] Bug 1484876 - Part 2: Check the trackingprotection-pb permission in the channel classifier code instead of accessing nsIPrivateBrowsingTrackingProtectionWhitelist; r=francois --- netwerk/base/nsChannelClassifier.cpp | 49 ++++++++++------------------ 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/netwerk/base/nsChannelClassifier.cpp b/netwerk/base/nsChannelClassifier.cpp index 81990a6eeff5..be7935f17155 100644 --- a/netwerk/base/nsChannelClassifier.cpp +++ b/netwerk/base/nsChannelClassifier.cpp @@ -22,7 +22,6 @@ #include "nsIIOService.h" #include "nsIParentChannel.h" #include "nsIPermissionManager.h" -#include "nsIPrivateBrowsingTrackingProtectionWhitelist.h" #include "nsIProtocolHandler.h" #include "nsIScriptError.h" #include "nsIScriptSecurityManager.h" @@ -487,44 +486,32 @@ nsChannelClassifier::ShouldEnableTrackingProtectionInternal( do_GetService(NS_PERMISSIONMANAGER_CONTRACTID, &rv); NS_ENSURE_SUCCESS(rv, rv); - uint32_t permissions = nsIPermissionManager::UNKNOWN_ACTION; - rv = permMgr->TestPermission(topWinURI, "trackingprotection", &permissions); - NS_ENSURE_SUCCESS(rv, rv); + // Check both the normal mode and private browsing mode user override permissions. + const char* types[] = { + "trackingprotection", + "trackingprotection-pb" + }; - if (permissions == nsIPermissionManager::ALLOW_ACTION) { - if (LOG_ENABLED()) { - nsCString chanSpec = chanURI->GetSpecOrDefault(); - chanSpec.Truncate(std::min(chanSpec.Length(), sMaxSpecLength)); - LOG(("nsChannelClassifier[%p]: User override on channel[%p] (%s) for %s", - this, aChannel, chanSpec.get(), escaped.get())); - } - mIsAllowListed = true; - *result = false; - } else { - *result = true; - } - - // In Private Browsing Mode we also check against an in-memory list. - if (NS_UsePrivateBrowsing(aChannel)) { - nsCOMPtr pbmtpWhitelist = - do_GetService(NS_PBTRACKINGPROTECTIONWHITELIST_CONTRACTID, &rv); + for (size_t i = 0; i < ArrayLength(types); ++i) { + uint32_t permissions = nsIPermissionManager::UNKNOWN_ACTION; + rv = permMgr->TestPermission(topWinURI, types[i], &permissions); NS_ENSURE_SUCCESS(rv, rv); - bool exists = false; - rv = pbmtpWhitelist->ExistsInAllowList(topWinURI, &exists); - NS_ENSURE_SUCCESS(rv, rv); - - if (exists) { - mIsAllowListed = true; + if (permissions == nsIPermissionManager::ALLOW_ACTION) { if (LOG_ENABLED()) { nsCString chanSpec = chanURI->GetSpecOrDefault(); chanSpec.Truncate(std::min(chanSpec.Length(), sMaxSpecLength)); - LOG(("nsChannelClassifier[%p]: User override (PBM) on channel[%p] (%s) for %s", - this, aChannel, chanSpec.get(), escaped.get())); + LOG(("nsChannelClassifier[%p]: User override on channel[%p] (%s) for %s (%s)", + this, aChannel, chanSpec.get(), escaped.get(), types[i])); } + mIsAllowListed = true; + *result = false; + // Stop checking the next permisson type if we decided to override. + break; + } else if (i == ArrayLength(types) - 1) { + // Do this on the last iteration. + *result = true; } - - *result = !exists; } // Tracking protection will be enabled so return without updating From b1a6405b4c62752b1f8c80cb1e32944966fd54b0 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Mon, 20 Aug 2018 20:09:52 -0400 Subject: [PATCH 16/45] Bug 1484876 - Part 3: Refactor the code responsible for checking whether the top window URI is on the content blocking allow list into AntiTrackingCommon; r=francois --- netwerk/base/nsChannelClassifier.cpp | 58 ++++------------- .../antitracking/AntiTrackingCommon.cpp | 65 +++++++++++++++++++ .../antitracking/AntiTrackingCommon.h | 5 ++ 3 files changed, 83 insertions(+), 45 deletions(-) diff --git a/netwerk/base/nsChannelClassifier.cpp b/netwerk/base/nsChannelClassifier.cpp index be7935f17155..9d40fc69893f 100644 --- a/netwerk/base/nsChannelClassifier.cpp +++ b/netwerk/base/nsChannelClassifier.cpp @@ -37,6 +37,7 @@ #include "nsIUrlClassifierDBService.h" #include "nsIURLFormatter.h" +#include "mozilla/AntiTrackingCommon.h" #include "mozilla/ErrorNames.h" #include "mozilla/Logging.h" #include "mozilla/Preferences.h" @@ -448,8 +449,8 @@ nsChannelClassifier::ShouldEnableTrackingProtectionInternal( return NS_OK; } - nsCOMPtr ios = do_GetService(NS_IOSERVICE_CONTRACTID, &rv); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr ios = services::GetIOService(); + NS_ENSURE_TRUE(ios, NS_ERROR_FAILURE); nsCOMPtr topWinURI; rv = chan->GetTopWindowURI(getter_AddRefs(topWinURI)); @@ -464,54 +465,21 @@ nsChannelClassifier::ShouldEnableTrackingProtectionInternal( NS_ENSURE_SUCCESS(rv, rv); } - // Take the host/port portion so we can allowlist by site. Also ignore the - // scheme, since users who put sites on the allowlist probably don't expect - // allowlisting to depend on scheme. - nsCOMPtr url = do_QueryInterface(topWinURI, &rv); + rv = AntiTrackingCommon::IsOnContentBlockingAllowList(topWinURI, mIsAllowListed); if (NS_FAILED(rv)) { return rv; // normal for some loads, no need to print a warning } - nsCString escaped(NS_LITERAL_CSTRING("https://")); - nsAutoCString temp; - rv = url->GetHostPort(temp); - NS_ENSURE_SUCCESS(rv, rv); - escaped.Append(temp); - - // Stuff the whole thing back into a URI for the permission manager. - rv = ios->NewURI(escaped, nullptr, nullptr, getter_AddRefs(topWinURI)); - NS_ENSURE_SUCCESS(rv, rv); - - nsCOMPtr permMgr = - do_GetService(NS_PERMISSIONMANAGER_CONTRACTID, &rv); - NS_ENSURE_SUCCESS(rv, rv); - - // Check both the normal mode and private browsing mode user override permissions. - const char* types[] = { - "trackingprotection", - "trackingprotection-pb" - }; - - for (size_t i = 0; i < ArrayLength(types); ++i) { - uint32_t permissions = nsIPermissionManager::UNKNOWN_ACTION; - rv = permMgr->TestPermission(topWinURI, types[i], &permissions); - NS_ENSURE_SUCCESS(rv, rv); - - if (permissions == nsIPermissionManager::ALLOW_ACTION) { - if (LOG_ENABLED()) { - nsCString chanSpec = chanURI->GetSpecOrDefault(); - chanSpec.Truncate(std::min(chanSpec.Length(), sMaxSpecLength)); - LOG(("nsChannelClassifier[%p]: User override on channel[%p] (%s) for %s (%s)", - this, aChannel, chanSpec.get(), escaped.get(), types[i])); - } - mIsAllowListed = true; - *result = false; - // Stop checking the next permisson type if we decided to override. - break; - } else if (i == ArrayLength(types) - 1) { - // Do this on the last iteration. - *result = true; + if (mIsAllowListed) { + *result = false; + if (LOG_ENABLED()) { + nsCString chanSpec = chanURI->GetSpecOrDefault(); + chanSpec.Truncate(std::min(chanSpec.Length(), sMaxSpecLength)); + LOG(("nsChannelClassifier[%p]: User override on channel[%p] (%s)", + this, aChannel, chanSpec.get())); } + } else { + *result = true; } // Tracking protection will be enabled so return without updating diff --git a/toolkit/components/antitracking/AntiTrackingCommon.cpp b/toolkit/components/antitracking/AntiTrackingCommon.cpp index 58d0ab4ec478..9c0d4f548679 100644 --- a/toolkit/components/antitracking/AntiTrackingCommon.cpp +++ b/toolkit/components/antitracking/AntiTrackingCommon.cpp @@ -16,9 +16,11 @@ #include "nsGlobalWindowInner.h" #include "nsICookiePermission.h" #include "nsICookieService.h" +#include "nsIIOService.h" #include "nsIPermissionManager.h" #include "nsIPrincipal.h" #include "nsIURI.h" +#include "nsIURL.h" #include "nsPIDOMWindow.h" #include "nsScriptSecurityManager.h" #include "prtime.h" @@ -642,3 +644,66 @@ AntiTrackingCommon::MaybeIsFirstPartyStorageAccessGrantedFor(nsPIDOMWindowInner* return result == nsIPermissionManager::ALLOW_ACTION; } + +nsresult +AntiTrackingCommon::IsOnContentBlockingAllowList(nsIURI* aTopWinURI, + bool& aIsAllowListed) +{ + aIsAllowListed = false; + + LOG_SPEC(("Deciding whether the user has overridden content blocking for %s", + _spec), aTopWinURI); + + nsCOMPtr ios = services::GetIOService(); + NS_ENSURE_TRUE(ios, NS_ERROR_FAILURE); + + // Take the host/port portion so we can allowlist by site. Also ignore the + // scheme, since users who put sites on the allowlist probably don't expect + // allowlisting to depend on scheme. + nsresult rv = NS_ERROR_FAILURE; + nsCOMPtr url = do_QueryInterface(aTopWinURI, &rv); + if (NS_FAILED(rv)) { + return rv; // normal for some loads, no need to print a warning + } + + nsCString escaped(NS_LITERAL_CSTRING("https://")); + nsAutoCString temp; + rv = url->GetHostPort(temp); + NS_ENSURE_SUCCESS(rv, rv); + escaped.Append(temp); + + // Stuff the whole thing back into a URI for the permission manager. + nsCOMPtr topWinURI; + rv = ios->NewURI(escaped, nullptr, nullptr, getter_AddRefs(topWinURI)); + NS_ENSURE_SUCCESS(rv, rv); + + nsCOMPtr permMgr = + do_GetService(NS_PERMISSIONMANAGER_CONTRACTID, &rv); + NS_ENSURE_SUCCESS(rv, rv); + + // Check both the normal mode and private browsing mode user override permissions. + const char* types[] = { + "trackingprotection", + "trackingprotection-pb" + }; + + for (size_t i = 0; i < ArrayLength(types); ++i) { + uint32_t permissions = nsIPermissionManager::UNKNOWN_ACTION; + rv = permMgr->TestPermission(topWinURI, types[i], &permissions); + NS_ENSURE_SUCCESS(rv, rv); + + if (permissions == nsIPermissionManager::ALLOW_ACTION) { + aIsAllowListed = true; + LOG_SPEC(("Found user override type %s for %s", types[i], _spec), + topWinURI); + // Stop checking the next permisson type if we decided to override. + break; + } + } + + if (!aIsAllowListed) { + LOG(("No user override found")); + } + + return NS_OK; +} diff --git a/toolkit/components/antitracking/AntiTrackingCommon.h b/toolkit/components/antitracking/AntiTrackingCommon.h index 390fe742536c..4733589ffdf9 100644 --- a/toolkit/components/antitracking/AntiTrackingCommon.h +++ b/toolkit/components/antitracking/AntiTrackingCommon.h @@ -83,6 +83,11 @@ public: const nsCString& aGrantedOrigin, FirstPartyStorageAccessGrantedForOriginResolver&& aResolver); + + // Check whether a top window URI is on the content blocking allow list. + static nsresult + IsOnContentBlockingAllowList(nsIURI* aTopWinURI, bool& aIsAllowListed); + }; } // namespace mozilla From 2346eb62c8013d8b1e9aab37c7cc0b4313e660cc Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 21 Aug 2018 09:55:49 -0400 Subject: [PATCH 17/45] Bug 1484876 - Part 4: Remove nsIPrivateBrowsingTrackingProtectionWhitelist.existsInAllowList(); r=johannh --- browser/base/content/browser-contentblocking.js | 12 +++++------- .../PrivateBrowsingTrackingProtectionWhitelist.js | 11 ----------- ...nsIPrivateBrowsingTrackingProtectionWhitelist.idl | 8 -------- toolkit/modules/PrivateBrowsingUtils.jsm | 4 ---- 4 files changed, 5 insertions(+), 30 deletions(-) diff --git a/browser/base/content/browser-contentblocking.js b/browser/base/content/browser-contentblocking.js index 4152504fdb5d..a94d905b8170 100644 --- a/browser/base/content/browser-contentblocking.js +++ b/browser/base/content/browser-contentblocking.js @@ -412,13 +412,11 @@ var ContentBlocking = { } // Check whether the user has added an exception for this site. - let hasException = false; - if (PrivateBrowsingUtils.isBrowserPrivate(gBrowser.selectedBrowser)) { - hasException = PrivateBrowsingUtils.existsInTrackingAllowlist(baseURI); - } else { - hasException = Services.perms.testExactPermission(baseURI, - "trackingprotection") == Services.perms.ALLOW_ACTION; - } + let type = PrivateBrowsingUtils.isBrowserPrivate(gBrowser.selectedBrowser) ? + "trackingprotection-pb" : + "trackingprotection"; + let hasException = Services.perms.testExactPermission(baseURI, type) == + Services.perms.ALLOW_ACTION; this.content.toggleAttribute("detected", detected); this.content.toggleAttribute("hasException", hasException); diff --git a/toolkit/components/privatebrowsing/PrivateBrowsingTrackingProtectionWhitelist.js b/toolkit/components/privatebrowsing/PrivateBrowsingTrackingProtectionWhitelist.js index b0b7aeaac7a9..0dda9f41d776 100644 --- a/toolkit/components/privatebrowsing/PrivateBrowsingTrackingProtectionWhitelist.js +++ b/toolkit/components/privatebrowsing/PrivateBrowsingTrackingProtectionWhitelist.js @@ -35,17 +35,6 @@ PrivateBrowsingTrackingProtectionWhitelist.prototype = { Services.perms.remove(uri, "trackingprotection-pb"); }, - /** - * Check if the provided URI exists in the list of allowed tracking sites. - * - * @param uri nsIURI - * The URI to add to the list. - */ - existsInAllowList(uri) { - return Services.perms.testPermission(uri, "trackingprotection-pb") == - Ci.nsIPermissionManager.ALLOW_ACTION; - }, - observe(subject, topic, data) { if (topic == "last-pb-context-exited") { Services.perms.removeByType("trackingprotection-pb"); diff --git a/toolkit/components/privatebrowsing/nsIPrivateBrowsingTrackingProtectionWhitelist.idl b/toolkit/components/privatebrowsing/nsIPrivateBrowsingTrackingProtectionWhitelist.idl index d572b4e7e15f..a8edb7e3b169 100644 --- a/toolkit/components/privatebrowsing/nsIPrivateBrowsingTrackingProtectionWhitelist.idl +++ b/toolkit/components/privatebrowsing/nsIPrivateBrowsingTrackingProtectionWhitelist.idl @@ -31,14 +31,6 @@ interface nsIPrivateBrowsingTrackingProtectionWhitelist : nsISupports * @param uri the uri to remove from the list */ void removeFromAllowList(in nsIURI uri); - - /** - * Check if a URI exists in the list of allowed tracking sites in Private - * Browsing mode (the tracking whitelist). - * - * @param uri the uri to look for in the list - */ - bool existsInAllowList(in nsIURI uri); }; %{ C++ diff --git a/toolkit/modules/PrivateBrowsingUtils.jsm b/toolkit/modules/PrivateBrowsingUtils.jsm index 1ab60eb846ad..94d35811dfa0 100644 --- a/toolkit/modules/PrivateBrowsingUtils.jsm +++ b/toolkit/modules/PrivateBrowsingUtils.jsm @@ -60,10 +60,6 @@ var PrivateBrowsingUtils = { gPBMTPWhitelist.addToAllowList(aURI); }, - existsInTrackingAllowlist(aURI) { - return gPBMTPWhitelist.existsInAllowList(aURI); - }, - removeFromTrackingAllowlist(aURI) { gPBMTPWhitelist.removeFromAllowList(aURI); }, From d99f8f170fe888c04ef98d3de88f64bd0bf6569e Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 21 Aug 2018 10:03:58 -0400 Subject: [PATCH 18/45] Bug 1484876 - Part 5: Merge the PrivateBrowsingTrackingProtectionWhitelist service with PrivateBrowsingUtils.jsm; r=johannh --- browser/installer/package-manifest.in | 4 -- mobile/android/installer/package-manifest.in | 4 -- toolkit/components/moz.build | 1 - .../privatebrowsing/PrivateBrowsing.manifest | 2 - ...vateBrowsingTrackingProtectionWhitelist.js | 45 ------------------ toolkit/components/privatebrowsing/moz.build | 19 -------- ...ateBrowsingTrackingProtectionWhitelist.idl | 38 --------------- toolkit/modules/PrivateBrowsingUtils.jsm | 47 ++++++++++++++++--- 8 files changed, 41 insertions(+), 119 deletions(-) delete mode 100644 toolkit/components/privatebrowsing/PrivateBrowsing.manifest delete mode 100644 toolkit/components/privatebrowsing/PrivateBrowsingTrackingProtectionWhitelist.js delete mode 100644 toolkit/components/privatebrowsing/moz.build delete mode 100644 toolkit/components/privatebrowsing/nsIPrivateBrowsingTrackingProtectionWhitelist.idl diff --git a/browser/installer/package-manifest.in b/browser/installer/package-manifest.in index 2ac99e7c0751..5b828784a048 100644 --- a/browser/installer/package-manifest.in +++ b/browser/installer/package-manifest.in @@ -388,10 +388,6 @@ @RESPATH@/components/nsUrlClassifierListManager.js @RESPATH@/components/nsUrlClassifierLib.js -; Private Browsing -@RESPATH@/components/PrivateBrowsing.manifest -@RESPATH@/components/PrivateBrowsingTrackingProtectionWhitelist.js - ; Security Reports @RESPATH@/components/SecurityReporter.manifest @RESPATH@/components/SecurityReporter.js diff --git a/mobile/android/installer/package-manifest.in b/mobile/android/installer/package-manifest.in index c261afd748d0..9327968203bf 100644 --- a/mobile/android/installer/package-manifest.in +++ b/mobile/android/installer/package-manifest.in @@ -253,10 +253,6 @@ @BINPATH@/components/nsUrlClassifierListManager.js @BINPATH@/components/nsUrlClassifierLib.js -; Private Browsing -@BINPATH@/components/PrivateBrowsing.manifest -@BINPATH@/components/PrivateBrowsingTrackingProtectionWhitelist.js - ; Security Reports @BINPATH@/components/SecurityReporter.manifest @BINPATH@/components/SecurityReporter.js diff --git a/toolkit/components/moz.build b/toolkit/components/moz.build index a58dbc7b65ca..77b743c140e9 100644 --- a/toolkit/components/moz.build +++ b/toolkit/components/moz.build @@ -49,7 +49,6 @@ DIRS += [ 'perf', 'perfmonitoring', 'places', - 'privatebrowsing', 'processsingleton', 'promiseworker', 'prompts', diff --git a/toolkit/components/privatebrowsing/PrivateBrowsing.manifest b/toolkit/components/privatebrowsing/PrivateBrowsing.manifest deleted file mode 100644 index 36b39bb85ee2..000000000000 --- a/toolkit/components/privatebrowsing/PrivateBrowsing.manifest +++ /dev/null @@ -1,2 +0,0 @@ -component {a319b616-c45d-4037-8d86-01c592b5a9af} PrivateBrowsingTrackingProtectionWhitelist.js -contract @mozilla.org/pbm-tp-whitelist;1 {a319b616-c45d-4037-8d86-01c592b5a9af} diff --git a/toolkit/components/privatebrowsing/PrivateBrowsingTrackingProtectionWhitelist.js b/toolkit/components/privatebrowsing/PrivateBrowsingTrackingProtectionWhitelist.js deleted file mode 100644 index 0dda9f41d776..000000000000 --- a/toolkit/components/privatebrowsing/PrivateBrowsingTrackingProtectionWhitelist.js +++ /dev/null @@ -1,45 +0,0 @@ -/* 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/. */ - -ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); -ChromeUtils.import("resource://gre/modules/Services.jsm"); - -function PrivateBrowsingTrackingProtectionWhitelist() { - Services.obs.addObserver(this, "last-pb-context-exited", true); -} - -PrivateBrowsingTrackingProtectionWhitelist.prototype = { - classID: Components.ID("{a319b616-c45d-4037-8d86-01c592b5a9af}"), - QueryInterface: ChromeUtils.generateQI([Ci.nsIPrivateBrowsingTrackingProtectionWhitelist, Ci.nsIObserver, Ci.nsISupportsWeakReference]), - _xpcom_factory: XPCOMUtils.generateSingletonFactory(PrivateBrowsingTrackingProtectionWhitelist), - - /** - * Add the provided URI to the list of allowed tracking sites. - * - * @param uri nsIURI - * The URI to add to the list. - */ - addToAllowList(uri) { - Services.perms.add(uri, "trackingprotection-pb", Ci.nsIPermissionManager.ALLOW_ACTION, - Ci.nsIPermissionManager.EXPIRE_SESSION); - }, - - /** - * Remove the provided URI from the list of allowed tracking sites. - * - * @param uri nsIURI - * The URI to add to the list. - */ - removeFromAllowList(uri) { - Services.perms.remove(uri, "trackingprotection-pb"); - }, - - observe(subject, topic, data) { - if (topic == "last-pb-context-exited") { - Services.perms.removeByType("trackingprotection-pb"); - } - } -}; - -this.NSGetFactory = XPCOMUtils.generateNSGetFactory([PrivateBrowsingTrackingProtectionWhitelist]); diff --git a/toolkit/components/privatebrowsing/moz.build b/toolkit/components/privatebrowsing/moz.build deleted file mode 100644 index 60e4cf9c0223..000000000000 --- a/toolkit/components/privatebrowsing/moz.build +++ /dev/null @@ -1,19 +0,0 @@ -# -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*- -# vim: set filetype=python: -# 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/. - -with Files('**'): - BUG_COMPONENT = ('Core', 'DOM: Security') - -XPIDL_SOURCES += [ - 'nsIPrivateBrowsingTrackingProtectionWhitelist.idl', -] - -XPIDL_MODULE = 'privatebrowsing' - -EXTRA_COMPONENTS += [ - 'PrivateBrowsing.manifest', - 'PrivateBrowsingTrackingProtectionWhitelist.js', -] diff --git a/toolkit/components/privatebrowsing/nsIPrivateBrowsingTrackingProtectionWhitelist.idl b/toolkit/components/privatebrowsing/nsIPrivateBrowsingTrackingProtectionWhitelist.idl deleted file mode 100644 index a8edb7e3b169..000000000000 --- a/toolkit/components/privatebrowsing/nsIPrivateBrowsingTrackingProtectionWhitelist.idl +++ /dev/null @@ -1,38 +0,0 @@ -/* 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/. */ - -#include "nsISupports.idl" - -interface nsIURI; - -/** - * The Private Browsing Tracking Protection service checks a URI against an - * in-memory list of tracking sites. - */ -[scriptable, uuid(c77ddfac-6cd6-43a9-84e8-91682a1a7b18)] -interface nsIPrivateBrowsingTrackingProtectionWhitelist : nsISupports -{ - /** - * Add a URI to the list of allowed tracking sites in Private Browsing mode - * (essentially a tracking whitelist). This operation will cause the URI to - * be registered if it does not currently exist. If it already exists, then - * the operation is essentially a no-op. - * - * @param uri the uri to add to the list - */ - void addToAllowList(in nsIURI uri); - - /** - * Remove a URI from the list of allowed tracking sites in Private Browsing - * mode (the tracking whitelist). If the URI is not already in the list, - * then the operation is essentially a no-op. - * - * @param uri the uri to remove from the list - */ - void removeFromAllowList(in nsIURI uri); -}; - -%{ C++ -#define NS_PBTRACKINGPROTECTIONWHITELIST_CONTRACTID "@mozilla.org/pbm-tp-whitelist;1" -%} diff --git a/toolkit/modules/PrivateBrowsingUtils.jsm b/toolkit/modules/PrivateBrowsingUtils.jsm index 94d35811dfa0..1f0132f9d0e8 100644 --- a/toolkit/modules/PrivateBrowsingUtils.jsm +++ b/toolkit/modules/PrivateBrowsingUtils.jsm @@ -5,11 +5,41 @@ var EXPORTED_SYMBOLS = ["PrivateBrowsingUtils"]; ChromeUtils.import("resource://gre/modules/Services.jsm"); -ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); -XPCOMUtils.defineLazyServiceGetter(this, "gPBMTPWhitelist", - "@mozilla.org/pbm-tp-whitelist;1", - "nsIPrivateBrowsingTrackingProtectionWhitelist"); +function PrivateBrowsingContentBlockingAllowList() { + Services.obs.addObserver(this, "last-pb-context-exited", true); +} + +PrivateBrowsingContentBlockingAllowList.prototype = { + QueryInterface: ChromeUtils.generateQI([Ci.nsIObserver, Ci.nsISupportsWeakReference]), + + /** + * Add the provided URI to the list of allowed tracking sites. + * + * @param uri nsIURI + * The URI to add to the list. + */ + addToAllowList(uri) { + Services.perms.add(uri, "trackingprotection-pb", Ci.nsIPermissionManager.ALLOW_ACTION, + Ci.nsIPermissionManager.EXPIRE_SESSION); + }, + + /** + * Remove the provided URI from the list of allowed tracking sites. + * + * @param uri nsIURI + * The URI to remove from the list. + */ + removeFromAllowList(uri) { + Services.perms.remove(uri, "trackingprotection-pb"); + }, + + observe(subject, topic, data) { + if (topic == "last-pb-context-exited") { + Services.perms.removeByType("trackingprotection-pb"); + } + } +}; const kAutoStartPref = "browser.privatebrowsing.autostart"; @@ -56,12 +86,17 @@ var PrivateBrowsingUtils = { return aWindow.docShell.QueryInterface(Ci.nsILoadContext); }, + get _pbCBAllowList() { + delete this._pbCBAllowList; + return this._pbCBAllowList = new PrivateBrowsingContentBlockingAllowList(); + }, + addToTrackingAllowlist(aURI) { - gPBMTPWhitelist.addToAllowList(aURI); + this._pbCBAllowList.addToAllowList(aURI); }, removeFromTrackingAllowlist(aURI) { - gPBMTPWhitelist.removeFromAllowList(aURI); + this._pbCBAllowList.removeFromAllowList(aURI); }, get permanentPrivateBrowsing() { From e0e604fbc5e8f4e4c9478daccf45d840d8d69f44 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Wed, 22 Aug 2018 09:07:00 -0400 Subject: [PATCH 19/45] Bug 1484876 - Part 6: Truncate the tracking URIs in the anti-tracking logs to 128 characters since they may be really long; r=francois --- toolkit/components/antitracking/AntiTrackingCommon.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/toolkit/components/antitracking/AntiTrackingCommon.cpp b/toolkit/components/antitracking/AntiTrackingCommon.cpp index 9c0d4f548679..8632f9b618c3 100644 --- a/toolkit/components/antitracking/AntiTrackingCommon.cpp +++ b/toolkit/components/antitracking/AntiTrackingCommon.cpp @@ -31,6 +31,7 @@ using namespace mozilla; using mozilla::dom::ContentChild; static LazyLogModule gAntiTrackingLog("AntiTracking"); +static const nsCString::size_type sMaxSpecLength = 128; #define LOG(format) MOZ_LOG(gAntiTrackingLog, mozilla::LogLevel::Debug, format) @@ -38,6 +39,7 @@ static LazyLogModule gAntiTrackingLog("AntiTracking"); PR_BEGIN_MACRO \ if (MOZ_LOG_TEST(gAntiTrackingLog, mozilla::LogLevel::Debug)) { \ nsAutoCString _specStr(NS_LITERAL_CSTRING("(null)")); \ + _specStr.Truncate(std::min(_specStr.Length(), sMaxSpecLength)); \ if (uri) { \ _specStr = uri->GetSpecOrDefault(); \ } \ From 13c9216e3613b226ac93199780dab27ad00c1fd7 Mon Sep 17 00:00:00 2001 From: Nathan Froyd Date: Wed, 22 Aug 2018 12:15:19 -0400 Subject: [PATCH 20/45] Bug 1485367 - disable libjpeg-turbo support for aarch64 windows; r=dmajor --- old-configure.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/old-configure.in b/old-configure.in index b4f7c4b2a564..30ed601d8423 100644 --- a/old-configure.in +++ b/old-configure.in @@ -2788,6 +2788,8 @@ if test -n "$MOZ_LIBJPEG_TURBO" -a -n "$COMPILE_ENVIRONMENT"; then WINNT:x86_64) LIBJPEG_TURBO_ASFLAGS="-D__x86_64__ -DPIC -DWIN64 -DMSVC" ;; + WINNT:*) + ;; *:arm) LIBJPEG_TURBO_ASFLAGS="-march=armv7-a -mfpu=neon" ;; From 704e337fc8e3f50a42359d3ee0fe7e6551a13523 Mon Sep 17 00:00:00 2001 From: Cosmin Sabou Date: Wed, 22 Aug 2018 19:27:02 +0300 Subject: [PATCH 21/45] Backed out changeset 0971297169a6 (bug 1480278) for linting failure on border-image-017.xht.ini --- .../tests/css/css-backgrounds/border-image-017.xht.ini | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 testing/web-platform/tests/css/css-backgrounds/border-image-017.xht.ini diff --git a/testing/web-platform/tests/css/css-backgrounds/border-image-017.xht.ini b/testing/web-platform/tests/css/css-backgrounds/border-image-017.xht.ini deleted file mode 100644 index 719701537ff6..000000000000 --- a/testing/web-platform/tests/css/css-backgrounds/border-image-017.xht.ini +++ /dev/null @@ -1,3 +0,0 @@ -[border-image-017] - disabled: - if (os == "linux"): https://bugzilla.mozilla.org/show_bug.cgi?id=1480278 From 9d4f4b2a44a49da556e0dd6369069a7951a1078c Mon Sep 17 00:00:00 2001 From: Masatoshi Kimura Date: Wed, 22 Aug 2018 02:02:29 +0900 Subject: [PATCH 22/45] Bug 1485028 - Use GetWindowsStackSize() on Windows even with ASAN builds. r=jandem --HG-- extra : source : 62941c04d652a0ce3688be069fb9dd997dd3c4a7 extra : intermediate-source : d78e18c21d52037de9ae0e9eaa44bfa4c37ad5ba --- js/xpconnect/src/XPCJSContext.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/js/xpconnect/src/XPCJSContext.cpp b/js/xpconnect/src/XPCJSContext.cpp index 59ebfd9079e1..47887d0c6926 100644 --- a/js/xpconnect/src/XPCJSContext.cpp +++ b/js/xpconnect/src/XPCJSContext.cpp @@ -1087,6 +1087,18 @@ XPCJSContext::Initialize(XPCJSContext* aPrimaryContext) # else const size_t kTrustedScriptBuffer = 180 * 1024; # endif +#elif defined(XP_WIN) + // 1MB is the default stack size on Windows. We use the -STACK linker flag + // (see WIN32_EXE_LDFLAGS in config/config.mk) to request a larger stack, + // so we determine the stack size at runtime. + const size_t kStackQuota = GetWindowsStackSize(); +# if defined(MOZ_ASAN) + // See the standalone MOZ_ASAN branch below for the ASan case. + const size_t kTrustedScriptBuffer = 450 * 1024; +# else + const size_t kTrustedScriptBuffer = (sizeof(size_t) == 8) ? 180 * 1024 //win64 + : 120 * 1024; //win32 +# endif #elif defined(MOZ_ASAN) // ASan requires more stack space due to red-zones, so give it double the // default (1MB on 32-bit, 2MB on 64-bit). ASAN stack frame measurements @@ -1099,13 +1111,6 @@ XPCJSContext::Initialize(XPCJSContext* aPrimaryContext) // (See bug 1415195) const size_t kStackQuota = 2 * kDefaultStackQuota; const size_t kTrustedScriptBuffer = 450 * 1024; -#elif defined(XP_WIN) - // 1MB is the default stack size on Windows. We use the -STACK linker flag - // (see WIN32_EXE_LDFLAGS in config/config.mk) to request a larger stack, - // so we determine the stack size at runtime. - const size_t kStackQuota = GetWindowsStackSize(); - const size_t kTrustedScriptBuffer = (sizeof(size_t) == 8) ? 180 * 1024 //win64 - : 120 * 1024; //win32 #elif defined(ANDROID) // Android appears to have 1MB stacks. Allow the use of 3/4 of that size // (768KB on 32-bit), since otherwise we can crash with a stack overflow From 49ee57f31df5fb2c143f254a8a964bc3af68d5b9 Mon Sep 17 00:00:00 2001 From: Masatoshi Kimura Date: Wed, 22 Aug 2018 02:02:56 +0900 Subject: [PATCH 23/45] Bug 1485028 - Fix warnings that are specific to clang-cl ASAN builds. r=dmajor --HG-- extra : source : f0b577cc8b920352dfe297e7ec9cca58b1838c5d --- dom/media/webrtc/moz.build | 3 --- js/xpconnect/src/moz.build | 3 --- media/webrtc/signaling/gtest/moz.build | 3 --- media/webrtc/signaling/src/media-conduit/moz.build | 3 --- media/webrtc/signaling/src/mediapipeline/moz.build | 3 --- 5 files changed, 15 deletions(-) diff --git a/dom/media/webrtc/moz.build b/dom/media/webrtc/moz.build index 71fd92f2eba6..75e4625ba8b4 100644 --- a/dom/media/webrtc/moz.build +++ b/dom/media/webrtc/moz.build @@ -88,6 +88,3 @@ if CONFIG['CC_TYPE'] in ('msvc', 'clang-cl'): # 'reinterpret_cast': conversion from 'DWORD' to 'HANDLE' of greater size ] DEFINES['__PRETTY_FUNCTION__'] = '__FUNCSIG__' - -if CONFIG['MOZ_ASAN'] and CONFIG['CC_TYPE'] == 'clang-cl': - AllowCompilerWarnings() # workaround for bug 1306642 diff --git a/js/xpconnect/src/moz.build b/js/xpconnect/src/moz.build index f4b2bec0fea1..7a03e1944400 100644 --- a/js/xpconnect/src/moz.build +++ b/js/xpconnect/src/moz.build @@ -64,6 +64,3 @@ if CONFIG['CC_TYPE'] in ('clang', 'gcc'): if CONFIG['NIGHTLY_BUILD']: DEFINES['ENABLE_WASM_GC'] = True - -if CONFIG['MOZ_ASAN'] and CONFIG['CC_TYPE'] == 'clang-cl': - AllowCompilerWarnings() # workaround for bug 1090497 diff --git a/media/webrtc/signaling/gtest/moz.build b/media/webrtc/signaling/gtest/moz.build index 6a4bd44eea81..85316c6b6473 100644 --- a/media/webrtc/signaling/gtest/moz.build +++ b/media/webrtc/signaling/gtest/moz.build @@ -48,6 +48,3 @@ if CONFIG['CC_TYPE'] in ('msvc', 'clang-cl'): # Disable warning for decorated name length exceeded, name was truncated CXXFLAGS += ['-wd4503'] - -if CONFIG['MOZ_ASAN'] and CONFIG['CC_TYPE'] == 'clang-cl': - AllowCompilerWarnings() # workaround for bug 1306642 diff --git a/media/webrtc/signaling/src/media-conduit/moz.build b/media/webrtc/signaling/src/media-conduit/moz.build index 0ee45385f806..ef663638b2d9 100644 --- a/media/webrtc/signaling/src/media-conduit/moz.build +++ b/media/webrtc/signaling/src/media-conduit/moz.build @@ -35,6 +35,3 @@ if CONFIG['OS_TARGET'] == 'Android': ] FINAL_LIBRARY = 'xul' - -if CONFIG['MOZ_ASAN'] and CONFIG['CC_TYPE'] == 'clang-cl': - AllowCompilerWarnings() # workaround for bug 1306642 diff --git a/media/webrtc/signaling/src/mediapipeline/moz.build b/media/webrtc/signaling/src/mediapipeline/moz.build index 2a3c0b21e6ad..b4c41d70a796 100644 --- a/media/webrtc/signaling/src/mediapipeline/moz.build +++ b/media/webrtc/signaling/src/mediapipeline/moz.build @@ -29,6 +29,3 @@ UNIFIED_SOURCES += [ DEFINES['TRACING'] = True FINAL_LIBRARY = 'xul' - -if CONFIG['MOZ_ASAN'] and CONFIG['CC_TYPE'] == 'clang-cl': - AllowCompilerWarnings() # workaround for bug 1306642 From f58b403d168e357c547524524a931551876f6f25 Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Wed, 22 Aug 2018 19:54:41 +0300 Subject: [PATCH 24/45] Bug 1466875 - Re-enable GC/CC idle time telemetry, r=chutten --- toolkit/components/telemetry/Histograms.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 63c42fe6036a..cca6d9be6425 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -469,7 +469,7 @@ "CYCLE_COLLECTOR_SLICE_DURING_IDLE": { "record_in_processes": ["main", "content"], "alert_emails": ["dev-telemetry-gc-alerts@mozilla.org"], - "expires_in_version": "62", + "expires_in_version": "never", "kind": "linear", "high": 100, "n_buckets": 50, @@ -521,7 +521,7 @@ "FORGET_SKIPPABLE_DURING_IDLE": { "record_in_processes": ["main", "content"], "alert_emails": ["dev-telemetry-gc-alerts@mozilla.org"], - "expires_in_version": "62", + "expires_in_version": "never", "kind": "linear", "high": 100, "n_buckets": 50, @@ -787,7 +787,7 @@ "GC_SLICE_DURING_IDLE": { "record_in_processes": ["main", "content"], "alert_emails": ["dev-telemetry-gc-alerts@mozilla.org"], - "expires_in_version": "62", + "expires_in_version": "never", "kind": "linear", "high": 100, "n_buckets": 50, From 5fabda3acff288ebf8001fe695df7e147d8e3783 Mon Sep 17 00:00:00 2001 From: Gabriel Luong Date: Wed, 22 Aug 2018 13:18:46 -0400 Subject: [PATCH 25/45] Bug 1485118 - Format the React and Redux components in RDM. r=rcaliman --- .../client/responsive.html/components/App.js | 91 +++++---- .../responsive.html/components/Browser.js | 38 ++-- .../responsive.html/components/DeviceAdder.js | 180 +++++++----------- .../responsive.html/components/DeviceModal.js | 155 +++++++-------- .../components/DevicePixelRatioMenu.js | 2 +- .../components/DeviceSelector.js | 8 +- .../components/ResizableViewport.js | 6 +- .../components/SettingsMenu.js | 4 +- .../components/ViewportDimension.js | 2 +- .../responsive.html/components/Viewports.js | 14 +- 10 files changed, 227 insertions(+), 273 deletions(-) diff --git a/devtools/client/responsive.html/components/App.js b/devtools/client/responsive.html/components/App.js index 98df6862c3df..e25a26fbef61 100644 --- a/devtools/client/responsive.html/components/App.js +++ b/devtools/client/responsive.html/components/App.js @@ -17,6 +17,7 @@ const Viewports = createFactory(require("./Viewports")); loader.lazyGetter(this, "DeviceModal", () => createFactory(require("./DeviceModal"))); +const { changeNetworkThrottling } = require("devtools/client/shared/components/throttling/actions"); const { addCustomDevice, removeCustomDevice, @@ -24,7 +25,6 @@ const { updateDeviceModal, updatePreferredDevices, } = require("../actions/devices"); -const { changeNetworkThrottling } = require("devtools/client/shared/components/throttling/actions"); const { changeReloadCondition } = require("../actions/reload-conditions"); const { takeScreenshot } = require("../actions/screenshot"); const { changeTouchSimulation } = require("../actions/touch-simulation"); @@ -222,53 +222,52 @@ class App extends Component { deviceAdderViewportTemplate = viewports[devices.modalOpenedFromViewport]; } - return dom.div( - { - id: "app", - }, - Toolbar({ - devices, - displayPixelRatio, - networkThrottling, - reloadConditions, - screenshot, - selectedDevice, - selectedPixelRatio, - touchSimulation, - viewport: viewports[0], - onChangeDevice, - onChangeNetworkThrottling, - onChangePixelRatio, - onChangeReloadCondition, - onChangeTouchSimulation, - onExit, - onRemoveDeviceAssociation, - onResizeViewport, - onRotateViewport, - onScreenshot, - onToggleLeftAlignment, - onUpdateDeviceModal, - }), - Viewports({ - screenshot, - viewports, - onBrowserMounted, - onContentResize, - onRemoveDeviceAssociation, - onResizeViewport, - }), - devices.isModalOpen ? - DeviceModal({ - deviceAdderViewportTemplate, + return ( + dom.div({ id: "app" }, + Toolbar({ devices, - onAddCustomDevice, - onDeviceListUpdate, - onRemoveCustomDevice, - onUpdateDeviceDisplayed, + displayPixelRatio, + networkThrottling, + reloadConditions, + screenshot, + selectedDevice, + selectedPixelRatio, + touchSimulation, + viewport: viewports[0], + onChangeDevice, + onChangeNetworkThrottling, + onChangePixelRatio, + onChangeReloadCondition, + onChangeTouchSimulation, + onExit, + onRemoveDeviceAssociation, + onResizeViewport, + onRotateViewport, + onScreenshot, + onToggleLeftAlignment, onUpdateDeviceModal, - }) - : - null + }), + Viewports({ + screenshot, + viewports, + onBrowserMounted, + onContentResize, + onRemoveDeviceAssociation, + onResizeViewport, + }), + devices.isModalOpen ? + DeviceModal({ + deviceAdderViewportTemplate, + devices, + onAddCustomDevice, + onDeviceListUpdate, + onRemoveCustomDevice, + onUpdateDeviceDisplayed, + onUpdateDeviceModal, + }) + : + null + ) ); } } diff --git a/devtools/client/responsive.html/components/Browser.js b/devtools/client/responsive.html/components/Browser.js index 2725d97d0b09..28dc34472d27 100644 --- a/devtools/client/responsive.html/components/Browser.js +++ b/devtools/client/responsive.html/components/Browser.js @@ -26,10 +26,10 @@ class Browser extends PureComponent { */ static get propTypes() { return { - swapAfterMount: PropTypes.bool.isRequired, - userContextId: PropTypes.number.isRequired, onBrowserMounted: PropTypes.func.isRequired, onContentResize: PropTypes.func.isRequired, + swapAfterMount: PropTypes.bool.isRequired, + userContextId: PropTypes.number.isRequired, }; } @@ -149,22 +149,24 @@ class Browser extends PureComponent { // @noisolation and @allowfullscreen are needed so that these frames have the same // access to browser features as regular browser tabs. The `swapFrameLoaders` platform // API we use compares such features before allowing the swap to proceed. - return dom.iframe( - { - allowFullScreen: "true", - className: "browser", - height: "100%", - mozbrowser: "true", - noisolation: "true", - remote: "true", - remotetype: "web", - src: "about:blank", - usercontextid: userContextId, - width: "100%", - ref: browser => { - this.browser = browser; - }, - } + return ( + dom.iframe( + { + allowFullScreen: "true", + className: "browser", + height: "100%", + mozbrowser: "true", + noisolation: "true", + remote: "true", + remotetype: "web", + src: "about:blank", + usercontextid: userContextId, + width: "100%", + ref: browser => { + this.browser = browser; + }, + } + ) ); } } diff --git a/devtools/client/responsive.html/components/DeviceAdder.js b/devtools/client/responsive.html/components/DeviceAdder.js index 98c8b9eb8e7e..444fb690f9d4 100644 --- a/devtools/client/responsive.html/components/DeviceAdder.js +++ b/devtools/client/responsive.html/components/DeviceAdder.js @@ -10,7 +10,7 @@ const { createFactory, PureComponent } = require("devtools/client/shared/vendor/ const dom = require("devtools/client/shared/vendor/react-dom-factories"); const PropTypes = require("devtools/client/shared/vendor/react-prop-types"); -const ViewportDimension = createFactory(require("./ViewportDimension.js")); +const ViewportDimension = createFactory(require("./ViewportDimension")); const { getFormatStr, getStr } = require("../utils/l10n"); const Types = require("../types"); @@ -19,8 +19,8 @@ class DeviceAdder extends PureComponent { static get propTypes() { return { devices: PropTypes.shape(Types.devices).isRequired, - viewportTemplate: PropTypes.shape(Types.viewport).isRequired, onAddCustomDevice: PropTypes.func.isRequired, + viewportTemplate: PropTypes.shape(Types.viewport).isRequired, }; } @@ -136,121 +136,81 @@ class DeviceAdder extends PureComponent { }); } - return dom.div( - { - id: "device-adder" - }, - dom.div( - { - id: "device-adder-content", - }, - dom.div( - { - id: "device-adder-column-1", - }, - dom.label( - { - id: "device-adder-name", - }, - dom.span( - { - className: "device-adder-label", - }, - getStr("responsive.deviceAdderName") + return ( + dom.div({ id: "device-adder" }, + dom.div({ id: "device-adder-content" }, + dom.div({ id: "device-adder-column-1" }, + dom.label({ id: "device-adder-name" }, + dom.span({ className: "device-adder-label" }, + getStr("responsive.deviceAdderName") + ), + dom.input({ + defaultValue: deviceName, + ref: input => { + this.nameInput = input; + }, + }) ), - dom.input({ - defaultValue: deviceName, - ref: input => { - this.nameInput = input; - }, - }) + dom.label({ id: "device-adder-size" }, + dom.span({ className: "device-adder-label" }, + getStr("responsive.deviceAdderSize") + ), + ViewportDimension({ + viewport: { + width, + height, + }, + onResizeViewport: this.onChangeSize, + onRemoveDeviceAssociation: () => {}, + }) + ), + dom.label({ id: "device-adder-pixel-ratio" }, + dom.span({ className: "device-adder-label" }, + getStr("responsive.deviceAdderPixelRatio") + ), + dom.input({ + type: "number", + step: "any", + defaultValue: normalizedViewport.pixelRatio, + ref: input => { + this.pixelRatioInput = input; + }, + }) + ) ), - dom.label( - { - id: "device-adder-size", - }, - dom.span( - { - className: "device-adder-label" - }, - getStr("responsive.deviceAdderSize") + dom.div({ id: "device-adder-column-2" }, + dom.label({ id: "device-adder-user-agent" }, + dom.span({ className: "device-adder-label" }, + getStr("responsive.deviceAdderUserAgent") + ), + dom.input({ + defaultValue: normalizedViewport.userAgent, + ref: input => { + this.userAgentInput = input; + }, + }) ), - ViewportDimension({ - viewport: { - width, - height, - }, - onResizeViewport: this.onChangeSize, - onRemoveDeviceAssociation: () => {}, - }) + dom.label({ id: "device-adder-touch" }, + dom.span({ className: "device-adder-label" }, + getStr("responsive.deviceAdderTouch") + ), + dom.input({ + defaultChecked: normalizedViewport.touch, + type: "checkbox", + ref: input => { + this.touchInput = input; + }, + }) + ) ), - dom.label( - { - id: "device-adder-pixel-ratio", - }, - dom.span( - { - className: "device-adder-label" - }, - getStr("responsive.deviceAdderPixelRatio") - ), - dom.input({ - type: "number", - step: "any", - defaultValue: normalizedViewport.pixelRatio, - ref: input => { - this.pixelRatioInput = input; - }, - }) - ) ), - dom.div( + dom.button( { - id: "device-adder-column-2", + id: "device-adder-save", + onClick: this.onDeviceAdderSave, }, - dom.label( - { - id: "device-adder-user-agent", - }, - dom.span( - { - className: "device-adder-label" - }, - getStr("responsive.deviceAdderUserAgent") - ), - dom.input({ - defaultValue: normalizedViewport.userAgent, - ref: input => { - this.userAgentInput = input; - }, - }) - ), - dom.label( - { - id: "device-adder-touch", - }, - dom.span( - { - className: "device-adder-label" - }, - getStr("responsive.deviceAdderTouch") - ), - dom.input({ - defaultChecked: normalizedViewport.touch, - type: "checkbox", - ref: input => { - this.touchInput = input; - }, - }) - ) - ), - ), - dom.button( - { - id: "device-adder-save", - onClick: this.onDeviceAdderSave, - }, - getStr("responsive.deviceAdderSave") + getStr("responsive.deviceAdderSave") + ) ) ); } diff --git a/devtools/client/responsive.html/components/DeviceModal.js b/devtools/client/responsive.html/components/DeviceModal.js index d4ffd2140330..af719da48aa7 100644 --- a/devtools/client/responsive.html/components/DeviceModal.js +++ b/devtools/client/responsive.html/components/DeviceModal.js @@ -133,93 +133,86 @@ class DeviceModal extends PureComponent { .sort((a, b) => a.name.localeCompare(b.name)); } - return dom.div( - { - id: "device-modal-wrapper", - className: this.props.devices.isModalOpen ? "opened" : "closed", - }, + return ( dom.div( { - className: "device-modal", + id: "device-modal-wrapper", + className: this.props.devices.isModalOpen ? "opened" : "closed", }, - dom.button({ - id: "device-close-button", - className: "devtools-button", - onClick: () => onUpdateDeviceModal(false), - }), + dom.div({ className: "device-modal" }, + dom.button({ + id: "device-close-button", + className: "devtools-button", + onClick: () => onUpdateDeviceModal(false), + }), + dom.div({ className: "device-modal-content" }, + devices.types.map(type => { + return dom.div( + { + className: `device-type device-type-${type}`, + key: type, + }, + dom.header({ className: "device-header" }, + type + ), + sortedDevices[type].map(device => { + const details = getFormatStr( + "responsive.deviceDetails", device.width, device.height, + device.pixelRatio, device.userAgent, device.touch + ); + + let removeDeviceButton; + if (type == "custom") { + removeDeviceButton = dom.button({ + className: "device-remove-button devtools-button", + onClick: () => onRemoveCustomDevice(device), + }); + } + + return dom.label( + { + className: "device-label", + key: device.name, + title: details, + }, + dom.input({ + className: "device-input-checkbox", + type: "checkbox", + value: device.name, + checked: this.state[device.name], + onChange: this.onDeviceCheckboxChange, + }), + dom.span( + { + className: "device-name", + }, + device.name + ), + removeDeviceButton + ); + }) + ); + }) + ), + DeviceAdder({ + devices, + viewportTemplate: deviceAdderViewportTemplate, + onAddCustomDevice, + }), + dom.button( + { + id: "device-submit-button", + onClick: this.onDeviceModalSubmit, + }, + getStr("responsive.done") + ) + ), dom.div( { - className: "device-modal-content", - }, - devices.types.map(type => { - return dom.div( - { - className: `device-type device-type-${type}`, - key: type, - }, - dom.header( - { - className: "device-header", - }, - type - ), - sortedDevices[type].map(device => { - const details = getFormatStr( - "responsive.deviceDetails", device.width, device.height, - device.pixelRatio, device.userAgent, device.touch - ); - - let removeDeviceButton; - if (type == "custom") { - removeDeviceButton = dom.button({ - className: "device-remove-button devtools-button", - onClick: () => onRemoveCustomDevice(device), - }); - } - - return dom.label( - { - className: "device-label", - key: device.name, - title: details, - }, - dom.input({ - className: "device-input-checkbox", - type: "checkbox", - value: device.name, - checked: this.state[device.name], - onChange: this.onDeviceCheckboxChange, - }), - dom.span( - { - className: "device-name", - }, - device.name - ), - removeDeviceButton - ); - }) - ); - }) - ), - DeviceAdder({ - devices, - viewportTemplate: deviceAdderViewportTemplate, - onAddCustomDevice, - }), - dom.button( - { - id: "device-submit-button", - onClick: this.onDeviceModalSubmit, - }, - getStr("responsive.done") + className: "modal-overlay", + onClick: () => onUpdateDeviceModal(false), + } ) - ), - dom.div( - { - className: "modal-overlay", - onClick: () => onUpdateDeviceModal(false), - } ) ); } diff --git a/devtools/client/responsive.html/components/DevicePixelRatioMenu.js b/devtools/client/responsive.html/components/DevicePixelRatioMenu.js index 15aea7b20dbe..75fa97d553e5 100644 --- a/devtools/client/responsive.html/components/DevicePixelRatioMenu.js +++ b/devtools/client/responsive.html/components/DevicePixelRatioMenu.js @@ -22,9 +22,9 @@ class DevicePixelRatioMenu extends PureComponent { return { devices: PropTypes.shape(Types.devices).isRequired, displayPixelRatio: Types.pixelRatio.value.isRequired, + onChangePixelRatio: PropTypes.func.isRequired, selectedDevice: PropTypes.string.isRequired, selectedPixelRatio: PropTypes.shape(Types.pixelRatio).isRequired, - onChangePixelRatio: PropTypes.func.isRequired, }; } diff --git a/devtools/client/responsive.html/components/DeviceSelector.js b/devtools/client/responsive.html/components/DeviceSelector.js index e8330de42b06..d3a029437d52 100644 --- a/devtools/client/responsive.html/components/DeviceSelector.js +++ b/devtools/client/responsive.html/components/DeviceSelector.js @@ -17,11 +17,11 @@ class DeviceSelector extends PureComponent { static get propTypes() { return { devices: PropTypes.shape(Types.devices).isRequired, - selectedDevice: PropTypes.string.isRequired, - viewportId: PropTypes.number.isRequired, onChangeDevice: PropTypes.func.isRequired, onResizeViewport: PropTypes.func.isRequired, onUpdateDeviceModal: PropTypes.func.isRequired, + selectedDevice: PropTypes.string.isRequired, + viewportId: PropTypes.number.isRequired, }; } @@ -33,11 +33,11 @@ class DeviceSelector extends PureComponent { onShowDeviceMenu(event) { const { devices, - selectedDevice, - viewportId, onChangeDevice, onResizeViewport, onUpdateDeviceModal, + selectedDevice, + viewportId, } = this.props; const menuItems = []; diff --git a/devtools/client/responsive.html/components/ResizableViewport.js b/devtools/client/responsive.html/components/ResizableViewport.js index be49f62cb466..eefcf5c72206 100644 --- a/devtools/client/responsive.html/components/ResizableViewport.js +++ b/devtools/client/responsive.html/components/ResizableViewport.js @@ -22,13 +22,13 @@ class ResizableViewport extends Component { static get propTypes() { return { leftAlignmentEnabled: PropTypes.bool.isRequired, - screenshot: PropTypes.shape(Types.screenshot).isRequired, - swapAfterMount: PropTypes.bool.isRequired, - viewport: PropTypes.shape(Types.viewport).isRequired, onBrowserMounted: PropTypes.func.isRequired, onContentResize: PropTypes.func.isRequired, onRemoveDeviceAssociation: PropTypes.func.isRequired, onResizeViewport: PropTypes.func.isRequired, + screenshot: PropTypes.shape(Types.screenshot).isRequired, + swapAfterMount: PropTypes.bool.isRequired, + viewport: PropTypes.shape(Types.viewport).isRequired, }; } diff --git a/devtools/client/responsive.html/components/SettingsMenu.js b/devtools/client/responsive.html/components/SettingsMenu.js index f03b530e9122..b8a5e7840eef 100644 --- a/devtools/client/responsive.html/components/SettingsMenu.js +++ b/devtools/client/responsive.html/components/SettingsMenu.js @@ -18,9 +18,9 @@ class SettingsMenu extends PureComponent { static get propTypes() { return { leftAlignmentEnabled: PropTypes.bool.isRequired, - reloadConditions: PropTypes.shape(Types.reloadConditions).isRequired, onChangeReloadCondition: PropTypes.func.isRequired, onToggleLeftAlignment: PropTypes.func.isRequired, + reloadConditions: PropTypes.shape(Types.reloadConditions).isRequired, }; } @@ -32,9 +32,9 @@ class SettingsMenu extends PureComponent { onToggleSettingMenu(event) { const { leftAlignmentEnabled, - reloadConditions, onChangeReloadCondition, onToggleLeftAlignment, + reloadConditions, } = this.props; const menuItems = [ diff --git a/devtools/client/responsive.html/components/ViewportDimension.js b/devtools/client/responsive.html/components/ViewportDimension.js index c0f336054bb2..6283427f82ac 100644 --- a/devtools/client/responsive.html/components/ViewportDimension.js +++ b/devtools/client/responsive.html/components/ViewportDimension.js @@ -15,9 +15,9 @@ const Types = require("../types"); class ViewportDimension extends Component { static get propTypes() { return { - viewport: PropTypes.shape(Types.viewport).isRequired, onResizeViewport: PropTypes.func.isRequired, onRemoveDeviceAssociation: PropTypes.func.isRequired, + viewport: PropTypes.shape(Types.viewport).isRequired, }; } diff --git a/devtools/client/responsive.html/components/Viewports.js b/devtools/client/responsive.html/components/Viewports.js index 2b229b7a8b8d..18aca2230417 100644 --- a/devtools/client/responsive.html/components/Viewports.js +++ b/devtools/client/responsive.html/components/Viewports.js @@ -17,24 +17,24 @@ class Viewports extends Component { static get propTypes() { return { leftAlignmentEnabled: PropTypes.bool.isRequired, - screenshot: PropTypes.shape(Types.screenshot).isRequired, - viewports: PropTypes.arrayOf(PropTypes.shape(Types.viewport)).isRequired, onBrowserMounted: PropTypes.func.isRequired, onContentResize: PropTypes.func.isRequired, onRemoveDeviceAssociation: PropTypes.func.isRequired, onResizeViewport: PropTypes.func.isRequired, + screenshot: PropTypes.shape(Types.screenshot).isRequired, + viewports: PropTypes.arrayOf(PropTypes.shape(Types.viewport)).isRequired, }; } render() { const { leftAlignmentEnabled, - screenshot, - viewports, onBrowserMounted, onContentResize, onRemoveDeviceAssociation, onResizeViewport, + screenshot, + viewports, } = this.props; const viewportSize = window.getViewportSize(); @@ -68,13 +68,13 @@ class Viewports extends Component { return ResizableViewport({ key: viewport.id, leftAlignmentEnabled, - screenshot, - swapAfterMount: i == 0, - viewport, onBrowserMounted, onContentResize, onRemoveDeviceAssociation, onResizeViewport, + screenshot, + swapAfterMount: i == 0, + viewport, }); }) ) From 734d0df91bffe3b259625ee8f07f02bc700af7e0 Mon Sep 17 00:00:00 2001 From: Gabriel Luong Date: Wed, 22 Aug 2018 13:18:46 -0400 Subject: [PATCH 26/45] Bug 1485115 - Format the React and Redux components in the Box Model and Grid Inspector. r=rcaliman --- .../inspector/boxmodel/actions/box-model.js | 6 +- .../inspector/boxmodel/actions/index.js | 6 +- .../inspector/boxmodel/components/BoxModel.js | 68 +-- .../boxmodel/components/BoxModelEditable.js | 34 +- .../boxmodel/components/BoxModelInfo.js | 42 +- .../boxmodel/components/BoxModelMain.js | 443 +++++++++--------- .../boxmodel/components/BoxModelProperties.js | 15 +- .../client/inspector/grids/actions/grids.js | 6 +- .../grids/actions/highlighter-settings.js | 6 +- .../client/inspector/grids/actions/index.js | 12 +- .../client/inspector/grids/components/Grid.js | 34 +- .../grids/components/GridDisplaySettings.js | 102 ++-- .../inspector/grids/components/GridItem.js | 76 ++- .../inspector/grids/components/GridList.js | 46 +- .../inspector/grids/components/GridOutline.js | 2 +- .../client/inspector/grids/grid-inspector.js | 3 +- .../inspector/layout/components/LayoutApp.js | 12 +- devtools/client/inspector/layout/layout.js | 14 +- 18 files changed, 437 insertions(+), 490 deletions(-) diff --git a/devtools/client/inspector/boxmodel/actions/box-model.js b/devtools/client/inspector/boxmodel/actions/box-model.js index 3f5684f7c3c6..b206dc6a3eef 100644 --- a/devtools/client/inspector/boxmodel/actions/box-model.js +++ b/devtools/client/inspector/boxmodel/actions/box-model.js @@ -13,7 +13,7 @@ const { module.exports = { /** - * Update the geometry editor's enabled state. + * Updates the geometry editor's enabled state. * * @param {Boolean} enabled * Whether or not the geometry editor is enabled or not. @@ -26,7 +26,7 @@ module.exports = { }, /** - * Update the layout state with the new layout properties. + * Updates the layout state with the new layout properties. */ updateLayout(layout) { return { @@ -36,7 +36,7 @@ module.exports = { }, /** - * Update the offset parent state with the new DOM node. + * Updates the offset parent state with the new DOM node. */ updateOffsetParent(offsetParent) { return { diff --git a/devtools/client/inspector/boxmodel/actions/index.js b/devtools/client/inspector/boxmodel/actions/index.js index 31f780f84288..15b585eb119d 100644 --- a/devtools/client/inspector/boxmodel/actions/index.js +++ b/devtools/client/inspector/boxmodel/actions/index.js @@ -8,13 +8,13 @@ const { createEnum } = require("devtools/client/shared/enum"); createEnum([ - // Update the geometry editor's enabled state. + // Updates the geometry editor's enabled state. "UPDATE_GEOMETRY_EDITOR_ENABLED", - // Update the layout state with the latest layout properties. + // Updates the layout state with the latest layout properties. "UPDATE_LAYOUT", - // Update the offset parent state with the new DOM node. + // Updates the offset parent state with the new DOM node. "UPDATE_OFFSET_PARENT", ], module.exports); diff --git a/devtools/client/inspector/boxmodel/components/BoxModel.js b/devtools/client/inspector/boxmodel/components/BoxModel.js index b85f2c19a09d..59ad2f876862 100644 --- a/devtools/client/inspector/boxmodel/components/BoxModel.js +++ b/devtools/client/inspector/boxmodel/components/BoxModel.js @@ -18,13 +18,13 @@ class BoxModel extends PureComponent { static get propTypes() { return { boxModel: PropTypes.shape(Types.boxModel).isRequired, - setSelectedNode: PropTypes.func.isRequired, - showBoxModelProperties: PropTypes.bool.isRequired, onHideBoxModelHighlighter: PropTypes.func.isRequired, onShowBoxModelEditor: PropTypes.func.isRequired, onShowBoxModelHighlighter: PropTypes.func.isRequired, onShowBoxModelHighlighterForNode: PropTypes.func.isRequired, onToggleGeometryEditor: PropTypes.func.isRequired, + showBoxModelProperties: PropTypes.bool.isRequired, + setSelectedNode: PropTypes.func.isRequired, }; } @@ -44,47 +44,49 @@ class BoxModel extends PureComponent { render() { const { boxModel, - setSelectedNode, - showBoxModelProperties, onHideBoxModelHighlighter, onShowBoxModelEditor, onShowBoxModelHighlighter, onShowBoxModelHighlighterForNode, onToggleGeometryEditor, + setSelectedNode, + showBoxModelProperties, } = this.props; - return dom.div( - { - className: "boxmodel-container", - tabIndex: 0, - ref: div => { - this.boxModelContainer = div; + return ( + dom.div( + { + className: "boxmodel-container", + tabIndex: 0, + ref: div => { + this.boxModelContainer = div; + }, + onKeyDown: this.onKeyDown, }, - onKeyDown: this.onKeyDown, - }, - BoxModelMain({ - boxModel, - boxModelContainer: this.boxModelContainer, - ref: boxModelMain => { - this.boxModelMain = boxModelMain; - }, - onHideBoxModelHighlighter, - onShowBoxModelEditor, - onShowBoxModelHighlighter, - }), - BoxModelInfo({ - boxModel, - onToggleGeometryEditor, - }), - showBoxModelProperties ? - BoxModelProperties({ + BoxModelMain({ boxModel, - setSelectedNode, + boxModelContainer: this.boxModelContainer, + ref: boxModelMain => { + this.boxModelMain = boxModelMain; + }, onHideBoxModelHighlighter, - onShowBoxModelHighlighterForNode, - }) - : - null + onShowBoxModelEditor, + onShowBoxModelHighlighter, + }), + BoxModelInfo({ + boxModel, + onToggleGeometryEditor, + }), + showBoxModelProperties ? + BoxModelProperties({ + boxModel, + setSelectedNode, + onHideBoxModelHighlighter, + onShowBoxModelHighlighterForNode, + }) + : + null + ) ); } } diff --git a/devtools/client/inspector/boxmodel/components/BoxModelEditable.js b/devtools/client/inspector/boxmodel/components/BoxModelEditable.js index 93ec3082c128..c72f0ce13857 100644 --- a/devtools/client/inspector/boxmodel/components/BoxModelEditable.js +++ b/devtools/client/inspector/boxmodel/components/BoxModelEditable.js @@ -18,9 +18,9 @@ class BoxModelEditable extends PureComponent { direction: PropTypes.string, focusable: PropTypes.bool.isRequired, level: PropTypes.string, + onShowBoxModelEditor: PropTypes.func.isRequired, property: PropTypes.string.isRequired, textContent: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired, - onShowBoxModelEditor: PropTypes.func.isRequired, }; } @@ -49,23 +49,25 @@ class BoxModelEditable extends PureComponent { box !== "position" && textContent.toString().length > LONG_TEXT_ROTATE_LIMIT; - return dom.p( - { - className: `boxmodel-${box} - ${direction ? " boxmodel-" + direction : "boxmodel-" + property} - ${rotate ? " boxmodel-rotate" : ""}`, - }, - dom.span( + return ( + dom.p( { - className: "boxmodel-editable", - "data-box": box, - tabIndex: box === level && focusable ? 0 : -1, - title: property, - ref: span => { - this.boxModelEditable = span; - }, + className: `boxmodel-${box} + ${direction ? " boxmodel-" + direction : "boxmodel-" + property} + ${rotate ? " boxmodel-rotate" : ""}`, }, - textContent + dom.span( + { + className: "boxmodel-editable", + "data-box": box, + tabIndex: box === level && focusable ? 0 : -1, + title: property, + ref: span => { + this.boxModelEditable = span; + }, + }, + textContent + ) ) ); } diff --git a/devtools/client/inspector/boxmodel/components/BoxModelInfo.js b/devtools/client/inspector/boxmodel/components/BoxModelInfo.js index 65a5b794e333..806fa431d996 100644 --- a/devtools/client/inspector/boxmodel/components/BoxModelInfo.js +++ b/devtools/client/inspector/boxmodel/components/BoxModelInfo.js @@ -49,33 +49,21 @@ class BoxModelInfo extends PureComponent { buttonClass += " checked"; } - return dom.div( - { - className: "boxmodel-info", - }, - dom.span( - { - className: "boxmodel-element-size", - }, - SHARED_L10N.getFormatStr("dimensions", width, height) - ), - dom.section( - { - className: "boxmodel-position-group", - }, - isPositionEditable ? - dom.button({ - className: buttonClass, - title: BOXMODEL_L10N.getStr("boxmodel.geometryButton.tooltip"), - onClick: this.onToggleGeometryEditor, - }) - : - null, - dom.span( - { - className: "boxmodel-element-position", - }, - position + return ( + dom.div({ className: "boxmodel-info" }, + dom.span({ className: "boxmodel-element-size" }, + SHARED_L10N.getFormatStr("dimensions", width, height) + ), + dom.section({ className: "boxmodel-position-group" }, + isPositionEditable ? + dom.button({ + className: buttonClass, + title: BOXMODEL_L10N.getStr("boxmodel.geometryButton.tooltip"), + onClick: this.onToggleGeometryEditor, + }) + : + null, + dom.span({ className: "boxmodel-element-position" }, position) ) ) ); diff --git a/devtools/client/inspector/boxmodel/components/BoxModelMain.js b/devtools/client/inspector/boxmodel/components/BoxModelMain.js index 52753d0859cc..12dd5bd8df7d 100644 --- a/devtools/client/inspector/boxmodel/components/BoxModelMain.js +++ b/devtools/client/inspector/boxmodel/components/BoxModelMain.js @@ -46,10 +46,10 @@ class BoxModelMain extends PureComponent { this.getPositionValue = this.getPositionValue.bind(this); this.getWidthValue = this.getWidthValue.bind(this); this.moveFocus = this.moveFocus.bind(this); - this.setAriaActive = this.setAriaActive.bind(this); this.onHighlightMouseOver = this.onHighlightMouseOver.bind(this); this.onKeyDown = this.onKeyDown.bind(this); this.onLevelClick = this.onLevelClick.bind(this); + this.setAriaActive = this.setAriaActive.bind(this); } componentDidUpdate() { @@ -414,10 +414,7 @@ class BoxModelMain extends PureComponent { width = this.getWidthValue(width); const contentBox = layout["box-sizing"] == "content-box" ? - dom.div( - { - className: "boxmodel-size", - }, + dom.div({ className: "boxmodel-size" }, BoxModelEditable({ box: "content", focusable, @@ -429,10 +426,7 @@ class BoxModelMain extends PureComponent { textContent: width, onShowBoxModelEditor }), - dom.span( - {}, - "\u00D7" - ), + dom.span({}, "\u00D7"), BoxModelEditable({ box: "content", focusable, @@ -443,277 +437,270 @@ class BoxModelMain extends PureComponent { }) ) : - dom.p( - { - className: "boxmodel-size", - }, - dom.span( - { - title: "content", - }, + dom.p({ className: "boxmodel-size" }, + dom.span({ title: "content" }, SHARED_L10N.getFormatStr("dimensions", width, height) ) ); - return dom.div( - { - className: "boxmodel-main devtools-monospace", - "data-box": "position", - ref: div => { - this.positionLayout = div; - }, - onClick: this.onLevelClick, - onKeyDown: this.onKeyDown, - onMouseOver: this.onHighlightMouseOver, - onMouseOut: this.props.onHideBoxModelHighlighter, - }, - displayPosition ? - dom.span( - { - className: "boxmodel-legend", - "data-box": "position", - title: "position", - }, - "position" - ) - : - null, + return ( dom.div( { - className: "boxmodel-box" + className: "boxmodel-main devtools-monospace", + "data-box": "position", + ref: div => { + this.positionLayout = div; + }, + onClick: this.onLevelClick, + onKeyDown: this.onKeyDown, + onMouseOver: this.onHighlightMouseOver, + onMouseOut: this.props.onHideBoxModelHighlighter, }, - dom.span( - { - className: "boxmodel-legend", - "data-box": "margin", - title: "margin", - }, - "margin" - ), - dom.div( - { - className: "boxmodel-margins", - "data-box": "margin", - title: "margin", - ref: div => { - this.marginLayout = div; - }, - }, + displayPosition ? dom.span( { className: "boxmodel-legend", - "data-box": "border", - title: "border", + "data-box": "position", + title: "position", }, - "border" + "position" + ) + : + null, + dom.div({ className: "boxmodel-box" }, + dom.span( + { + className: "boxmodel-legend", + "data-box": "margin", + title: "margin", + }, + "margin" ), dom.div( { - className: "boxmodel-borders", - "data-box": "border", - title: "border", + className: "boxmodel-margins", + "data-box": "margin", + title: "margin", ref: div => { - this.borderLayout = div; + this.marginLayout = div; }, }, dom.span( { className: "boxmodel-legend", - "data-box": "padding", - title: "padding", + "data-box": "border", + title: "border", }, - "padding" + "border" ), dom.div( { - className: "boxmodel-paddings", - "data-box": "padding", - title: "padding", + className: "boxmodel-borders", + "data-box": "border", + title: "border", ref: div => { - this.paddingLayout = div; + this.borderLayout = div; }, }, - dom.div({ - className: "boxmodel-contents", - "data-box": "content", - title: "content", - ref: div => { - this.contentLayout = div; + dom.span( + { + className: "boxmodel-legend", + "data-box": "padding", + title: "padding", }, - }) + "padding" + ), + dom.div( + { + className: "boxmodel-paddings", + "data-box": "padding", + title: "padding", + ref: div => { + this.paddingLayout = div; + }, + }, + dom.div({ + className: "boxmodel-contents", + "data-box": "content", + title: "content", + ref: div => { + this.contentLayout = div; + }, + }) + ) ) ) - ) - ), - displayPosition ? + ), + displayPosition ? + BoxModelEditable({ + box: "position", + direction: "top", + focusable, + level, + property: "position-top", + ref: editable => { + this.positionEditable = editable; + }, + textContent: positionTop, + onShowBoxModelEditor, + }) + : + null, + displayPosition ? + BoxModelEditable({ + box: "position", + direction: "right", + focusable, + level, + property: "position-right", + textContent: positionRight, + onShowBoxModelEditor, + }) + : + null, + displayPosition ? + BoxModelEditable({ + box: "position", + direction: "bottom", + focusable, + level, + property: "position-bottom", + textContent: positionBottom, + onShowBoxModelEditor, + }) + : + null, + displayPosition ? + BoxModelEditable({ + box: "position", + direction: "left", + focusable, + level, + property: "position-left", + textContent: positionLeft, + onShowBoxModelEditor, + }) + : + null, BoxModelEditable({ - box: "position", + box: "margin", direction: "top", focusable, level, - property: "position-top", + property: "margin-top", ref: editable => { - this.positionEditable = editable; + this.marginEditable = editable; }, - textContent: positionTop, + textContent: marginTop, onShowBoxModelEditor, - }) - : - null, - displayPosition ? + }), BoxModelEditable({ - box: "position", + box: "margin", direction: "right", focusable, level, - property: "position-right", - textContent: positionRight, + property: "margin-right", + textContent: marginRight, onShowBoxModelEditor, - }) - : - null, - displayPosition ? + }), BoxModelEditable({ - box: "position", + box: "margin", direction: "bottom", focusable, level, - property: "position-bottom", - textContent: positionBottom, + property: "margin-bottom", + textContent: marginBottom, onShowBoxModelEditor, - }) - : - null, - displayPosition ? + }), BoxModelEditable({ - box: "position", + box: "margin", direction: "left", focusable, level, - property: "position-left", - textContent: positionLeft, + property: "margin-left", + textContent: marginLeft, onShowBoxModelEditor, - }) - : - null, - BoxModelEditable({ - box: "margin", - direction: "top", - focusable, - level, - property: "margin-top", - ref: editable => { - this.marginEditable = editable; - }, - textContent: marginTop, - onShowBoxModelEditor, - }), - BoxModelEditable({ - box: "margin", - direction: "right", - focusable, - level, - property: "margin-right", - textContent: marginRight, - onShowBoxModelEditor, - }), - BoxModelEditable({ - box: "margin", - direction: "bottom", - focusable, - level, - property: "margin-bottom", - textContent: marginBottom, - onShowBoxModelEditor, - }), - BoxModelEditable({ - box: "margin", - direction: "left", - focusable, - level, - property: "margin-left", - textContent: marginLeft, - onShowBoxModelEditor, - }), - BoxModelEditable({ - box: "border", - direction: "top", - focusable, - level, - property: "border-top-width", - ref: editable => { - this.borderEditable = editable; - }, - textContent: borderTop, - onShowBoxModelEditor, - }), - BoxModelEditable({ - box: "border", - direction: "right", - focusable, - level, - property: "border-right-width", - textContent: borderRight, - onShowBoxModelEditor, - }), - BoxModelEditable({ - box: "border", - direction: "bottom", - focusable, - level, - property: "border-bottom-width", - textContent: borderBottom, - onShowBoxModelEditor, - }), - BoxModelEditable({ - box: "border", - direction: "left", - focusable, - level, - property: "border-left-width", - textContent: borderLeft, - onShowBoxModelEditor, - }), - BoxModelEditable({ - box: "padding", - direction: "top", - focusable, - level, - property: "padding-top", - ref: editable => { - this.paddingEditable = editable; - }, - textContent: paddingTop, - onShowBoxModelEditor, - }), - BoxModelEditable({ - box: "padding", - direction: "right", - focusable, - level, - property: "padding-right", - textContent: paddingRight, - onShowBoxModelEditor, - }), - BoxModelEditable({ - box: "padding", - direction: "bottom", - focusable, - level, - property: "padding-bottom", - textContent: paddingBottom, - onShowBoxModelEditor, - }), - BoxModelEditable({ - box: "padding", - direction: "left", - focusable, - level, - property: "padding-left", - textContent: paddingLeft, - onShowBoxModelEditor, - }), - contentBox + }), + BoxModelEditable({ + box: "border", + direction: "top", + focusable, + level, + property: "border-top-width", + ref: editable => { + this.borderEditable = editable; + }, + textContent: borderTop, + onShowBoxModelEditor, + }), + BoxModelEditable({ + box: "border", + direction: "right", + focusable, + level, + property: "border-right-width", + textContent: borderRight, + onShowBoxModelEditor, + }), + BoxModelEditable({ + box: "border", + direction: "bottom", + focusable, + level, + property: "border-bottom-width", + textContent: borderBottom, + onShowBoxModelEditor, + }), + BoxModelEditable({ + box: "border", + direction: "left", + focusable, + level, + property: "border-left-width", + textContent: borderLeft, + onShowBoxModelEditor, + }), + BoxModelEditable({ + box: "padding", + direction: "top", + focusable, + level, + property: "padding-top", + ref: editable => { + this.paddingEditable = editable; + }, + textContent: paddingTop, + onShowBoxModelEditor, + }), + BoxModelEditable({ + box: "padding", + direction: "right", + focusable, + level, + property: "padding-right", + textContent: paddingRight, + onShowBoxModelEditor, + }), + BoxModelEditable({ + box: "padding", + direction: "bottom", + focusable, + level, + property: "padding-bottom", + textContent: paddingBottom, + onShowBoxModelEditor, + }), + BoxModelEditable({ + box: "padding", + direction: "left", + focusable, + level, + property: "padding-left", + textContent: paddingLeft, + onShowBoxModelEditor, + }), + contentBox + ) ); } } diff --git a/devtools/client/inspector/boxmodel/components/BoxModelProperties.js b/devtools/client/inspector/boxmodel/components/BoxModelProperties.js index 7a2c80af3fb0..ec4f44c623d0 100644 --- a/devtools/client/inspector/boxmodel/components/BoxModelProperties.js +++ b/devtools/client/inspector/boxmodel/components/BoxModelProperties.js @@ -20,9 +20,9 @@ class BoxModelProperties extends PureComponent { static get propTypes() { return { boxModel: PropTypes.shape(Types.boxModel).isRequired, - setSelectedNode: PropTypes.func.isRequired, onHideBoxModelHighlighter: PropTypes.func.isRequired, onShowBoxModelHighlighterForNode: PropTypes.func.isRequired, + setSelectedNode: PropTypes.func.isRequired, }; } @@ -73,9 +73,9 @@ class BoxModelProperties extends PureComponent { render() { const { boxModel, - setSelectedNode, onHideBoxModelHighlighter, onShowBoxModelHighlighterForNode, + setSelectedNode, } = this.props; const { layout } = boxModel; @@ -88,19 +88,16 @@ class BoxModelProperties extends PureComponent { return ComputedProperty({ name: info, key: info, - value: layout[info], + onHideBoxModelHighlighter, + onShowBoxModelHighlighterForNode, referenceElement, referenceElementType, setSelectedNode, - onHideBoxModelHighlighter, - onShowBoxModelHighlighterForNode, + value: layout[info], }); }); - return dom.div( - { - className: "boxmodel-properties", - }, + return dom.div({ className: "boxmodel-properties" }, dom.div( { className: "boxmodel-properties-header", diff --git a/devtools/client/inspector/grids/actions/grids.js b/devtools/client/inspector/grids/actions/grids.js index 092b9482e0cc..a36ccd2493a4 100644 --- a/devtools/client/inspector/grids/actions/grids.js +++ b/devtools/client/inspector/grids/actions/grids.js @@ -13,7 +13,7 @@ const { module.exports = { /** - * Update the color used for the grid's highlighter. + * Updates the color used for the grid's highlighter. * * @param {NodeFront} nodeFront * The NodeFront of the DOM node to toggle the grid highlighter. @@ -29,7 +29,7 @@ module.exports = { }, /** - * Update the grid highlighted state. + * Updates the grid highlighted state. * * @param {NodeFront} nodeFront * The NodeFront of the DOM node to toggle the grid highlighter. @@ -45,7 +45,7 @@ module.exports = { }, /** - * Update the grid state with the new list of grids. + * Updates the grid state with the new list of grids. */ updateGrids(grids) { return { diff --git a/devtools/client/inspector/grids/actions/highlighter-settings.js b/devtools/client/inspector/grids/actions/highlighter-settings.js index 78ece5c8fc90..a6cf288238b3 100644 --- a/devtools/client/inspector/grids/actions/highlighter-settings.js +++ b/devtools/client/inspector/grids/actions/highlighter-settings.js @@ -13,7 +13,7 @@ const { module.exports = { /** - * Update the grid highlighter's show grid areas preference. + * Updates the grid highlighter's show grid areas preference. * * @param {Boolean} enabled * Whether or not the grid highlighter should show the grid areas. @@ -26,7 +26,7 @@ module.exports = { }, /** - * Update the grid highlighter's show grid line numbers preference. + * Updates the grid highlighter's show grid line numbers preference. * * @param {Boolean} enabled * Whether or not the grid highlighter should show the grid line numbers. @@ -39,7 +39,7 @@ module.exports = { }, /** - * Update the grid highlighter's show infinite lines preference. + * Updates the grid highlighter's show infinite lines preference. * * @param {Boolean} enabled * Whether or not the grid highlighter should extend grid lines infinitely. diff --git a/devtools/client/inspector/grids/actions/index.js b/devtools/client/inspector/grids/actions/index.js index d117dfc2c0c5..ab3f8dbbe782 100644 --- a/devtools/client/inspector/grids/actions/index.js +++ b/devtools/client/inspector/grids/actions/index.js @@ -8,22 +8,22 @@ const { createEnum } = require("devtools/client/shared/enum"); createEnum([ - // Update the color used for the overlay of a grid. + // Updates the color used for the overlay of a grid. "UPDATE_GRID_COLOR", - // Update the grid highlighted state. + // Updates the grid highlighted state. "UPDATE_GRID_HIGHLIGHTED", - // Update the entire grids state with the new list of grids. + // Updates the entire grids state with the new list of grids. "UPDATE_GRIDS", - // Update the grid highlighter's show grid areas state. + // Updates the grid highlighter's show grid areas state. "UPDATE_SHOW_GRID_AREAS", - // Update the grid highlighter's show grid line numbers state. + // Updates the grid highlighter's show grid line numbers state. "UPDATE_SHOW_GRID_LINE_NUMBERS", - // Update the grid highlighter's show infinite lines state. + // Updates the grid highlighter's show infinite lines state. "UPDATE_SHOW_INFINITE_LINES", ], module.exports); diff --git a/devtools/client/inspector/grids/components/Grid.js b/devtools/client/inspector/grids/components/Grid.js index c143c8ce825f..fd08af3168fd 100644 --- a/devtools/client/inspector/grids/components/Grid.js +++ b/devtools/client/inspector/grids/components/Grid.js @@ -21,7 +21,6 @@ class Grid extends PureComponent { getSwatchColorPickerTooltip: PropTypes.func.isRequired, grids: PropTypes.arrayOf(PropTypes.shape(Types.grid)).isRequired, highlighterSettings: PropTypes.shape(Types.highlighterSettings).isRequired, - setSelectedNode: PropTypes.func.isRequired, onHideBoxModelHighlighter: PropTypes.func.isRequired, onSetGridOverlayColor: PropTypes.func.isRequired, onShowBoxModelHighlighterForNode: PropTypes.func.isRequired, @@ -30,6 +29,7 @@ class Grid extends PureComponent { onToggleShowGridAreas: PropTypes.func.isRequired, onToggleShowGridLineNumbers: PropTypes.func.isRequired, onToggleShowInfiniteLines: PropTypes.func.isRequired, + setSelectedNode: PropTypes.func.isRequired, }; } @@ -38,7 +38,6 @@ class Grid extends PureComponent { getSwatchColorPickerTooltip, grids, highlighterSettings, - setSelectedNode, onHideBoxModelHighlighter, onSetGridOverlayColor, onShowBoxModelHighlighterForNode, @@ -47,25 +46,28 @@ class Grid extends PureComponent { onToggleGridHighlighter, onToggleShowGridLineNumbers, onToggleShowInfiniteLines, + setSelectedNode, } = this.props; - return grids.length ? - dom.div( - { - id: "layout-grid-container", - }, - dom.div( - { - className: "grid-content", - }, + if (!grids.length) { + return ( + dom.div({ className: "devtools-sidepanel-no-result" }, + getStr("layout.noGridsOnThisPage") + ) + ); + } + + return ( + dom.div({ id: "layout-grid-container" }, + dom.div({ className: "grid-content" }, GridList({ getSwatchColorPickerTooltip, grids, - setSelectedNode, onHideBoxModelHighlighter, onSetGridOverlayColor, onShowBoxModelHighlighterForNode, onToggleGridHighlighter, + setSelectedNode, }), GridDisplaySettings({ highlighterSettings, @@ -79,13 +81,7 @@ class Grid extends PureComponent { onShowGridOutlineHighlight, }) ) - : - dom.div( - { - className: "devtools-sidepanel-no-result", - }, - getStr("layout.noGridsOnThisPage") - ); + ); } } diff --git a/devtools/client/inspector/grids/components/GridDisplaySettings.js b/devtools/client/inspector/grids/components/GridDisplaySettings.js index 9f338a2fb08e..f647a9486f89 100644 --- a/devtools/client/inspector/grids/components/GridDisplaySettings.js +++ b/devtools/client/inspector/grids/components/GridDisplaySettings.js @@ -23,6 +23,7 @@ class GridDisplaySettings extends PureComponent { constructor(props) { super(props); + this.onShowGridAreasCheckboxClick = this.onShowGridAreasCheckboxClick.bind(this); this.onShowGridLineNumbersCheckboxClick = this.onShowGridLineNumbersCheckboxClick.bind(this); @@ -62,65 +63,48 @@ class GridDisplaySettings extends PureComponent { highlighterSettings, } = this.props; - return dom.div( - { - className: "grid-container", - }, - dom.span( - {}, - getStr("layout.gridDisplaySettings") - ), - dom.ul( - {}, - dom.li( - { - className: "grid-settings-item", - }, - dom.label( - {}, - dom.input( - { - id: "grid-setting-show-grid-line-numbers", - type: "checkbox", - checked: highlighterSettings.showGridLineNumbers, - onChange: this.onShowGridLineNumbersCheckboxClick, - } - ), - getStr("layout.displayLineNumbers") - ) - ), - dom.li( - { - className: "grid-settings-item", - }, - dom.label( - {}, - dom.input( - { - id: "grid-setting-show-grid-areas", - type: "checkbox", - checked: highlighterSettings.showGridAreasOverlay, - onChange: this.onShowGridAreasCheckboxClick, - } - ), - getStr("layout.displayAreaNames") - ) - ), - dom.li( - { - className: "grid-settings-item", - }, - dom.label( - {}, - dom.input( - { - id: "grid-setting-extend-grid-lines", - type: "checkbox", - checked: highlighterSettings.showInfiniteLines, - onChange: this.onShowInfiniteLinesCheckboxClick, - } - ), - getStr("layout.extendLinesInfinitely") + return ( + dom.div({ className: "grid-container" }, + dom.span({}, getStr("layout.gridDisplaySettings")), + dom.ul({}, + dom.li({ className: "grid-settings-item" }, + dom.label({}, + dom.input( + { + id: "grid-setting-show-grid-line-numbers", + type: "checkbox", + checked: highlighterSettings.showGridLineNumbers, + onChange: this.onShowGridLineNumbersCheckboxClick, + } + ), + getStr("layout.displayLineNumbers") + ) + ), + dom.li({ className: "grid-settings-item" }, + dom.label({}, + dom.input( + { + id: "grid-setting-show-grid-areas", + type: "checkbox", + checked: highlighterSettings.showGridAreasOverlay, + onChange: this.onShowGridAreasCheckboxClick, + } + ), + getStr("layout.displayAreaNames") + ) + ), + dom.li({ className: "grid-settings-item" }, + dom.label({}, + dom.input( + { + id: "grid-setting-extend-grid-lines", + type: "checkbox", + checked: highlighterSettings.showInfiniteLines, + onChange: this.onShowInfiniteLinesCheckboxClick, + } + ), + getStr("layout.extendLinesInfinitely") + ) ) ) ) diff --git a/devtools/client/inspector/grids/components/GridItem.js b/devtools/client/inspector/grids/components/GridItem.js index c62fd3b56014..0a1b6dba3ee9 100644 --- a/devtools/client/inspector/grids/components/GridItem.js +++ b/devtools/client/inspector/grids/components/GridItem.js @@ -22,19 +22,20 @@ class GridItem extends PureComponent { return { getSwatchColorPickerTooltip: PropTypes.func.isRequired, grid: PropTypes.shape(Types.grid).isRequired, - setSelectedNode: PropTypes.func.isRequired, onHideBoxModelHighlighter: PropTypes.func.isRequired, onSetGridOverlayColor: PropTypes.func.isRequired, onShowBoxModelHighlighterForNode: PropTypes.func.isRequired, onToggleGridHighlighter: PropTypes.func.isRequired, + setSelectedNode: PropTypes.func.isRequired, }; } constructor(props) { super(props); - this.setGridColor = this.setGridColor.bind(this); + this.onGridCheckboxClick = this.onGridCheckboxClick.bind(this); this.onGridInspectIconClick = this.onGridInspectIconClick.bind(this); + this.setGridColor = this.setGridColor.bind(this); } componentDidMount() { @@ -97,47 +98,42 @@ class GridItem extends PureComponent { } = this.props; const { nodeFront } = grid; - return dom.li( - {}, - dom.label( - {}, - dom.input( + return ( + dom.li({}, + dom.label({}, + dom.input( + { + checked: grid.highlighted, + type: "checkbox", + value: grid.id, + onChange: this.onGridCheckboxClick, + } + ), + Rep( + { + defaultRep: ElementNode, + mode: MODE.TINY, + object: translateNodeFrontToGrip(nodeFront), + onDOMNodeMouseOut: () => onHideBoxModelHighlighter(), + onDOMNodeMouseOver: () => onShowBoxModelHighlighterForNode(nodeFront), + onInspectIconClick: () => this.onGridInspectIconClick(nodeFront), + } + ) + ), + dom.div( { - checked: grid.highlighted, - type: "checkbox", - value: grid.id, - onChange: this.onGridCheckboxClick, + className: "grid-color-swatch", + style: { + backgroundColor: grid.color, + }, + title: grid.color, } ), - Rep( - { - defaultRep: ElementNode, - mode: MODE.TINY, - object: translateNodeFrontToGrip(nodeFront), - onDOMNodeMouseOut: () => onHideBoxModelHighlighter(), - onDOMNodeMouseOver: () => onShowBoxModelHighlighterForNode(nodeFront), - onInspectIconClick: () => this.onGridInspectIconClick(nodeFront), - } - ) - ), - dom.div( - { - className: "grid-color-swatch", - style: { - backgroundColor: grid.color, - }, - title: grid.color, - } - ), - // The SwatchColorPicker relies on the nextSibling of the swatch element to apply - // the selected color. This is why we use a span in display: none for now. - // Ideally we should modify the SwatchColorPickerTooltip to bypass this requirement. - // See https://bugzilla.mozilla.org/show_bug.cgi?id=1341578 - dom.span( - { - className: "grid-color-value" - }, - grid.color + // The SwatchColorPicker relies on the nextSibling of the swatch element to apply + // the selected color. This is why we use a span in display: none for now. + // Ideally we should modify the SwatchColorPickerTooltip to bypass this + // requirement. See https://bugzilla.mozilla.org/show_bug.cgi?id=1341578 + dom.span({ className: "grid-color-value" }, grid.color) ) ); } diff --git a/devtools/client/inspector/grids/components/GridList.js b/devtools/client/inspector/grids/components/GridList.js index 627b70f6455f..f39b62ce8d24 100644 --- a/devtools/client/inspector/grids/components/GridList.js +++ b/devtools/client/inspector/grids/components/GridList.js @@ -18,11 +18,11 @@ class GridList extends PureComponent { return { getSwatchColorPickerTooltip: PropTypes.func.isRequired, grids: PropTypes.arrayOf(PropTypes.shape(Types.grid)).isRequired, - setSelectedNode: PropTypes.func.isRequired, onHideBoxModelHighlighter: PropTypes.func.isRequired, onSetGridOverlayColor: PropTypes.func.isRequired, onShowBoxModelHighlighterForNode: PropTypes.func.isRequired, onToggleGridHighlighter: PropTypes.func.isRequired, + setSelectedNode: PropTypes.func.isRequired, }; } @@ -30,36 +30,32 @@ class GridList extends PureComponent { const { getSwatchColorPickerTooltip, grids, - setSelectedNode, onHideBoxModelHighlighter, onSetGridOverlayColor, onShowBoxModelHighlighterForNode, onToggleGridHighlighter, + setSelectedNode, } = this.props; - return dom.div( - { - className: "grid-container", - }, - dom.span( - {}, - getStr("layout.overlayGrid") - ), - dom.ul( - { - id: "grid-list", - className: "devtools-monospace", - }, - grids.map(grid => GridItem({ - key: grid.id, - getSwatchColorPickerTooltip, - grid, - setSelectedNode, - onHideBoxModelHighlighter, - onSetGridOverlayColor, - onShowBoxModelHighlighterForNode, - onToggleGridHighlighter, - })) + return ( + dom.div({ className: "grid-container" }, + dom.span({}, getStr("layout.overlayGrid")), + dom.ul( + { + id: "grid-list", + className: "devtools-monospace", + }, + grids.map(grid => GridItem({ + key: grid.id, + getSwatchColorPickerTooltip, + grid, + onHideBoxModelHighlighter, + onSetGridOverlayColor, + onShowBoxModelHighlighterForNode, + onToggleGridHighlighter, + setSelectedNode, + })) + ) ) ); } diff --git a/devtools/client/inspector/grids/components/GridOutline.js b/devtools/client/inspector/grids/components/GridOutline.js index 0cbf58f4a82a..1ee54ad7055b 100644 --- a/devtools/client/inspector/grids/components/GridOutline.js +++ b/devtools/client/inspector/grids/components/GridOutline.js @@ -58,13 +58,13 @@ class GridOutline extends PureComponent { this.getGridAreaName = this.getGridAreaName.bind(this); this.getHeight = this.getHeight.bind(this); this.getTotalWidthAndHeight = this.getTotalWidthAndHeight.bind(this); + this.onHighlightCell = this.onHighlightCell.bind(this); this.renderCannotShowOutlineText = this.renderCannotShowOutlineText.bind(this); this.renderGrid = this.renderGrid.bind(this); this.renderGridCell = this.renderGridCell.bind(this); this.renderGridOutline = this.renderGridOutline.bind(this); this.renderGridOutlineBorder = this.renderGridOutlineBorder.bind(this); this.renderOutline = this.renderOutline.bind(this); - this.onHighlightCell = this.onHighlightCell.bind(this); } componentWillReceiveProps({ grids }) { diff --git a/devtools/client/inspector/grids/grid-inspector.js b/devtools/client/inspector/grids/grid-inspector.js index 30e3ca2fe54c..23bb5152f144 100644 --- a/devtools/client/inspector/grids/grid-inspector.js +++ b/devtools/client/inspector/grids/grid-inspector.js @@ -56,8 +56,6 @@ class GridInspector { this.telemetry = inspector.telemetry; this.walker = this.inspector.walker; - this.updateGridPanel = this.updateGridPanel.bind(this); - this.onHighlighterShown = this.onHighlighterShown.bind(this); this.onHighlighterHidden = this.onHighlighterHidden.bind(this); this.onNavigate = this.onNavigate.bind(this); @@ -69,6 +67,7 @@ class GridInspector { this.onToggleShowGridAreas = this.onToggleShowGridAreas.bind(this); this.onToggleShowGridLineNumbers = this.onToggleShowGridLineNumbers.bind(this); this.onToggleShowInfiniteLines = this.onToggleShowInfiniteLines.bind(this); + this.updateGridPanel = this.updateGridPanel.bind(this); this.init(); } diff --git a/devtools/client/inspector/layout/components/LayoutApp.js b/devtools/client/inspector/layout/components/LayoutApp.js index eb45a9199f2b..8f391fc33844 100644 --- a/devtools/client/inspector/layout/components/LayoutApp.js +++ b/devtools/client/inspector/layout/components/LayoutApp.js @@ -27,7 +27,6 @@ const LAYOUT_STRINGS_URI = "devtools/client/locales/layout.properties"; const LAYOUT_L10N = new LocalizationHelper(LAYOUT_STRINGS_URI); const FLEXBOX_ENABLED_PREF = "devtools.flexboxinspector.enabled"; - const BOXMODEL_OPENED_PREF = "devtools.layout.boxmodel.opened"; const FLEXBOX_OPENED_PREF = "devtools.layout.flexbox.opened"; const GRID_OPENED_PREF = "devtools.layout.grid.opened"; @@ -39,8 +38,6 @@ class LayoutApp extends PureComponent { getSwatchColorPickerTooltip: PropTypes.func.isRequired, grids: PropTypes.arrayOf(PropTypes.shape(GridTypes.grid)).isRequired, highlighterSettings: PropTypes.shape(GridTypes.highlighterSettings).isRequired, - setSelectedNode: PropTypes.func.isRequired, - showBoxModelProperties: PropTypes.bool.isRequired, onHideBoxModelHighlighter: PropTypes.func.isRequired, onSetFlexboxOverlayColor: PropTypes.func.isRequired, onSetGridOverlayColor: PropTypes.func.isRequired, @@ -50,6 +47,8 @@ class LayoutApp extends PureComponent { onToggleGridHighlighter: PropTypes.func.isRequired, onToggleShowGridLineNumbers: PropTypes.func.isRequired, onToggleShowInfiniteLines: PropTypes.func.isRequired, + setSelectedNode: PropTypes.func.isRequired, + showBoxModelProperties: PropTypes.bool.isRequired, }; } @@ -93,9 +92,10 @@ class LayoutApp extends PureComponent { ]; } - return dom.div( - { id: "layout-container" }, - Accordion({ items }) + return ( + dom.div({ id: "layout-container" }, + Accordion({ items }) + ) ); } } diff --git a/devtools/client/inspector/layout/layout.js b/devtools/client/inspector/layout/layout.js index e4f503711672..6aafa236780b 100644 --- a/devtools/client/inspector/layout/layout.js +++ b/devtools/client/inspector/layout/layout.js @@ -34,8 +34,8 @@ class LayoutView { } const { - setSelectedNode, onShowBoxModelHighlighterForNode, + setSelectedNode, } = this.inspector.getCommonComponentProps(); const { @@ -63,12 +63,6 @@ class LayoutView { const layoutApp = LayoutApp({ getSwatchColorPickerTooltip: this.getSwatchColorPickerTooltip, - setSelectedNode, - /** - * Shows the box model properties under the box model if true, otherwise, hidden by - * default. - */ - showBoxModelProperties: true, onHideBoxModelHighlighter, onSetFlexboxOverlayColor, onSetGridOverlayColor, @@ -82,6 +76,12 @@ class LayoutView { onToggleShowGridAreas, onToggleShowGridLineNumbers, onToggleShowInfiniteLines, + setSelectedNode, + /** + * Shows the box model properties under the box model if true, otherwise, hidden by + * default. + */ + showBoxModelProperties: true, }); const provider = createElement(Provider, { From 3788f265e335b542a82a62331ede4cd3166517ff Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 21 Aug 2018 17:59:49 -0400 Subject: [PATCH 27/45] Bug 1485182 - Part 1: Run http-on-modify-request observers when tracking protection cancels an HTTP channel; r=mayhemer --- netwerk/base/nsChannelClassifier.cpp | 7 +- netwerk/protocol/http/HttpBaseChannel.cpp | 6 ++ netwerk/protocol/http/HttpBaseChannel.h | 1 + netwerk/protocol/http/nsHttpChannel.cpp | 86 +++++++++++++++++++ netwerk/protocol/http/nsHttpChannel.h | 4 + .../protocol/http/nsIHttpChannelInternal.idl | 8 ++ 6 files changed, 111 insertions(+), 1 deletion(-) diff --git a/netwerk/base/nsChannelClassifier.cpp b/netwerk/base/nsChannelClassifier.cpp index 9d40fc69893f..b2a968c00f1e 100644 --- a/netwerk/base/nsChannelClassifier.cpp +++ b/netwerk/base/nsChannelClassifier.cpp @@ -1187,7 +1187,12 @@ TrackingURICallback::OnTrackerFound(nsresult aErrorCode) mList, mProvider, mFullHash); LOG(("TrackingURICallback[%p]::OnTrackerFound, cancelling channel[%p]", mChannelClassifier.get(), channel.get())); - channel->Cancel(aErrorCode); + nsCOMPtr httpChannel = do_QueryInterface(channel); + if (httpChannel) { + Unused << httpChannel->CancelForTrackingProtection(); + } else { + Unused << channel->Cancel(aErrorCode); + } } else { MOZ_ASSERT(aErrorCode == NS_ERROR_TRACKING_ANNOTATION_URI); MOZ_ASSERT(mChannelClassifier->ShouldEnableTrackingAnnotation()); diff --git a/netwerk/protocol/http/HttpBaseChannel.cpp b/netwerk/protocol/http/HttpBaseChannel.cpp index 099d10398a19..e9354b4aafc2 100644 --- a/netwerk/protocol/http/HttpBaseChannel.cpp +++ b/netwerk/protocol/http/HttpBaseChannel.cpp @@ -4724,5 +4724,11 @@ HttpBaseChannel::GetNativeServerTiming(nsTArray>& aSer return NS_OK; } +NS_IMETHODIMP +HttpBaseChannel::CancelForTrackingProtection() +{ + return Cancel(NS_ERROR_TRACKING_URI); +} + } // namespace net } // namespace mozilla diff --git a/netwerk/protocol/http/HttpBaseChannel.h b/netwerk/protocol/http/HttpBaseChannel.h index 3c4e6b06aa97..56230ce245c1 100644 --- a/netwerk/protocol/http/HttpBaseChannel.h +++ b/netwerk/protocol/http/HttpBaseChannel.h @@ -271,6 +271,7 @@ public: NS_IMETHOD SetLastRedirectFlags(uint32_t aValue) override; NS_IMETHOD GetNavigationStartTimeStamp(TimeStamp* aTimeStamp) override; NS_IMETHOD SetNavigationStartTimeStamp(TimeStamp aTimeStamp) override; + NS_IMETHOD CancelForTrackingProtection() override; inline void CleanRedirectCacheChainIfNecessary() { diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 7153902fa2ad..ccf0a27aaac5 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -466,6 +466,21 @@ nsHttpChannel::PrepareToConnect() return OnBeforeConnect(); } +void +nsHttpChannel::HandleContinueCancelledByTrackingProtection() +{ + MOZ_ASSERT(!mCallOnResume, "How did that happen?"); + + if (mSuspendCount) { + LOG(("Waiting until resume HandleContinueCancelledByTrackingProtection [this=%p]\n", this)); + mCallOnResume = &nsHttpChannel::HandleContinueCancelledByTrackingProtection; + return; + } + + LOG(("nsHttpChannel::HandleContinueCancelledByTrackingProtection [this=%p]\n", this)); + ContinueCancelledByTrackingProtection(); +} + void nsHttpChannel::HandleOnBeforeConnect() { @@ -5987,6 +6002,8 @@ nsHttpChannel::Cancel(nsresult status) MOZ_ASSERT(NS_IsMainThread()); // We should never have a pump open while a CORS preflight is in progress. MOZ_ASSERT_IF(mPreflightChannel, !mCachePump); + MOZ_ASSERT(status != NS_ERROR_TRACKING_URI, + "NS_ERROR_TRACKING_URI needs to be handled by CancelForTrackingProtection()"); LOG(("nsHttpChannel::Cancel [this=%p status=%" PRIx32 "]\n", this, static_cast(status))); @@ -5998,6 +6015,75 @@ nsHttpChannel::Cancel(nsresult status) if (mWaitingForRedirectCallback) { LOG(("channel canceled during wait for redirect callback")); } + + return CancelInternal(status); +} + +NS_IMETHODIMP +nsHttpChannel::CancelForTrackingProtection() +{ + MOZ_ASSERT(NS_IsMainThread()); + // We should never have a pump open while a CORS preflight is in progress. + MOZ_ASSERT_IF(mPreflightChannel, !mCachePump); + + LOG(("nsHttpChannel::CancelForTrackingProtection [this=%p]\n", this)); + + if (mCanceled) { + LOG((" ignoring; already canceled\n")); + return NS_OK; + } + + // We are being canceled by the channel classifier because of tracking + // protection, but we haven't yet had a chance to dispatch the + // "http-on-modify-request" notifications yet (this would normally be + // done in PrepareToConnect()). So do that now, before proceeding to + // cancel. + // + // Note that running these observers can itself result in the channel + // being canceled. In that case, we accept that cancelation code as + // the cause of the cancelation, as if the classification of the channel + // would have occurred past this point! + + // notify "http-on-modify-request" observers + CallOnModifyRequestObservers(); + + SetLoadGroupUserAgentOverride(); + + // Check if request was cancelled during on-modify-request or on-useragent. + if (mCanceled) { + return mStatus; + } + + if (mSuspendCount) { + LOG(("Waiting until resume in Cancel [this=%p]\n", this)); + MOZ_ASSERT(!mCallOnResume); + mCallOnResume = &nsHttpChannel::HandleContinueCancelledByTrackingProtection; + return NS_OK; + } + + return CancelInternal(NS_ERROR_TRACKING_URI); +} + +void +nsHttpChannel::ContinueCancelledByTrackingProtection() +{ + MOZ_ASSERT(NS_IsMainThread()); + // We should never have a pump open while a CORS preflight is in progress. + MOZ_ASSERT_IF(mPreflightChannel, !mCachePump); + + LOG(("nsHttpChannel::ContinueCancelledByTrackingProtection [this=%p]\n", + this)); + if (mCanceled) { + LOG((" ignoring; already canceled\n")); + return; + } + + Unused << CancelInternal(NS_ERROR_TRACKING_URI); +} + +nsresult +nsHttpChannel::CancelInternal(nsresult status) +{ mCanceled = true; mStatus = status; if (mProxyRequest) diff --git a/netwerk/protocol/http/nsHttpChannel.h b/netwerk/protocol/http/nsHttpChannel.h index dd8c9c116c63..2f04fc9b74cc 100644 --- a/netwerk/protocol/http/nsHttpChannel.h +++ b/netwerk/protocol/http/nsHttpChannel.h @@ -163,6 +163,7 @@ public: NS_IMETHOD SetChannelIsForDownload(bool aChannelIsForDownload) override; NS_IMETHOD GetNavigationStartTimeStamp(TimeStamp* aTimeStamp) override; NS_IMETHOD SetNavigationStartTimeStamp(TimeStamp aTimeStamp) override; + NS_IMETHOD CancelForTrackingProtection() override; // nsISupportsPriority NS_IMETHOD SetPriority(int32_t value) override; // nsIClassOfService @@ -284,6 +285,9 @@ private: typedef nsresult (nsHttpChannel::*nsContinueRedirectionFunc)(nsresult result); bool RequestIsConditional(); + void HandleContinueCancelledByTrackingProtection(); + nsresult CancelInternal(nsresult status); + void ContinueCancelledByTrackingProtection(); // Connections will only be established in this function. // (including DNS prefetch and speculative connection.) diff --git a/netwerk/protocol/http/nsIHttpChannelInternal.idl b/netwerk/protocol/http/nsIHttpChannelInternal.idl index 4bb3defb482c..63d9acdb2652 100644 --- a/netwerk/protocol/http/nsIHttpChannelInternal.idl +++ b/netwerk/protocol/http/nsIHttpChannelInternal.idl @@ -333,4 +333,12 @@ interface nsIHttpChannelInternal : nsISupports // This is use to determine the duration since navigation started. [noscript] attribute TimeStamp navigationStartTimeStamp; + /** + * Cancel a channel because we have determined that it needs to be blocked + * for tracking protection. This is an internal API that is meant to be + * called by the channel classifier. Please DO NOT use this API if you don't + * know whether you should be using it. + */ + [noscript] void cancelForTrackingProtection(); + }; From d07d6c1b5c2e747fc6c20c32f53cf8a7e7d4e87c Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Thu, 16 Aug 2018 11:35:50 -0700 Subject: [PATCH 28/45] Bug 1483865: Make NotStackAllocated assertion a bit safer. r=froydnj This assertion was always meant to be a best-effort thing to catch obvious errors, but the cases where the assumptions it makes fail have been growing. I could remove it entirely, but I'd be happier keeping at least some basic sanity checks. This compromise continues allowing any address below the first argument pointer, and extends the assertion to also allow anything more than 2KiB above it. We could probably get away with stretching that to at least 4KiB, but 2 seems safer, and likely enough to catch the obvious cases. Differential Revision: https://phabricator.services.mozilla.com/D3542 --HG-- extra : rebase_source : 9e17aa3d751f75deff67e40d223fda7aaa4f84f0 extra : amend_source : 86ada4dd01584defeaa5dcb093ad9a73b34f0318 --- xpcom/components/nsComponentManager.cpp | 33 +++++++++++++++++++------ 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/xpcom/components/nsComponentManager.cpp b/xpcom/components/nsComponentManager.cpp index 654f42099154..9cc16ae70f68 100644 --- a/xpcom/components/nsComponentManager.cpp +++ b/xpcom/components/nsComponentManager.cpp @@ -480,14 +480,31 @@ template static void AssertNotStackAllocated(T* aPtr) { - // The main thread's stack should be allocated at the top of our address - // space. Anything stack allocated should be above us on the stack, and - // therefore above our first argument pointer. - // Only this is apparently not the case on Windows. -#if !(defined(XP_WIN) || defined(ANDROID)) - MOZ_ASSERT(NS_IsMainThread()); - MOZ_ASSERT(uintptr_t(aPtr) < uintptr_t(&aPtr)); -#endif + // On all of our supported platforms, the stack grows down. Any address + // located below the address of our argument is therefore guaranteed not to be + // stack-allocated by the caller. + // + // For addresses above our argument, things get trickier. The main thread + // stack is traditionally placed at the top of the program's address space, + // but that is becoming less reliable as more and more systems adopt address + // space layout randomization strategies, so we have to guess how much space + // above our argument pointer we need to care about. + // + // On most systems, we're guaranteed at least several KiB at the top of each + // stack for TLS. We'd probably be safe assuming at least 4KiB in the stack + // segment above our argument address, but safer is... well, safer. + // + // For threads with huge stacks, it's theoretically possible that we could + // wind up being passed a stack-allocated string from farther up the stack, + // but this is a best-effort thing, so we'll assume we only care about the + // immediate caller. For that case, max 2KiB per stack frame is probably a + // reasonable guess most of the time, and is less than the ~4KiB that we + // expect for TLS, so go with that to avoid the risk of bumping into heap + // data just above the stack. + static constexpr size_t kFuzz = 2048; + + MOZ_ASSERT(uintptr_t(aPtr) < uintptr_t(&aPtr) || + uintptr_t(aPtr) > uintptr_t(&aPtr) + kFuzz); } static inline nsCString From 55b51772a47970495d08f2d06dea40877d1540b6 Mon Sep 17 00:00:00 2001 From: shindli Date: Wed, 22 Aug 2018 21:31:45 +0300 Subject: [PATCH 29/45] Backed out changeset a5f51c76930c (bug 1483865) for bustages in builds/worker/workspace/build/src/xpcom/components/nsComponentManager.cpp on a CLOSED TREE --- xpcom/components/nsComponentManager.cpp | 33 ++++++------------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/xpcom/components/nsComponentManager.cpp b/xpcom/components/nsComponentManager.cpp index 9cc16ae70f68..654f42099154 100644 --- a/xpcom/components/nsComponentManager.cpp +++ b/xpcom/components/nsComponentManager.cpp @@ -480,31 +480,14 @@ template static void AssertNotStackAllocated(T* aPtr) { - // On all of our supported platforms, the stack grows down. Any address - // located below the address of our argument is therefore guaranteed not to be - // stack-allocated by the caller. - // - // For addresses above our argument, things get trickier. The main thread - // stack is traditionally placed at the top of the program's address space, - // but that is becoming less reliable as more and more systems adopt address - // space layout randomization strategies, so we have to guess how much space - // above our argument pointer we need to care about. - // - // On most systems, we're guaranteed at least several KiB at the top of each - // stack for TLS. We'd probably be safe assuming at least 4KiB in the stack - // segment above our argument address, but safer is... well, safer. - // - // For threads with huge stacks, it's theoretically possible that we could - // wind up being passed a stack-allocated string from farther up the stack, - // but this is a best-effort thing, so we'll assume we only care about the - // immediate caller. For that case, max 2KiB per stack frame is probably a - // reasonable guess most of the time, and is less than the ~4KiB that we - // expect for TLS, so go with that to avoid the risk of bumping into heap - // data just above the stack. - static constexpr size_t kFuzz = 2048; - - MOZ_ASSERT(uintptr_t(aPtr) < uintptr_t(&aPtr) || - uintptr_t(aPtr) > uintptr_t(&aPtr) + kFuzz); + // The main thread's stack should be allocated at the top of our address + // space. Anything stack allocated should be above us on the stack, and + // therefore above our first argument pointer. + // Only this is apparently not the case on Windows. +#if !(defined(XP_WIN) || defined(ANDROID)) + MOZ_ASSERT(NS_IsMainThread()); + MOZ_ASSERT(uintptr_t(aPtr) < uintptr_t(&aPtr)); +#endif } static inline nsCString From 884a0a38023e20a47361ea89006f9b34e533ebf2 Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Wed, 11 Jul 2018 17:07:16 -0500 Subject: [PATCH 30/45] Bug 1475417 - Part 0: Add some passing tests. r=jimb Also deletes two tests that are completely redundant ever since we removed legacy generators. --HG-- extra : rebase_source : edb5b59cdc40b9425bcf6eec26dd7a824cd30f10 --- .../Debugger-onEnterFrame-resumption-03.js | 4 +- js/src/jit-test/tests/debug/Frame-live-06.js | 26 ++++++++ .../tests/debug/Frame-onPop-generators-04.js | 26 ++++++++ .../debug/Frame-onPop-star-generators-01.js | 20 ------- .../debug/Frame-onPop-star-generators-02.js | 21 ------- .../debug/Frame-onStep-generators-defaults.js | 33 ++++++++++ .../jit-test/tests/debug/breakpoint-oom-01.js | 42 +++++++++++++ ...Statement-async-generator-resumption-01.js | 60 +++++++++++++++++++ ...onDebuggerStatement-async-resumption-01.js | 34 +++++++++++ .../debug/onEnterFrame-async-resumption-01.js | 33 ++++++++++ 10 files changed, 256 insertions(+), 43 deletions(-) create mode 100644 js/src/jit-test/tests/debug/Frame-live-06.js create mode 100644 js/src/jit-test/tests/debug/Frame-onPop-generators-04.js delete mode 100644 js/src/jit-test/tests/debug/Frame-onPop-star-generators-01.js delete mode 100644 js/src/jit-test/tests/debug/Frame-onPop-star-generators-02.js create mode 100644 js/src/jit-test/tests/debug/Frame-onStep-generators-defaults.js create mode 100644 js/src/jit-test/tests/debug/breakpoint-oom-01.js create mode 100644 js/src/jit-test/tests/debug/onDebuggerStatement-async-generator-resumption-01.js create mode 100644 js/src/jit-test/tests/debug/onDebuggerStatement-async-resumption-01.js create mode 100644 js/src/jit-test/tests/debug/onEnterFrame-async-resumption-01.js diff --git a/js/src/jit-test/tests/debug/Debugger-onEnterFrame-resumption-03.js b/js/src/jit-test/tests/debug/Debugger-onEnterFrame-resumption-03.js index a42f2183a3a4..e28b014944f2 100644 --- a/js/src/jit-test/tests/debug/Debugger-onEnterFrame-resumption-03.js +++ b/js/src/jit-test/tests/debug/Debugger-onEnterFrame-resumption-03.js @@ -1,4 +1,4 @@ -// If debugger.onEnterFrame returns {return:val}, the frame returns immediately. +// If debugger.onEnterFrame returns null, the debuggee is terminated immediately. load(libdir + "asserts.js"); @@ -13,7 +13,7 @@ dbg.onDebuggerStatement = function (frame) { innerSavedFrame = frame; return null; }; - // Using frame.eval lets us catch termination. + // Using frame.eval lets us catch termination. assertEq(frame.eval("set = true;"), null); assertEq(innerSavedFrame.live, false); savedFrame = frame; diff --git a/js/src/jit-test/tests/debug/Frame-live-06.js b/js/src/jit-test/tests/debug/Frame-live-06.js new file mode 100644 index 000000000000..f1cc4abf1af8 --- /dev/null +++ b/js/src/jit-test/tests/debug/Frame-live-06.js @@ -0,0 +1,26 @@ +// frame.live is false for generator frames after they return. + +let g = newGlobal(); +g.eval("function* f() { debugger; }"); + +let dbg = Debugger(g); +let savedFrame; + +dbg.onDebuggerStatement = frame => { + savedFrame = frame; + assertEq(frame.callee.name, "f"); + assertEq(frame.live, true); + frame.onPop = function() { + assertEq(frame.live, true); + }; +}; +g.f().next(); + +assertEq(savedFrame.live, false); +try { + savedFrame.older; + throw new Error("expected exception, none thrown"); +} catch (exc) { + assertEq(exc.message, "Debugger.Frame is not live"); +} + diff --git a/js/src/jit-test/tests/debug/Frame-onPop-generators-04.js b/js/src/jit-test/tests/debug/Frame-onPop-generators-04.js new file mode 100644 index 000000000000..2c686e278efb --- /dev/null +++ b/js/src/jit-test/tests/debug/Frame-onPop-generators-04.js @@ -0,0 +1,26 @@ +// Terminating a generator from the onPop callback for its initial yield +// leaves the Frame in a sane but inactive state. + +load(libdir + "asserts.js"); + +let g = newGlobal(); +g.eval("function* f(x) { yield x; }"); +let dbg = new Debugger; +let gw = dbg.addDebuggee(g); + +let genFrame = null; +dbg.onDebuggerStatement = frame => { + dbg.onEnterFrame = frame => { + if (frame.callee == gw.getOwnPropertyDescriptor("f").value) { + genFrame = frame; + frame.onPop = completion => null; + } + }; + assertEq(frame.eval("f(0);"), null); +}; + +g.eval("debugger;"); + +assertEq(genFrame instanceof Debugger.Frame, true); +assertEq(genFrame.live, false); +assertThrowsInstanceOf(() => genFrame.callee, Error); diff --git a/js/src/jit-test/tests/debug/Frame-onPop-star-generators-01.js b/js/src/jit-test/tests/debug/Frame-onPop-star-generators-01.js deleted file mode 100644 index 73cf3c8c7d3b..000000000000 --- a/js/src/jit-test/tests/debug/Frame-onPop-star-generators-01.js +++ /dev/null @@ -1,20 +0,0 @@ -// Returning {throw:} from an onPop handler when yielding works and -// closes the generator-iterator. - -load(libdir + "iteration.js"); - -var g = newGlobal(); -var dbg = new Debugger; -var gw = dbg.addDebuggee(g); -dbg.onDebuggerStatement = function handleDebugger(frame) { - frame.onPop = function (c) { - return {throw: "fit"}; - }; -}; -g.eval("function* g() { for (var i = 0; i < 10; i++) { debugger; yield i; } }"); -g.eval("var it = g();"); -var rv = gw.executeInGlobal("it.next();"); -assertEq(rv.throw, "fit"); - -dbg.enabled = false; -assertIteratorDone(g.it); diff --git a/js/src/jit-test/tests/debug/Frame-onPop-star-generators-02.js b/js/src/jit-test/tests/debug/Frame-onPop-star-generators-02.js deleted file mode 100644 index de9ad5009825..000000000000 --- a/js/src/jit-test/tests/debug/Frame-onPop-star-generators-02.js +++ /dev/null @@ -1,21 +0,0 @@ -// |jit-test| error: fit - -// Throwing an exception from an onPop handler when yielding terminates the debuggee -// but does not close the generator-iterator. - -load(libdir + 'iteration.js') - -var g = newGlobal(); -var dbg = new Debugger; -var gw = dbg.addDebuggee(g); -dbg.onDebuggerStatement = function handleDebugger(frame) { - frame.onPop = function (c) { - throw "fit"; - }; -}; -g.eval("function* g() { for (var i = 0; i < 10; i++) { debugger; yield i; } }"); -g.eval("var it = g();"); -assertEq(gw.executeInGlobal("it.next();"), null); - -dbg.enabled = false; -assertIteratorNext(g.it, 1); diff --git a/js/src/jit-test/tests/debug/Frame-onStep-generators-defaults.js b/js/src/jit-test/tests/debug/Frame-onStep-generators-defaults.js new file mode 100644 index 000000000000..0c337636ee3c --- /dev/null +++ b/js/src/jit-test/tests/debug/Frame-onStep-generators-defaults.js @@ -0,0 +1,33 @@ +// onStep works during the evaluation of default parameter values in generators. +// +// (They're evaluated at a weird time in the generator life cycle, before the +// generator object is created.) + +let g = newGlobal(); +g.eval(`\ + function f1() {} // line 1 + function f2() {} // 2 + function f3() {} // 3 + // 4 + function* gen( // 5 + name, // 6 + schema = f1(), // 7 + timeToLive = f2(), // 8 + lucidity = f3() // 9 + ) { // 10 + } // 11 +`); + +let dbg = Debugger(g); +let log = []; +dbg.onEnterFrame = frame => { + frame.onStep = () => { + let line = frame.script.getOffsetLocation(frame.offset).lineNumber; + if (log.length == 0 || line != log[log.length - 1]) { + log.push(line); + } + }; +}; + +g.gen(0); +assertEq(log.toSource(), [5, 7, 1, 8, 2, 9, 3, 10].toSource()); diff --git a/js/src/jit-test/tests/debug/breakpoint-oom-01.js b/js/src/jit-test/tests/debug/breakpoint-oom-01.js new file mode 100644 index 000000000000..938b5f28e690 --- /dev/null +++ b/js/src/jit-test/tests/debug/breakpoint-oom-01.js @@ -0,0 +1,42 @@ +// Test for OOM hitting a breakpoint in a generator. +// +// (The purpose is to test OOM-handling in the code that creates the +// Debugger.Frame object and associates it with the generator object.) + +if (!('oomTest' in this)) + quit(); + +let g = newGlobal(); +g.eval(`\ + function* gen(x) { // line 1 + x++; // 2 + yield x; // 3 + } // 4 +`); + +let dbg = new Debugger; + +// On OOM in the debugger, propagate it to the debuggee. +dbg.uncaughtExceptionHook = exc => exc === "out of memory" ? {throw: exc} : null; + +let gw = dbg.addDebuggee(g); +let script = gw.makeDebuggeeValue(g.gen).script; +let hits = 0; +let handler = { + hit(frame) { + hits++; + print("x=", frame.environment.getVariable("x")); + } +}; +for (let offset of script.getLineOffsets(2)) + script.setBreakpoint(offset, handler); + +let result; +oomTest(() => { + hits = 0; + result = g.gen(1).next(); +}, false); +assertEq(hits, 1); +assertEq(result.done, false); +assertEq(result.value, 2); + diff --git a/js/src/jit-test/tests/debug/onDebuggerStatement-async-generator-resumption-01.js b/js/src/jit-test/tests/debug/onDebuggerStatement-async-generator-resumption-01.js new file mode 100644 index 000000000000..bb83f6470e2c --- /dev/null +++ b/js/src/jit-test/tests/debug/onDebuggerStatement-async-generator-resumption-01.js @@ -0,0 +1,60 @@ +// A Debugger can {return:} from onDebuggerStatement in an async generator. +// A resolved promise for a {value: _, done: true} object is returned. + +load(libdir + "asserts.js"); + +let g = newGlobal(); +g.eval(` + async function* f(x) { + debugger; // when==0 to force return here + await x; + yield 1; + debugger; // when==1 to force return here + } +`); + +let exc = null; +let dbg = new Debugger; +let gw = dbg.addDebuggee(g); +function test(when) { + let hits = 0; + let outcome = "FAIL"; + dbg.onDebuggerStatement = frame => { + if (hits++ == when) + return {return: "ponies"}; + }; + + let iter = g.f(0); + + // At the initial suspend. + assertEq(hits, 0); + iter.next().then(result => { + // At the yield point, unless we already force-returned from the first + // debugger statement. + assertEq(hits, 1); + if (when == 0) + return result; + assertEq(result.value, 1); + assertEq(result.done, false); + return iter.next(); + }).then(result => { + // After forced return. + assertEq(hits, when + 1); + assertEq(result.value, "ponies"); + assertEq(result.done, true); + outcome = "pass"; + }).catch(e => { + // An assertion failed. + exc = e; + }); + + assertEq(hits, 1); + drainJobQueue(); + if (exc !== null) + throw exc; + assertEq(outcome, "pass"); +} + +for (let i = 0; i < 2; i++) { + test(i); +} diff --git a/js/src/jit-test/tests/debug/onDebuggerStatement-async-resumption-01.js b/js/src/jit-test/tests/debug/onDebuggerStatement-async-resumption-01.js new file mode 100644 index 000000000000..8f3f2c094224 --- /dev/null +++ b/js/src/jit-test/tests/debug/onDebuggerStatement-async-resumption-01.js @@ -0,0 +1,34 @@ +// A Debugger can {return:} from onDebuggerStatement in an async function. +// The async function's promise is resolved with the returned value. + +load(libdir + "asserts.js"); + +let g = newGlobal(); +g.eval(` + async function f(x) { + debugger; // when==0 to force return here + await x; + debugger; // when==1 to force return here + } +`); + +let dbg = new Debugger; +let gw = dbg.addDebuggee(g); +function test(when, what, expected) { + let hits = 0; + let result = "FAIL"; + dbg.onDebuggerStatement = frame => { + if (hits++ == when) + return {return: gw.makeDebuggeeValue(what)}; + }; + g.f(0).then(x => { result = x; }); + assertEq(hits, 1); + drainJobQueue(); + assertEq(hits, when + 1); + assertEq(result, expected); +} + +for (let i = 0; i < 2; i++) { + test(i, "ok", "ok"); + test(i, g.Promise.resolve(37), 37); +} diff --git a/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-01.js b/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-01.js new file mode 100644 index 000000000000..fbfc2c84a5a8 --- /dev/null +++ b/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-01.js @@ -0,0 +1,33 @@ +// A Debugger can {return:} from the first onEnterFrame for an async function. +// (The exact behavior is undocumented; we're testing that it doesn't crash.) + +let g = newGlobal(); +g.hit2 = false; +g.eval(`async function f(x) { await x; return "ponies"; }`); + +let dbg = new Debugger; +let gw = dbg.addDebuggee(g); +let hits = 0; +let resumption = undefined; +dbg.onEnterFrame = frame => { + if (frame.type == "call" && frame.callee.name === "f") { + frame.onPop = completion => { + assertEq(completion.return, resumption.return); + hits++; + }; + + // Don't tell anyone, but if we force-return a generator object here, + // the robots accept it as one of their own and plug it right into the + // async function machinery. This may be handy against Skynet someday. + resumption = frame.eval(`(function* f2() { hit2 = true; throw "fit"; })()`); + assertEq(resumption.return.class, "Generator"); + return resumption; + } +}; + +let p = g.f(0); +assertEq(hits, 1); +let pw = gw.makeDebuggeeValue(p); +assertEq(pw.isPromise, true); +assertEq(pw.promiseState, "rejected"); +assertEq(pw.promiseReason, "fit"); From 7bf28c7b7bbfe91558200a833709a6fa907a81ec Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Thu, 12 Jul 2018 19:43:55 -0500 Subject: [PATCH 31/45] Bug 1475417 - Part 1: Minor code tweaks, in preparation. There should be no observable change in behavior yet. r=jandem --HG-- extra : rebase_source : 8642f35da127b04292e71aff6b2cde049899c802 --- js/src/jit/BaselineCompiler.cpp | 4 ++-- js/src/jit/VMFunctions.cpp | 4 ++++ js/src/vm/GeneratorObject.cpp | 18 +++--------------- js/src/vm/GeneratorObject.h | 4 ++-- js/src/vm/Interpreter.cpp | 16 +++++++++++++--- js/src/vm/Stack.h | 2 +- 6 files changed, 25 insertions(+), 23 deletions(-) diff --git a/js/src/jit/BaselineCompiler.cpp b/js/src/jit/BaselineCompiler.cpp index 02855855dd09..df3a04f1c470 100644 --- a/js/src/jit/BaselineCompiler.cpp +++ b/js/src/jit/BaselineCompiler.cpp @@ -4693,7 +4693,7 @@ static const VMFunction InterpretResumeInfo = typedef bool (*GeneratorThrowFn)(JSContext*, BaselineFrame*, Handle, HandleValue, uint32_t); -static const VMFunction GeneratorThrowInfo = +static const VMFunction GeneratorThrowOrReturnInfo = FunctionInfo(jit::GeneratorThrowOrReturn, "GeneratorThrowOrReturn", TailCall); bool @@ -4878,7 +4878,7 @@ BaselineCompiler::emit_JSOP_RESUME() pushArg(genObj); pushArg(scratch2); - TrampolinePtr code = cx->runtime()->jitRuntime()->getVMWrapper(GeneratorThrowInfo); + TrampolinePtr code = cx->runtime()->jitRuntime()->getVMWrapper(GeneratorThrowOrReturnInfo); // Create the frame descriptor. masm.subStackPtrFrom(scratch1); diff --git a/js/src/jit/VMFunctions.cpp b/js/src/jit/VMFunctions.cpp index e54d4063811d..8b91a778cd43 100644 --- a/js/src/jit/VMFunctions.cpp +++ b/js/src/jit/VMFunctions.cpp @@ -977,6 +977,10 @@ GeneratorThrowOrReturn(JSContext* cx, BaselineFrame* frame, HandleyieldAndAwaitOffsets()[genObj->yieldAndAwaitIndex()]; frame->setOverridePc(script->offsetToPC(offset)); + // In the interpreter, GeneratorObject::resume marks the generator as running, + // so we do the same. + genObj->setRunning(); + MOZ_ALWAYS_TRUE(DebugAfterYield(cx, frame)); MOZ_ALWAYS_FALSE(js::GeneratorThrowOrReturn(cx, frame, genObj, arg, resumeKind)); return false; diff --git a/js/src/vm/GeneratorObject.cpp b/js/src/vm/GeneratorObject.cpp index 4e983ae57d09..d4d61743c23e 100644 --- a/js/src/vm/GeneratorObject.cpp +++ b/js/src/vm/GeneratorObject.cpp @@ -134,7 +134,6 @@ js::GeneratorThrowOrReturn(JSContext* cx, AbstractFramePtr frame, HandlesetPendingException(arg); - genObj->setRunning(); } else { MOZ_ASSERT(resumeKind == GeneratorObject::RETURN); @@ -150,9 +149,8 @@ js::GeneratorThrowOrReturn(JSContext* cx, AbstractFramePtr frame, Handle genObj, HandleValue arg) { - Rooted genObj(cx, &obj->as()); MOZ_ASSERT(genObj->isSuspended()); RootedFunction callee(cx, &genObj->callee()); @@ -184,18 +182,8 @@ GeneratorObject::resume(JSContext* cx, InterpreterActivation& activation, MOZ_ASSERT(activation.regs().spForStackDepth(activation.regs().stackDepth())); activation.regs().sp[-1] = arg; - switch (resumeKind) { - case NEXT: - genObj->setRunning(); - return true; - - case THROW: - case RETURN: - return GeneratorThrowOrReturn(cx, activation.regs().fp(), genObj, arg, resumeKind); - - default: - MOZ_CRASH("bad resumeKind"); - } + genObj->setRunning(); + return true; } const Class GeneratorObject::class_ = { diff --git a/js/src/vm/GeneratorObject.h b/js/src/vm/GeneratorObject.h index e9f9de70a204..d09510021117 100644 --- a/js/src/vm/GeneratorObject.h +++ b/js/src/vm/GeneratorObject.h @@ -60,7 +60,7 @@ class GeneratorObject : public NativeObject static JSObject* create(JSContext* cx, AbstractFramePtr frame); static bool resume(JSContext* cx, InterpreterActivation& activation, - HandleObject obj, HandleValue arg, ResumeKind resumeKind); + Handle genObj, HandleValue arg); static bool initialSuspend(JSContext* cx, HandleObject obj, AbstractFramePtr frame, jsbytecode* pc) { return suspend(cx, obj, frame, pc, nullptr, 0); @@ -147,7 +147,7 @@ class GeneratorObject : public NativeObject setFixedSlot(YIELD_AND_AWAIT_INDEX_SLOT, Int32Value(YIELD_AND_AWAIT_INDEX_RUNNING)); } void setClosing() { - MOZ_ASSERT(isSuspended()); + MOZ_ASSERT(isRunning()); setFixedSlot(YIELD_AND_AWAIT_INDEX_SLOT, Int32Value(YIELD_AND_AWAIT_INDEX_CLOSING)); } void setYieldAndAwaitIndex(uint32_t yieldAndAwaitIndex) { diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index 47c84f3ea35a..468cb1033766 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -4267,13 +4267,14 @@ CASE(JSOP_AWAIT) CASE(JSOP_RESUME) { { - ReservedRooted gen(&rootObject0, ®S.sp[-2].toObject()); + Rooted gen(cx, ®S.sp[-2].toObject().as()); ReservedRooted val(&rootValue0, REGS.sp[-1]); // popInlineFrame expects there to be an additional value on the stack // to pop off, so leave "gen" on the stack. GeneratorObject::ResumeKind resumeKind = GeneratorObject::getResumeKind(REGS.pc); - bool ok = GeneratorObject::resume(cx, activation, gen, val, resumeKind); + if (!GeneratorObject::resume(cx, activation, gen, val)) + goto error; JSScript* generatorScript = REGS.fp()->script(); if (cx->realm() != generatorScript->realm()) @@ -4285,8 +4286,17 @@ CASE(JSOP_RESUME) TraceLogStartEvent(logger, scriptEvent); TraceLogStartEvent(logger, TraceLogger_Interpreter); - if (!ok) + switch (resumeKind) { + case GeneratorObject::NEXT: + break; + case GeneratorObject::THROW: + case GeneratorObject::RETURN: + MOZ_ALWAYS_FALSE(GeneratorThrowOrReturn(cx, activation.regs().fp(), gen, val, + resumeKind)); goto error; + default: + MOZ_CRASH("bad resumeKind"); + } } ADVANCE_AND_DISPATCH(0); } diff --git a/js/src/vm/Stack.h b/js/src/vm/Stack.h index 65c5307acae4..ddc2cd2b210d 100644 --- a/js/src/vm/Stack.h +++ b/js/src/vm/Stack.h @@ -2159,7 +2159,7 @@ class FrameIter bool ensureHasRematerializedFrame(JSContext* cx); // True when isInterp() or isBaseline(). True when isIon() if it - // has a rematerialized frame. False otherwise false otherwise. + // has a rematerialized frame. False otherwise. bool hasUsableAbstractFramePtr() const; // ----------------------------------------------------------- From 383345c1468146d289f4d4e4b94b4fffc8d05b10 Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Wed, 15 Aug 2018 15:09:30 -0500 Subject: [PATCH 32/45] Bug 1475417 - Part 2: Fire onEnterFrame when resuming a generator or async function. r=jandem, r=jimb --HG-- extra : rebase_source : c6f2cd2cc77bea35e13af0dcc3fa7c50beb07e9b extra : source : a3901162a1ece01cb8bacf606ad57e3cef741b7e --- js/src/jit-test/tests/debug/Frame-live-07.js | 53 ++++++++++++ .../tests/debug/Frame-onStep-generators-01.js | 31 +++++++ .../tests/debug/Frame-onStep-generators-02.js | 44 ++++++++++ .../tests/debug/Frame-onStep-generators-03.js | 45 +++++++++++ .../debug/onEnterFrame-async-resumption-02.js | 36 +++++++++ .../debug/onEnterFrame-async-resumption-03.js | 30 +++++++ .../tests/debug/onEnterFrame-generator-01.js | 81 +++++++++++++++++++ .../tests/debug/onEnterFrame-generator-02.js | 27 +++++++ .../tests/debug/onEnterFrame-generator-03.js | 25 ++++++ .../onEnterFrame-generator-resumption-01.js | 36 +++++++++ .../onEnterFrame-generator-resumption-02.js | 39 +++++++++ .../onEnterFrame-generator-resumption-03.js | 35 ++++++++ js/src/jit/BaselineCompiler.cpp | 17 +++- js/src/jit/BaselineDebugModeOSR.cpp | 22 ++++- js/src/jit/SharedIC.h | 3 +- js/src/jit/VMFunctions.cpp | 31 +++++-- js/src/jit/VMFunctions.h | 2 +- js/src/vm/Debugger.cpp | 9 +++ js/src/vm/GeneratorObject.h | 4 + js/src/vm/Interpreter.cpp | 14 ++++ 20 files changed, 570 insertions(+), 14 deletions(-) create mode 100644 js/src/jit-test/tests/debug/Frame-live-07.js create mode 100644 js/src/jit-test/tests/debug/Frame-onStep-generators-01.js create mode 100644 js/src/jit-test/tests/debug/Frame-onStep-generators-02.js create mode 100644 js/src/jit-test/tests/debug/Frame-onStep-generators-03.js create mode 100644 js/src/jit-test/tests/debug/onEnterFrame-async-resumption-02.js create mode 100644 js/src/jit-test/tests/debug/onEnterFrame-async-resumption-03.js create mode 100644 js/src/jit-test/tests/debug/onEnterFrame-generator-01.js create mode 100644 js/src/jit-test/tests/debug/onEnterFrame-generator-02.js create mode 100644 js/src/jit-test/tests/debug/onEnterFrame-generator-03.js create mode 100644 js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-01.js create mode 100644 js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-02.js create mode 100644 js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-03.js diff --git a/js/src/jit-test/tests/debug/Frame-live-07.js b/js/src/jit-test/tests/debug/Frame-live-07.js new file mode 100644 index 000000000000..da3b4e1bba93 --- /dev/null +++ b/js/src/jit-test/tests/debug/Frame-live-07.js @@ -0,0 +1,53 @@ +// frame.live is false for generator frames popped due to exception or termination. + +load(libdir + "/asserts.js"); + +function test(when, what) { + let g = newGlobal(); + g.eval("function* f(x) { yield x; }"); + + let dbg = new Debugger; + let gw = dbg.addDebuggee(g); + let fw = gw.getOwnPropertyDescriptor("f").value; + + let t = 0; + let poppedFrame; + + function tick(frame) { + if (frame.callee == fw) { + if (t == when) { + poppedFrame = frame; + dbg.onEnterFrame = undefined; + frame.onPop = undefined; + return what; + } + t++; + } + return undefined; + } + + dbg.onDebuggerStatement = frame => { + dbg.onEnterFrame = frame => { + frame.onPop = function() { + return tick(this); + }; + return tick(frame); + }; + let result = frame.eval("for (let _ of f(0)) {}"); + assertDeepEq(result, what); + }; + g.eval("debugger;"); + + assertEq(t, when); + assertEq(poppedFrame.live, false); + assertErrorMessage(() => poppedFrame.older, + Error, + "Debugger.Frame is not live"); +} + +for (let when = 0; when < 6; when++) { + for (let what of [null, {throw: "fit"}]) { + test(when, what); + } +} + diff --git a/js/src/jit-test/tests/debug/Frame-onStep-generators-01.js b/js/src/jit-test/tests/debug/Frame-onStep-generators-01.js new file mode 100644 index 000000000000..1183f01380e3 --- /dev/null +++ b/js/src/jit-test/tests/debug/Frame-onStep-generators-01.js @@ -0,0 +1,31 @@ +// Stepping into the `.next()` method of a generator works as expected. + +let g = newGlobal(); +g.eval(`\ +function* nums() { // line 1 + yield 1; // 2 + yield 2; // 3 +} // 4 +function f() { // 5 + let gen = nums(); // 6 + gen.next(); // 7 + gen.next(); // 8 + gen.next(); // 9 +} // 10 +`); + +let log = []; +let previousLine = -1; +let dbg = new Debugger(g); +dbg.onEnterFrame = frame => { + frame.onStep = () => { + let line = frame.script.getOffsetLocation(frame.offset).lineNumber; + if (previousLine != line) { // We stepped to a new line. + log.push(line); + previousLine = line; + } + }; +}; + +g.f(); +assertEq(log.join(" "), "5 6 1 6 7 1 2 7 8 2 3 8 9 3 9 10"); diff --git a/js/src/jit-test/tests/debug/Frame-onStep-generators-02.js b/js/src/jit-test/tests/debug/Frame-onStep-generators-02.js new file mode 100644 index 000000000000..bc77b3d3cdd3 --- /dev/null +++ b/js/src/jit-test/tests/debug/Frame-onStep-generators-02.js @@ -0,0 +1,44 @@ +// Stepping into the `.throw()` method of a generator with no relevant catch block. +// +// The debugger fires onEnterFrame and then frame.onPop for the generator frame when +// `gen.throw()` is called. + +load(libdir + "asserts.js"); + +let g = newGlobal(); +g.eval(`\ +function* z() { // line 1 + yield 1; // 2 + yield 2; // 3 +} // 4 +function f() { // 5 + let gen = z(); // 6 + gen.next(); // 7 + gen.throw("fit"); // 8 +} // 9 +`); + +let log = ""; +let previousLine = -1; +let dbg = new Debugger(g); +dbg.onEnterFrame = frame => { + log += frame.callee.name + "{"; + frame.onStep = () => { + let line = frame.script.getOffsetLocation(frame.offset).lineNumber; + if (previousLine != line) { // We stepped to a new line. + log += line; + previousLine = line; + } + }; + frame.onPop = completion => { + if ("throw" in completion) + log += "!"; + log += "}"; + } +}; + +assertThrowsValue(() => g.f(), "fit"); +// z{1} is the initial generator setup. +// z{12} is the first .next() call, running to `yield 1` on line 2 +// The final `z{!}` is for the .throw() call. +assertEq(log, "f{56z{1}67z{12}78z{!}!}"); diff --git a/js/src/jit-test/tests/debug/Frame-onStep-generators-03.js b/js/src/jit-test/tests/debug/Frame-onStep-generators-03.js new file mode 100644 index 000000000000..5a49b3c6df08 --- /dev/null +++ b/js/src/jit-test/tests/debug/Frame-onStep-generators-03.js @@ -0,0 +1,45 @@ +// Stepping into the `.throw()` method of a generator with a relevant catch block. + +load(libdir + "asserts.js"); + +let g = newGlobal(); +g.eval(`\ +function* z() { // line 1 + try { // 2 + yield 1; // 3 + } catch (exc) { // 4 + yield 2; // 5 + } // 6 +} // 7 +function f() { // 8 + let gen = z(); // 9 + gen.next(); // 10 + gen.throw("fit"); // 11 +} // 12 +`); + +let log = []; +let previousLine = -1; +let dbg = new Debugger(g); +dbg.onEnterFrame = frame => { + log.push(frame.callee.name + " in"); + frame.onStep = () => { + let line = frame.script.getOffsetLocation(frame.offset).lineNumber; + if (previousLine != line) { // We stepped to a new line. + log.push(line); + previousLine = line; + } + }; + frame.onPop = completion => { + log.push(frame.callee.name + " out"); + }; +}; + +g.f(); +assertEq( + log.join(", "), + "f in, 8, 9, z in, 1, z out, " + + "9, 10, z in, 1, 2, 3, z out, " + + "10, 11, z in, 2, 4, 5, z out, " + // not sure why we hit line 2 here, source notes bug maybe + "11, 12, f out" +); diff --git a/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-02.js b/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-02.js new file mode 100644 index 000000000000..3125654d9772 --- /dev/null +++ b/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-02.js @@ -0,0 +1,36 @@ +// A Debugger can {throw:} from onEnterFrame in an async function. +// The resulting promise (if any) is rejected with the thrown error value. + +load(libdir + "asserts.js"); + +let g = newGlobal(); +g.eval(` + async function f() { await 1; } + var err = new TypeError("object too hairy"); +`); + +let dbg = new Debugger; +let gw = dbg.addDebuggee(g); +let errw = gw.makeDebuggeeValue(g.err); + +// Repeat the test for each onEnterFrame event. +// It fires up to three times: +// - when the async function g.f is called; +// - when we enter it to run to `await 1`; +// - when we resume after the await to run to the end. +for (let when = 0; when < 3; when++) { + let hits = 0; + dbg.onEnterFrame = frame => { + return hits++ < when ? undefined : {throw: errw}; + }; + + let result = undefined; + g.f() + .then(value => { result = {returned: value}; }) + .catch(err => { result = {threw: err}; }); + + drainJobQueue(); + + assertEq(hits, when + 1); + assertDeepEq(result, {threw: g.err}); +} diff --git a/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-03.js b/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-03.js new file mode 100644 index 000000000000..68c3ded27f4f --- /dev/null +++ b/js/src/jit-test/tests/debug/onEnterFrame-async-resumption-03.js @@ -0,0 +1,30 @@ +// A Debugger can {return:} from onEnterFrame at any resume point in an async function. +// The async function's promise is resolved with the returned value. + +let g = newGlobal(); +g.eval(`async function f(x) { await x; }`); + +let dbg = new Debugger(g); +function test(when) { + let hits = 0; + dbg.onEnterFrame = frame => { + if (frame.type == "call" && frame.callee.name === "f") { + if (hits++ == when) { + return {return: "exit"}; + } + } + }; + + let result = undefined; + let finished = false; + g.f("hello").then(value => { result = value; finished = true; }); + drainJobQueue(); + assertEq(finished, true); + assertEq(hits, when + 1); + assertEq(result, "exit"); +} + +// onEnterFrame with hits==0 is not a resume point; {return:} behaves differently there +// (see onEnterFrame-async-resumption-02.js). +test(1); // force return from first resume point, immediately after the initial suspend +test(2); // force return from second resume point, immediately after the await instruction diff --git a/js/src/jit-test/tests/debug/onEnterFrame-generator-01.js b/js/src/jit-test/tests/debug/onEnterFrame-generator-01.js new file mode 100644 index 000000000000..8cfceba5ba3f --- /dev/null +++ b/js/src/jit-test/tests/debug/onEnterFrame-generator-01.js @@ -0,0 +1,81 @@ +// Frame properties and methods work in generator-resuming onEnterFrame events. +// Also tests onPop events, for good measure. + +let g = newGlobal(); +g.eval(`\ + function* gen(lo, hi) { + var a = 1/2; + yield a; + yield a * a; + } +`); +let dbg = new Debugger; +let gw = dbg.addDebuggee(g); + +let hits = 0; +let savedScript = null; +let savedEnv = null; +let savedOffsets = new Set; + +function check(frame) { + assertEq(frame.type, "call"); + assertEq(frame.constructing, false); + assertEq(frame.callee, gw.makeDebuggeeValue(g.gen)); + + // `arguments` elements don't work in resumed generator frames, + // because generators don't keep the arguments around. + // The first onEnterFrame and onPop events can see them. + assertEq(frame.arguments.length, hits < 2 ? args.length : 0); + for (var i = 0; i < frame.arguments.length; i++) { + assertEq(frame.arguments.hasOwnProperty(i), true); + + if (hits < 2) + assertEq(frame.arguments[i], gw.makeDebuggeeValue(args[i]), `arguments[${i}]`); + else + assertEq(frame.arguments[i], undefined); + } + + if (savedEnv === null) { + savedEnv = frame.environment; + assertEq(savedScript, null); + savedScript = frame.script; + } else { + assertEq(frame.environment, savedEnv); + assertEq(frame.script, savedScript); + } + let a_expected = hits < 3 ? undefined : 1/2; + assertEq(savedEnv.getVariable("a"), a_expected); + + assertEq(frame.generator, true); + assertEq(frame.live, true); + + let pc = frame.offset; + assertEq(savedOffsets.has(pc), false); + savedOffsets.add(pc); + + assertEq(frame.older, null); + assertEq(frame.this, gw); + assertEq(typeof frame.implementation, "string"); + + // And the moment of truth: + assertEq(frame.eval("2 + 2").return, 4); + assertEq(frame.eval("a").return, a_expected); + assertEq(frame.eval("if (a !== undefined) { assertEq(a < (lo + hi) / 2, true); } 7;").return, 7); +} + +dbg.onEnterFrame = frame => { + if (frame.type === "eval") + return; + check(frame); + hits++; + frame.onPop = completion => { + check(frame); + hits++; + }; +}; + +// g.gen ignores the arguments passed to it, but we use them to test +// frame.arguments. +let args = [0, 10, g, dbg]; +for (let v of g.gen(...args)) {} +assertEq(hits, 8); diff --git a/js/src/jit-test/tests/debug/onEnterFrame-generator-02.js b/js/src/jit-test/tests/debug/onEnterFrame-generator-02.js new file mode 100644 index 000000000000..b6b5e5181650 --- /dev/null +++ b/js/src/jit-test/tests/debug/onEnterFrame-generator-02.js @@ -0,0 +1,27 @@ +// onEnterFrame fires after the [[GeneratorState]] is set to "executing". +// +// This test checks that Debugger doesn't accidentally make it possible to +// reenter a generator frame that's already on the stack. (Also tests a fun +// corner case in baseline debug-mode OSR.) + +load(libdir + "asserts.js"); + +let g = newGlobal(); +g.eval('function* f() { yield 1; yield 2; }'); +let dbg = Debugger(g); +let genObj = null; +let hits = 0; +dbg.onEnterFrame = frame => { + // The first time onEnterFrame fires, there is no generator object, so + // there's nothing to test. The generator object doesn't exist until + // JSOP_GENERATOR is reached, right before the initial yield. + if (genObj !== null) { + dbg.removeDebuggee(g); // avoid the DebuggeeWouldRun exception + assertThrowsInstanceOf(() => genObj.next(), g.TypeError); + dbg.addDebuggee(g); + hits++; + } +}; +genObj = g.f(); +for (let x of genObj) {} +assertEq(hits, 3); diff --git a/js/src/jit-test/tests/debug/onEnterFrame-generator-03.js b/js/src/jit-test/tests/debug/onEnterFrame-generator-03.js new file mode 100644 index 000000000000..744313e8d7dd --- /dev/null +++ b/js/src/jit-test/tests/debug/onEnterFrame-generator-03.js @@ -0,0 +1,25 @@ +// If onEnterFrame terminates a generator, the Frame is left in a sane but inactive state. + +load(libdir + "asserts.js"); + +let g = newGlobal(); +g.eval("function* f(x) { yield x; }"); +let dbg = new Debugger; +let gw = dbg.addDebuggee(g); + +let genFrame = null; +dbg.onDebuggerStatement = frame => { + dbg.onEnterFrame = frame => { + if (frame.callee == gw.getOwnPropertyDescriptor("f").value) { + genFrame = frame; + return null; + } + }; + assertEq(frame.eval("f(0);"), null); +}; + +g.eval("debugger;"); + +assertEq(genFrame instanceof Debugger.Frame, true); +assertEq(genFrame.live, false); +assertThrowsInstanceOf(() => genFrame.callee, Error); diff --git a/js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-01.js b/js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-01.js new file mode 100644 index 000000000000..88dcf7b27e35 --- /dev/null +++ b/js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-01.js @@ -0,0 +1,36 @@ +// A debugger can {throw:} from onEnterFrame at any resume point in a generator. +// It closes the generator. + +load(libdir + "asserts.js"); + +let g = newGlobal(); +g.eval(` + function* f() { yield 1; } + var exn = new TypeError("object too hairy"); +`); + +let dbg = new Debugger; +let gw = dbg.addDebuggee(g); + +// Repeat the test for each onEnterFrame event. +// It fires up to three times: +// - when the generator g.f is called; +// - when we enter it to run to `yield 1`; +// - when we resume after the yield to run to the end. +for (let i = 0; i < 3; i++) { + let hits = 0; + dbg.onEnterFrame = frame => { + return hits++ < i ? undefined : {throw: gw.makeDebuggeeValue(g.exn)}; + }; + let genObj; + assertThrowsValue( + () => { + genObj = g.f(); + for (let x of genObj) {} + }, + g.exn + ); + assertEq(hits, i + 1); + if (hits > 1) + assertEq(genObj.next().done, true); +} diff --git a/js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-02.js b/js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-02.js new file mode 100644 index 000000000000..a3fa5be57c56 --- /dev/null +++ b/js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-02.js @@ -0,0 +1,39 @@ +// A Debugger can {return:} from onEnterFrame at any resume point in a generator. +// Force-returning closes the generator. + +load(libdir + "asserts.js"); + +let g = newGlobal(); +g.values = [1, 2, 3]; +g.eval(`function* f() { yield* values; }`); + +let dbg = Debugger(g); + +// onEnterFrame will fire up to 5 times. +// - once for the initial call to g.f(); +// - four times at resume points: +// - initial resume at the top of the generator body +// - resume after yielding 1 +// - resume after yielding 2 +// - resume after yielding 3 (this resumption will run to the end). +// This test ignores the initial call and focuses on resume points. +for (let i = 1; i < 5; i++) { + let hits = 0; + dbg.onEnterFrame = frame => { + return hits++ < i ? undefined : {return: "we're done here"}; + }; + + let genObj = g.f(); + let actual = []; + while (true) { + let r = genObj.next(); + if (r.done) { + assertDeepEq(r, {value: "we're done here", done: true}); + break; + } + actual.push(r.value); + } + assertEq(hits, i + 1); + assertDeepEq(actual, g.values.slice(0, i - 1)); + assertDeepEq(genObj.next(), {value: undefined, done: true}); +} diff --git a/js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-03.js b/js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-03.js new file mode 100644 index 000000000000..1340c1633c13 --- /dev/null +++ b/js/src/jit-test/tests/debug/onEnterFrame-generator-resumption-03.js @@ -0,0 +1,35 @@ +// Returning {throw:} from onEnterFrame when resuming inside a try block in a +// generator causes control to jump to the catch block. + +let g = newGlobal(); +g.eval(` + function* gen() { + try { + yield 0; + return "fail"; + } catch (exc) { + assertEq(exc, "fit"); + return "ok"; + } + } +`) + +let dbg = new Debugger(g); +let hits = 0; +dbg.onEnterFrame = frame => { + assertEq(frame.callee.name, "gen"); + if (++hits == 3) { + // First hit is when calling gen(); + // second hit is resuming at the implicit initial yield; + // third hit is resuming inside the try block. + return {throw: "fit"}; + } +}; + +let it = g.gen(); +let result = it.next(); +assertEq(result.done, false); +assertEq(result.value, 0); +result = it.next(); +assertEq(result.done, true); +assertEq(result.value, "ok"); diff --git a/js/src/jit/BaselineCompiler.cpp b/js/src/jit/BaselineCompiler.cpp index df3a04f1c470..0f5414099732 100644 --- a/js/src/jit/BaselineCompiler.cpp +++ b/js/src/jit/BaselineCompiler.cpp @@ -4647,7 +4647,7 @@ BaselineCompiler::emit_JSOP_AWAIT() return emit_JSOP_YIELD(); } -typedef bool (*DebugAfterYieldFn)(JSContext*, BaselineFrame*); +typedef bool (*DebugAfterYieldFn)(JSContext*, BaselineFrame*, jsbytecode*, bool*); static const VMFunction DebugAfterYieldInfo = FunctionInfo(jit::DebugAfterYield, "DebugAfterYield"); @@ -4660,8 +4660,21 @@ BaselineCompiler::emit_JSOP_DEBUGAFTERYIELD() frame.assertSyncedStack(); masm.loadBaselineFramePtr(BaselineFrameReg, R0.scratchReg()); prepareVMCall(); + pushArg(ImmPtr(pc)); pushArg(R0.scratchReg()); - return callVM(DebugAfterYieldInfo); + if (!callVM(DebugAfterYieldInfo)) + return false; + + icEntries_.back().setFakeKind(ICEntry::Kind_DebugAfterYield); + + Label done; + masm.branchTest32(Assembler::Zero, ReturnReg, ReturnReg, &done); + { + masm.loadValue(frame.addressOfReturnValue(), JSReturnOperand); + masm.jump(&return_); + } + masm.bind(&done); + return true; } typedef bool (*FinalSuspendFn)(JSContext*, HandleObject, jsbytecode*); diff --git a/js/src/jit/BaselineDebugModeOSR.cpp b/js/src/jit/BaselineDebugModeOSR.cpp index fa8664b7d0e6..674ee3237c28 100644 --- a/js/src/jit/BaselineDebugModeOSR.cpp +++ b/js/src/jit/BaselineDebugModeOSR.cpp @@ -103,6 +103,7 @@ struct DebugModeOSREntry frameKind == ICEntry::Kind_EarlyStackCheck || frameKind == ICEntry::Kind_DebugTrap || frameKind == ICEntry::Kind_DebugPrologue || + frameKind == ICEntry::Kind_DebugAfterYield || frameKind == ICEntry::Kind_DebugEpilogue; } @@ -307,6 +308,8 @@ ICEntryKindToString(ICEntry::Kind kind) return "debug trap"; case ICEntry::Kind_DebugPrologue: return "debug prologue"; + case ICEntry::Kind_DebugAfterYield: + return "debug after yield"; case ICEntry::Kind_DebugEpilogue: return "debug epilogue"; default: @@ -367,6 +370,7 @@ PatchBaselineFramesForDebugMode(JSContext* cx, // - All the ways above. // C. From the debug trap handler. // D. From the debug prologue. + // K. From a JSOP_DEBUGAFTERYIELD instruction. // E. From the debug epilogue. // // Cycles (On to Off to On)+ or (Off to On to Off)+: @@ -470,6 +474,7 @@ PatchBaselineFramesForDebugMode(JSContext* cx, kind == ICEntry::Kind_EarlyStackCheck || kind == ICEntry::Kind_DebugTrap || kind == ICEntry::Kind_DebugPrologue || + kind == ICEntry::Kind_DebugAfterYield || kind == ICEntry::Kind_DebugEpilogue); // We will have allocated a new recompile info, so delete the @@ -546,6 +551,17 @@ PatchBaselineFramesForDebugMode(JSContext* cx, popFrameReg = true; break; + case ICEntry::Kind_DebugAfterYield: + // Case K above. + // + // Resume at the next instruction. + MOZ_ASSERT(*pc == JSOP_DEBUGAFTERYIELD); + recompInfo->resumeAddr = bl->nativeCodeForPC(script, + pc + JSOP_DEBUGAFTERYIELD_LENGTH, + &recompInfo->slotInfo); + popFrameReg = true; + break; + default: // Case E above. // @@ -945,9 +961,9 @@ HasForcedReturn(BaselineDebugModeOSRInfo* info, bool rv) if (kind == ICEntry::Kind_DebugEpilogue) return true; - // |rv| is the value in ReturnReg. If true, in the case of the prologue, - // it means a forced return. - if (kind == ICEntry::Kind_DebugPrologue) + // |rv| is the value in ReturnReg. If true, in the case of the prologue or + // after yield, it means a forced return. + if (kind == ICEntry::Kind_DebugPrologue || kind == ICEntry::Kind_DebugAfterYield) return rv; // N.B. The debug trap handler handles its own forced return, so no diff --git a/js/src/jit/SharedIC.h b/js/src/jit/SharedIC.h index da4380d1cec5..17e06668691b 100644 --- a/js/src/jit/SharedIC.h +++ b/js/src/jit/SharedIC.h @@ -256,8 +256,9 @@ class ICEntry Kind_DebugTrap, // A fake IC entry for returning from a callVM to - // Debug{Prologue,Epilogue}. + // Debug{Prologue,AfterYield,Epilogue}. Kind_DebugPrologue, + Kind_DebugAfterYield, Kind_DebugEpilogue, Kind_Invalid diff --git a/js/src/jit/VMFunctions.cpp b/js/src/jit/VMFunctions.cpp index 8b91a778cd43..d170b0ee3c41 100644 --- a/js/src/jit/VMFunctions.cpp +++ b/js/src/jit/VMFunctions.cpp @@ -957,12 +957,19 @@ InterpretResume(JSContext* cx, HandleObject obj, HandleValue val, HandleProperty } bool -DebugAfterYield(JSContext* cx, BaselineFrame* frame) +DebugAfterYield(JSContext* cx, BaselineFrame* frame, jsbytecode* pc, bool* mustReturn) { + *mustReturn = false; + // The BaselineFrame has just been constructed by JSOP_RESUME in the // caller. We need to set its debuggee flag as necessary. - if (frame->script()->isDebuggee()) + // + // If a breakpoint is set on JSOP_DEBUGAFTERYIELD, or stepping is enabled, + // we may already have done this work. Don't fire onEnterFrame again. + if (frame->script()->isDebuggee() && !frame->isDebuggee()) { frame->setIsDebuggee(); + return DebugPrologue(cx, frame, pc, mustReturn); + } return true; } @@ -975,13 +982,19 @@ GeneratorThrowOrReturn(JSContext* cx, BaselineFrame* frame, Handlescript(); uint32_t offset = script->yieldAndAwaitOffsets()[genObj->yieldAndAwaitIndex()]; - frame->setOverridePc(script->offsetToPC(offset)); + jsbytecode* pc = script->offsetToPC(offset); + frame->setOverridePc(pc); // In the interpreter, GeneratorObject::resume marks the generator as running, // so we do the same. genObj->setRunning(); - MOZ_ALWAYS_TRUE(DebugAfterYield(cx, frame)); + bool mustReturn = false; + if (!DebugAfterYield(cx, frame, pc, &mustReturn)) + return false; + if (mustReturn) + resumeKind = GeneratorObject::RETURN; + MOZ_ALWAYS_FALSE(js::GeneratorThrowOrReturn(cx, frame, genObj, arg, resumeKind)); return false; } @@ -1091,11 +1104,15 @@ HandleDebugTrap(JSContext* cx, BaselineFrame* frame, uint8_t* retAddr, bool* mus jsbytecode* pc = script->baselineScript()->icEntryFromReturnAddress(retAddr).pc(script); if (*pc == JSOP_DEBUGAFTERYIELD) { - // JSOP_DEBUGAFTERYIELD will set the frame's debuggee flag, but if we - // set a breakpoint there we have to do it now. + // JSOP_DEBUGAFTERYIELD will set the frame's debuggee flag and call the + // onEnterFrame handler, but if we set a breakpoint there we have to do + // it now. MOZ_ASSERT(!frame->isDebuggee()); - if (!DebugAfterYield(cx, frame)) + + if (!DebugAfterYield(cx, frame, pc, mustReturn)) return false; + if (*mustReturn) + return true; } MOZ_ASSERT(frame->isDebuggee()); diff --git a/js/src/jit/VMFunctions.h b/js/src/jit/VMFunctions.h index 85722eef583a..ae024af85377 100644 --- a/js/src/jit/VMFunctions.h +++ b/js/src/jit/VMFunctions.h @@ -785,7 +785,7 @@ MOZ_MUST_USE bool InterpretResume(JSContext* cx, HandleObject obj, HandleValue val, HandlePropertyName kind, MutableHandleValue rval); MOZ_MUST_USE bool -DebugAfterYield(JSContext* cx, BaselineFrame* frame); +DebugAfterYield(JSContext* cx, BaselineFrame* frame, jsbytecode* pc, bool* mustReturn); MOZ_MUST_USE bool GeneratorThrowOrReturn(JSContext* cx, BaselineFrame* frame, Handle genObj, HandleValue arg, uint32_t resumeKind); diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index 79c5752b802f..28aa12bb123f 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -1817,6 +1817,15 @@ Debugger::fireEnterFrame(JSContext* cx, MutableHandleValue vp) RootedValue scriptFrame(cx); FrameIter iter(cx); + +#if DEBUG + // Assert that the hook won't be able to re-enter the generator. + if (iter.hasScript() && *iter.pc() == JSOP_DEBUGAFTERYIELD) { + GeneratorObject* genObj = GetGeneratorObjectForFrame(cx, iter.abstractFramePtr()); + MOZ_ASSERT(genObj->isRunning() || genObj->isClosing()); + } +#endif + if (!getFrame(cx, iter, &scriptFrame)) return reportUncaughtException(ar); diff --git a/js/src/vm/GeneratorObject.h b/js/src/vm/GeneratorObject.h index d09510021117..d74ab5e1acc1 100644 --- a/js/src/vm/GeneratorObject.h +++ b/js/src/vm/GeneratorObject.h @@ -154,6 +154,10 @@ class GeneratorObject : public NativeObject MOZ_ASSERT_IF(yieldAndAwaitIndex == 0, getFixedSlot(YIELD_AND_AWAIT_INDEX_SLOT).isUndefined()); MOZ_ASSERT_IF(yieldAndAwaitIndex != 0, isRunning() || isClosing()); + setYieldAndAwaitIndexNoAssert(yieldAndAwaitIndex); + } + // Debugger has to flout the state machine rules a bit. + void setYieldAndAwaitIndexNoAssert(uint32_t yieldAndAwaitIndex) { MOZ_ASSERT(yieldAndAwaitIndex < uint32_t(YIELD_AND_AWAIT_INDEX_CLOSING)); setFixedSlot(YIELD_AND_AWAIT_INDEX_SLOT, Int32Value(yieldAndAwaitIndex)); MOZ_ASSERT(isSuspended()); diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index 468cb1033766..593c2e7ca1b3 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -4286,6 +4286,20 @@ CASE(JSOP_RESUME) TraceLogStartEvent(logger, scriptEvent); TraceLogStartEvent(logger, TraceLogger_Interpreter); + switch (Debugger::onEnterFrame(cx, REGS.fp())) { + case ResumeMode::Continue: + break; + case ResumeMode::Throw: + case ResumeMode::Terminate: + goto error; + case ResumeMode::Return: + MOZ_ASSERT_IF(REGS.fp()->callee().isGenerator(), // as opposed to an async function + gen->isClosed()); + if (!ForcedReturn(cx, REGS)) + goto error; + goto successful_return_continuation; + } + switch (resumeKind) { case GeneratorObject::NEXT: break; From 7164db9da73e7264ee4695983cdda9aca568c4c4 Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Tue, 21 Aug 2018 14:34:50 -0500 Subject: [PATCH 33/45] Bug 1484943 - Don't assert when trying to formatToParts a NaN value whose sign bit is set. r=anba --HG-- extra : rebase_source : fb0ef69a0e6611cbeda719eccabd95f2abcdec1a --- js/src/builtin/intl/NumberFormat.cpp | 22 +++++++++--- .../Intl/NumberFormat/formatting-NaN.js | 35 +++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) create mode 100644 js/src/tests/non262/Intl/NumberFormat/formatting-NaN.js diff --git a/js/src/builtin/intl/NumberFormat.cpp b/js/src/builtin/intl/NumberFormat.cpp index 5e7e80419760..f6df12d93d72 100644 --- a/js/src/builtin/intl/NumberFormat.cpp +++ b/js/src/builtin/intl/NumberFormat.cpp @@ -35,6 +35,7 @@ using mozilla::AssertedCast; using mozilla::IsFinite; using mozilla::IsNaN; using mozilla::IsNegative; +using mozilla::SpecificNaN; using js::intl::CallICU; using js::intl::DateTimeFormatOptions; @@ -376,11 +377,17 @@ NewUNumberFormat(JSContext* cx, Handle numberFormat) } static JSString* -PartitionNumberPattern(JSContext* cx, UNumberFormat* nf, double x, +PartitionNumberPattern(JSContext* cx, UNumberFormat* nf, double* x, UFieldPositionIterator* fpositer) { - return CallICU(cx, [nf, x, fpositer](UChar* chars, int32_t size, UErrorCode* status) { - return unum_formatDoubleForFields(nf, x, chars, size, fpositer, status); + // ICU incorrectly formats NaN values with the sign bit set, as if they + // were negative. Replace all NaNs with a single pattern with sign bit + // unset ("positive", that is) until ICU is fixed. + if (MOZ_UNLIKELY(IsNaN(*x))) + *x = SpecificNaN(0, 1); + + return CallICU(cx, [nf, d = *x, fpositer](UChar* chars, int32_t size, UErrorCode* status) { + return unum_formatDoubleForFields(nf, d, chars, size, fpositer, status); }); } @@ -389,7 +396,7 @@ intl_FormatNumber(JSContext* cx, UNumberFormat* nf, double x, MutableHandleValue { // Passing null for |fpositer| will just not compute partition information, // letting us common up all ICU number-formatting code. - JSString* str = PartitionNumberPattern(cx, nf, x, nullptr); + JSString* str = PartitionNumberPattern(cx, nf, &x, nullptr); if (!str) return false; @@ -428,6 +435,11 @@ GetFieldTypeForNumberField(UNumberFormatFields fieldName, double d) // Manual trawling through the ICU call graph appears to indicate that // the basic formatting we request will never include a positive sign. // But this analysis may be mistaken, so don't absolutely trust it. + MOZ_ASSERT(!IsNaN(d), + "ICU appearing not to produce positive-sign among fields, " + "plus our coercing all NaNs to one with sign bit unset " + "(i.e. \"positive\"), means we shouldn't reach here with a " + "NaN value"); return IsNegative(d) ? &JSAtomState::minusSign : &JSAtomState::plusSign; } @@ -478,7 +490,7 @@ intl_FormatNumberToParts(JSContext* cx, UNumberFormat* nf, double x, MutableHand MOZ_ASSERT(fpositer); ScopedICUObject toClose(fpositer); - RootedString overallResult(cx, PartitionNumberPattern(cx, nf, x, fpositer)); + RootedString overallResult(cx, PartitionNumberPattern(cx, nf, &x, fpositer)); if (!overallResult) return false; diff --git a/js/src/tests/non262/Intl/NumberFormat/formatting-NaN.js b/js/src/tests/non262/Intl/NumberFormat/formatting-NaN.js new file mode 100644 index 000000000000..430d74222e33 --- /dev/null +++ b/js/src/tests/non262/Intl/NumberFormat/formatting-NaN.js @@ -0,0 +1,35 @@ +// |reftest| skip-if(!this.hasOwnProperty("Intl")) +// Any copyright is dedicated to the Public Domain. +// http://creativecommons.org/licenses/publicdomain/ + +//----------------------------------------------------------------------------- +var BUGNUMBER = 1484943; +var summary = "Don't crash doing format/formatToParts on -NaN"; + +print(BUGNUMBER + ": " + summary); + +//----------------------------------------------------------------------------- + +assertEq("formatToParts" in Intl.NumberFormat(), true); + +var nf = new Intl.NumberFormat("en-US"); +var parts; + +var values = [NaN, -NaN]; + +for (var v of values) +{ + assertEq(nf.format(v), "NaN"); + + parts = nf.formatToParts(v); + assertEq(parts.length, 1); + assertEq(parts[0].type, "nan"); + assertEq(parts[0].value, "NaN"); +} + +//----------------------------------------------------------------------------- + +if (typeof reportCompare === "function") + reportCompare(0, 0, 'ok'); + +print("Tests complete"); From 589e40bc4ce1e2fdd17cb61073864b50714372df Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Wed, 22 Aug 2018 13:12:50 -0500 Subject: [PATCH 34/45] Bug 1485119 - Don't pass the result of ValueToPrintableUTF8 to a function called JS_ReportErrorASCII. r=anba --HG-- extra : rebase_source : 325264b507d2b3eb7ffb817f28c40042bc891a5f --- js/src/shell/js.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 0ffc17b336be..11edf64524a1 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -4477,7 +4477,10 @@ BinParse(JSContext* cx, unsigned argc, Value* vp) useMultipart = false; } else { JSAutoByteString printable; - JS_ReportErrorASCII(cx, "Unknown value for option `format`, expected 'multipart' or 'simple', got %s", ValueToPrintableUTF8(cx, optionFormat, &printable)); + JS_ReportErrorUTF8(cx, + "Unknown value for option `format`, expected 'multipart' or " + "'simple', got %s", + ValueToPrintableUTF8(cx, optionFormat, &printable)); return false; } } else { From edf3cdbd07253126512f22563210e94fdc1c06f1 Mon Sep 17 00:00:00 2001 From: Anny Gakhokidze Date: Mon, 18 Jun 2018 23:43:31 -0400 Subject: [PATCH 35/45] Bug 1453153 - Initial removal of moz* APIs in DataTransfer, r=enndeakin,r=nika In DataTransfer, change mozItemCount, mozTypesAt, mozClearDataAt, mozSetDataAt, mozGetDataAt APIs to be ChromeOnly. MozReview-Commit-ID: 9uJ9ncrcBL2 --- dom/base/test/test_bug574596.html | 1 + dom/events/DataTransfer.cpp | 19 +++++++++++ dom/events/DataTransfer.h | 5 +++ .../test/test_DataTransferItemList.html | 4 +-- dom/events/test/test_bug1264380.html | 2 +- dom/events/test/test_dragstart.html | 32 +++++++++---------- dom/tests/mochitest/general/mochitest.ini | 1 + .../general/test_clipboard_disallowed.html | 14 ++------ .../general/test_clipboard_events.html | 24 +++++++------- .../general/test_datatransfer_disallowed.html | 31 ++++++++++++++++++ dom/webidl/DataTransfer.webidl | 10 +++--- modules/libpref/init/all.js | 7 ++++ .../mochitest/tests/SimpleTest/EventUtils.js | 2 +- 13 files changed, 104 insertions(+), 48 deletions(-) create mode 100644 dom/tests/mochitest/general/test_datatransfer_disallowed.html diff --git a/dom/base/test/test_bug574596.html b/dom/base/test/test_bug574596.html index 0978f4333d38..af8f5720daa6 100644 --- a/dom/base/test/test_bug574596.html +++ b/dom/base/test/test_bug574596.html @@ -42,6 +42,7 @@ var dragLinkText = [[ function dumpTransfer(dataTransfer,expect) { + dataTransfer = SpecialPowers.wrap(dataTransfer); dtData = dataTransfer.mozItemCount + "items:\n"; for (var i = 0; i < dataTransfer.mozItemCount; i++) { var dtTypes = dataTransfer.mozTypesAt(i); diff --git a/dom/events/DataTransfer.cpp b/dom/events/DataTransfer.cpp index 24d5673d451a..85da8a06af67 100644 --- a/dom/events/DataTransfer.cpp +++ b/dom/events/DataTransfer.cpp @@ -40,6 +40,8 @@ #include "mozilla/dom/Promise.h" #include "nsNetUtil.h" +#define MOZ_CALLS_ENABLED_PREF "dom.datatransfer.mozAtAPIs" + namespace mozilla { namespace dom { @@ -1523,5 +1525,22 @@ DataTransfer::SetMode(DataTransfer::Mode aMode) } } +/* static */ +bool +DataTransfer::MozAtAPIsEnabled(JSContext* aCx, JSObject* aObj /*unused*/) +{ + // Read the pref + static bool sPrefCached = false; + static bool sPrefCacheValue = false; + + if (!sPrefCached) { + sPrefCached = true; + Preferences::AddBoolVarCache(&sPrefCacheValue, MOZ_CALLS_ENABLED_PREF); + } + + // We can expose moz* APIs if we are chrome code or if pref is enabled + return nsContentUtils::IsSystemCaller(aCx) || sPrefCacheValue; +} + } // namespace dom } // namespace mozilla diff --git a/dom/events/DataTransfer.h b/dom/events/DataTransfer.h index 6b58996bda91..a83ab68403f3 100644 --- a/dom/events/DataTransfer.h +++ b/dom/events/DataTransfer.h @@ -428,6 +428,11 @@ public: const bool& aPlainTextOnly, nsTArray* aResult); + // Returns true if moz* APIs should be exposed (true for chrome code or if + // dom.datatransfer.moz pref is enabled). + // The affected moz* APIs are mozItemCount, mozTypesAt, mozClearDataAt, mozSetDataAt, mozGetDataAt + static bool MozAtAPIsEnabled(JSContext* cx, JSObject* obj); + protected: // caches text and uri-list data formats that exist in the drag service or diff --git a/dom/events/test/test_DataTransferItemList.html b/dom/events/test/test_DataTransferItemList.html index e0866c8abdba..034eae2fde83 100644 --- a/dom/events/test/test_DataTransferItemList.html +++ b/dom/events/test/test_DataTransferItemList.html @@ -1,6 +1,6 @@ - Tests for the DatTransferItemList object + Tests for the DataTransferItemList object @@ -10,7 +10,7 @@

      -
      +
      drag over here
      diff --git a/dom/events/test/test_bug1264380.html b/dom/events/test/test_bug1264380.html index 8c166ae4c731..fe5331500364 100644 --- a/dom/events/test/test_bug1264380.html +++ b/dom/events/test/test_bug1264380.html @@ -30,7 +30,7 @@ function runTests() ok(true, "Got dragstart event"); dataTransfer = event.dataTransfer; ok(dataTransfer, "DataTransfer object is available."); - is(dataTransfer.mozItemCount, 1, "initial link item count"); + is(SpecialPowers.wrap(dataTransfer).mozItemCount, 1, "initial link item count"); is(dataTransfer.getData("text/uri-list"), "http://www.mozilla.org/", "link text/uri-list"); is(dataTransfer.getData("text/plain"), "http://www.mozilla.org/", "link text/plain"); } diff --git a/dom/events/test/test_dragstart.html b/dom/events/test/test_dragstart.html index 8154ddaabce9..93192487127f 100644 --- a/dom/events/test/test_dragstart.html +++ b/dom/events/test/test_dragstart.html @@ -41,10 +41,6 @@ function afterDragTests() "NoModificationAllowedError", "setData when read only"); expectError(() => gDataTransfer.clearData("text/plain"), "NoModificationAllowedError", "clearData when read only"); - expectError(() => gDataTransfer.mozSetDataAt("text/plain", "Some Text", 0), - "NoModificationAllowedError", "setDataAt when read only"); - expectError(() => gDataTransfer.mozClearDataAt("text/plain", 0), - "NoModificationAllowedError", "clearDataAt when read only"); expectError(() => gDataTransfer.addElement(draggable), "NoModificationAllowedError", "addElement when read only"); @@ -130,7 +126,7 @@ function doDragStartSelection(event) is(dt.getData("TEXT"), "This is a draggable bit of text.", "initial selection TEXT"); is(dt.getData("text/UNICODE"), "This is a draggable bit of text.", "initial selection text/UNICODE"); - is(dt.mozItemCount, 1, "initial selection item count"); + is(SpecialPowers.wrap(dt).mozItemCount, 1, "initial selection item count"); dt.clearData("text/plain"); dt.clearData("text/html"); @@ -143,10 +139,14 @@ function doDragStartSelection(event) function test_DataTransfer(dt) { - is(dt.mozItemCount, 0, "empty itemCount"); + is(SpecialPowers.wrap(dt).mozItemCount, 0, "empty itemCount"); var types = dt.types; ok(Array.isArray(types), "empty types is an Array"); + // The above test fails if we have SpecialPowers.wrap(dt).types instead of dt.types + // because chrome consumers get the 'ReturnValueNeedsContainsHack'. + // So wrap with special powers after the test + dt = SpecialPowers.wrap(dt); checkTypes(dt, [], 0, "empty"); is(dt.getData("text/plain"), "", "empty data is empty"); @@ -316,11 +316,9 @@ function test_DataTransfer(dt) checkOneDataItem(dt, ["application/-moz-node"], ["draggable"], 2, "setDataAt node item at index 2"); // Try to add and then remove a non-string type to the DataTransfer and ensure - // that the type appears in DataTransfer.types. These calls need to be called - // with SpecialPowers.wrap(), as adding and removing non-string/file types to - // DataTransfer is chrome-only. + // that the type appears in DataTransfer.types. { - SpecialPowers.wrap(dt).mozSetDataAt("application/-x-body", document.body, 0); + dt.mozSetDataAt("application/-x-body", document.body, 0); let found = false; for (let i = 0; i < dt.types.length; ++i) { if (dt.types[i] == "application/-x-body") { @@ -329,7 +327,7 @@ function test_DataTransfer(dt) } } ok(found, "Data should appear in datatransfer.types despite having a non-string type"); - SpecialPowers.wrap(dt).mozClearDataAt("application/-x-body", 0); + dt.mozClearDataAt("application/-x-body", 0); } dt.mozClearDataAt("text/html", 1); @@ -419,7 +417,7 @@ function doDragStartLink(event) checkTypes(dt, ["text/x-moz-url", "text/x-moz-url-data", "text/x-moz-url-desc", "text/uri-list", "text/_moz_htmlcontext", "text/_moz_htmlinfo", "text/html", "text/plain"], 0, "initial link"); - is(dt.mozItemCount, 1, "initial link item count"); + is(SpecialPowers.wrap(dt).mozItemCount, 1, "initial link item count"); is(dt.getData("text/uri-list"), "http://www.mozilla.org/", "link text/uri-list"); is(dt.getData("text/plain"), "http://www.mozilla.org/", "link text/plain"); @@ -436,7 +434,7 @@ function doDragStartImage(event) checkTypes(dt, ["text/x-moz-url", "text/x-moz-url-data", "text/x-moz-url-desc", "text/uri-list", "text/_moz_htmlcontext", "text/_moz_htmlinfo", "text/html", "text/plain"], 0, "initial image"); - is(dt.mozItemCount, 1, "initial image item count"); + is(SpecialPowers.wrap(dt).mozItemCount, 1, "initial image item count"); is(dt.getData("text/uri-list"), dataurl, "image text/uri-list"); is(dt.getData("text/plain"), dataurl, "image text/plain"); @@ -450,7 +448,7 @@ function doDragStartInput(event) var dt = event.dataTransfer; checkTypes(dt, ["text/plain"], 0, "initial input"); - is(dt.mozItemCount, 1, "initial input item count"); + is(SpecialPowers.wrap(dt).mozItemCount, 1, "initial input item count"); // is(dt.getData("text/plain"), "Text", "input text/plain"); // event.preventDefault(); @@ -511,11 +509,12 @@ function doDragOverSynthetic(event) function onDragStartDraggable(event) { var dt = event.dataTransfer; - ok(dt.mozItemCount == 0 && dt.types.length == 0 && event.originalTarget == gDragInfo.target, gDragInfo.testid); + ok(SpecialPowers.wrap(dt).mozItemCount == 0 && dt.types.length == 0 && event.originalTarget == gDragInfo.target, gDragInfo.testid); gExtraDragTests++; } +// Expects dt wrapped in SpecialPowers function checkOneDataItem(dt, expectedtypes, expecteddata, index, testid) { checkTypes(dt, expectedtypes, index, testid); @@ -537,13 +536,14 @@ function checkTypes(dt, expectedtypes, index, testid) } } - types = dt.mozTypesAt(index); + types = SpecialPowers.wrap(dt).mozTypesAt(index); is(types.length, expectedtypes.length, testid + " typesAt length"); for (var f = 0; f < expectedtypes.length; f++) { is(types[f], expectedtypes[f], testid + " " + types[f] + " at " + index + " check"); } } +// Expects dt wrapped in SpecialPowers function checkURL(dt, url, fullurllist, index, testid) { is(dt.getData("text/uri-list"), fullurllist, testid + " text/uri-list"); diff --git a/dom/tests/mochitest/general/mochitest.ini b/dom/tests/mochitest/general/mochitest.ini index 814c39d7cb18..a7ee843b4f2e 100644 --- a/dom/tests/mochitest/general/mochitest.ini +++ b/dom/tests/mochitest/general/mochitest.ini @@ -90,6 +90,7 @@ subsuite = clipboard [test_consoleAPI.html] [test_contentViewer_overrideDPPX.html] [test_CCW_optimization.html] +[test_datatransfer_disallowed.html] [test_devicePixelRatio_with_zoom.html] [test_DOMMatrix.html] [test_domWindowUtils.html] diff --git a/dom/tests/mochitest/general/test_clipboard_disallowed.html b/dom/tests/mochitest/general/test_clipboard_disallowed.html index 71ba09d62426..d42f3c9b0f71 100644 --- a/dom/tests/mochitest/general/test_clipboard_disallowed.html +++ b/dom/tests/mochitest/general/test_clipboard_disallowed.html @@ -24,15 +24,7 @@ function checkAllowed(event) let exception; try { - clipboardData.mozSetDataAt("text/customdata", document.getElementById("input"), 0); - } catch(ex) { - exception = ex; - } - is(String(exception).indexOf("SecurityError"), 0, "Cannot set non-string"); - - exception = null; - try { - clipboardData.mozSetDataAt("application/x-moz-file", "Test", 0); + clipboardData.setData("application/x-moz-file", "Test"); } catch(ex) { exception = ex; } @@ -40,7 +32,7 @@ function checkAllowed(event) exception = null; try { - clipboardData.mozSetDataAt("application/x-moz-file-promise", "Test", 0); + clipboardData.setData("application/x-moz-file-promise", "Test"); } catch(ex) { exception = ex; } @@ -48,7 +40,7 @@ function checkAllowed(event) exception = null; try { - clipboardData.mozSetDataAt("application/something", "This is data", 0); + clipboardData.setData("application/something", "This is data"); } catch(ex) { exception = ex; } diff --git a/dom/tests/mochitest/general/test_clipboard_events.html b/dom/tests/mochitest/general/test_clipboard_events.html index e952ebe9a618..b107d06abe74 100644 --- a/dom/tests/mochitest/general/test_clipboard_events.html +++ b/dom/tests/mochitest/general/test_clipboard_events.html @@ -302,7 +302,7 @@ add_task(async function test_input_cut_dataTransfer() { ok(event instanceof ClipboardEvent, "cut event is a ClipboardEvent"); ok(event.clipboardData instanceof DataTransfer, "cut event dataTransfer is a DataTransfer"); is(event.target, contentInput, "cut event target"); - is(event.clipboardData.mozItemCount, 0, "cut event mozItemCount"); + is(SpecialPowers.wrap(event.clipboardData).mozItemCount, 0, "cut event mozItemCount"); is(event.clipboardData.getData("text/plain"), "", "cut event getData"); event.clipboardData.setData("text/plain", "This is some dataTransfer text"); cachedCutData = event.clipboardData; @@ -349,7 +349,7 @@ add_task(async function test_input_copy_dataTransfer() { ok(event instanceof ClipboardEvent, "copy event is a ClipboardEvent"); ok(event.clipboardData instanceof DataTransfer, "copy event dataTransfer is a DataTransfer"); is(event.target, contentInput, "copy event target"); - is(event.clipboardData.mozItemCount, 0, "copy event mozItemCount"); + is(SpecialPowers.wrap(event.clipboardData).mozItemCount, 0, "copy event mozItemCount"); is(event.clipboardData.getData("text/plain"), "", "copy event getData"); event.clipboardData.setData("text/plain", "Copied dataTransfer text"); cachedCopyData = event.clipboardData; @@ -394,7 +394,7 @@ add_task(async function test_input_paste_dataTransfer() { ok(event instanceof ClipboardEvent, "paste event is an ClipboardEvent"); ok(event.clipboardData instanceof DataTransfer, "paste event dataTransfer is a DataTransfer"); is(event.target, contentInput, "paste event target"); - is(event.clipboardData.mozItemCount, 1, "paste event mozItemCount"); + is(SpecialPowers.wrap(event.clipboardData).mozItemCount, 1, "paste event mozItemCount"); is(event.clipboardData.getData("text/plain"), clipboardInitialValue, "paste event getData"); cachedPasteData = event.clipboardData; }; @@ -440,21 +440,21 @@ add_task(async function test_input_copypaste_dataTransfer_multiple() { cd.setData("text/plain", "would be a phrase"); var exh = false; - try { cd.mozSetDataAt("text/plain", "Text", 1); } catch (ex) { exh = true; } + try { SpecialPowers.wrap(cd).mozSetDataAt("text/plain", "Text", 1); } catch (ex) { exh = true; } ok(exh, "exception occured mozSetDataAt 1"); exh = false; - try { cd.mozTypesAt(1); } catch (ex) { exh = true; } + try { SpecialPowers.wrap(cd).mozTypesAt(1); } catch (ex) { exh = true; } ok(exh, "exception occured mozTypesAt 1"); exh = false; - try { cd.mozGetDataAt("text/plain", 1); } catch (ex) { exh = true; } + try { SpecialPowers.wrap(cd).mozGetDataAt("text/plain", 1); } catch (ex) { exh = true; } ok(exh, "exception occured mozGetDataAt 1"); exh = false; try { cd.mozClearDataAt("text/plain", 1); } catch (ex) { exh = true; } ok(exh, "exception occured mozClearDataAt 1"); cd.setData("text/x-moz-url", "http://www.mozilla.org"); - cd.mozSetDataAt("text/x-custom", "Custom Text with \u0000 null", 0); - is(cd.mozItemCount, 1, "mozItemCount after set multiple types"); + SpecialPowers.wrap(cd).mozSetDataAt("text/x-custom", "Custom Text with \u0000 null", 0); + is(SpecialPowers.wrap(cd).mozItemCount, 1, "mozItemCount after set multiple types"); return false; }; @@ -473,7 +473,7 @@ add_task(async function test_input_copypaste_dataTransfer_multiple() { contentInput.onpaste = function(event) { var cd = event.clipboardData; - is(cd.mozItemCount, 1, "paste after copy multiple types mozItemCount"); + is(SpecialPowers.wrap(cd).mozItemCount, 1, "paste after copy multiple types mozItemCount"); is(cd.getData("text/plain"), "would be a phrase", "paste text/plain multiple types"); // Firefox for Android's clipboard code doesn't handle x-moz-url. Therefore @@ -640,10 +640,10 @@ function compareSynthetic(event, eventtype) { is(event.type, expectedData.type, "synthetic " + eventtype + " event type"); if (expectedData.data === null) { - is(event.clipboardData.mozItemCount, 0, "synthetic " + eventtype + " empty data"); + is(SpecialPowers.wrap(event.clipboardData).mozItemCount, 0, "synthetic " + eventtype + " empty data"); } else { - is(event.clipboardData.mozItemCount, 1, "synthetic " + eventtype + " item count"); + is(SpecialPowers.wrap(event.clipboardData).mozItemCount, 1, "synthetic " + eventtype + " item count"); is(event.clipboardData.types.length, 1, "synthetic " + eventtype + " types length"); is(event.clipboardData.getData(expectedData.dataType), expectedData.data, "synthetic " + eventtype + " data"); @@ -666,7 +666,7 @@ async function checkCachedDataTransfer(cd, eventtype) { ok(!oldtext, "clipboard get using " + testprefix); try { - cd.mozSetDataAt("text/plain", "Test Cache Data", 0); + SpecialPowers.wrap(cd).mozSetDataAt("text/plain", "Test Cache Data", 0); } catch (ex) {} ok(!cd.getData("text/plain"), "clipboard set using " + testprefix); diff --git a/dom/tests/mochitest/general/test_datatransfer_disallowed.html b/dom/tests/mochitest/general/test_datatransfer_disallowed.html new file mode 100644 index 000000000000..726db26a870c --- /dev/null +++ b/dom/tests/mochitest/general/test_datatransfer_disallowed.html @@ -0,0 +1,31 @@ + + + + Test for DataTransfer moz* APIs + + + + + + diff --git a/dom/webidl/DataTransfer.webidl b/dom/webidl/DataTransfer.webidl index 0fbec23ad04a..d880cdfbb5d3 100644 --- a/dom/webidl/DataTransfer.webidl +++ b/dom/webidl/DataTransfer.webidl @@ -54,7 +54,7 @@ partial interface DataTransfer { /** * The number of items being dragged. */ - [UseCounter] + [Func="DataTransfer::MozAtAPIsEnabled"] readonly attribute unsigned long mozItemCount; /** @@ -77,7 +77,7 @@ partial interface DataTransfer { * at the specified index. If the index is not in the range from 0 to * itemCount - 1, an empty string list is returned. */ - [Throws, NeedsCallerType, UseCounter] + [Throws, NeedsCallerType, Func="DataTransfer::MozAtAPIsEnabled"] DOMStringList mozTypesAt(unsigned long index); /** @@ -94,7 +94,7 @@ partial interface DataTransfer { * @throws NS_ERROR_DOM_INDEX_SIZE_ERR if index is greater or equal than itemCount * @throws NO_MODIFICATION_ALLOWED_ERR if the item cannot be modified */ - [Throws, NeedsSubjectPrincipal, UseCounter] + [Throws, NeedsSubjectPrincipal, Func="DataTransfer::MozAtAPIsEnabled"] void mozClearDataAt(DOMString format, unsigned long index); /* @@ -118,7 +118,7 @@ partial interface DataTransfer { * @throws NS_ERROR_DOM_INDEX_SIZE_ERR if index is greater than itemCount * @throws NO_MODIFICATION_ALLOWED_ERR if the item cannot be modified */ - [Throws, NeedsSubjectPrincipal, UseCounter] + [Throws, NeedsSubjectPrincipal, Func="DataTransfer::MozAtAPIsEnabled"] void mozSetDataAt(DOMString format, any data, unsigned long index); /** @@ -130,7 +130,7 @@ partial interface DataTransfer { * @returns the data of the given format, or null if it doesn't exist. * @throws NS_ERROR_DOM_INDEX_SIZE_ERR if index is greater or equal than itemCount */ - [Throws, NeedsSubjectPrincipal, UseCounter] + [Throws, NeedsSubjectPrincipal, Func="DataTransfer::MozAtAPIsEnabled"] any mozGetDataAt(DOMString format, unsigned long index); /** diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index ec3dc475658b..7c68fdd48d77 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -5937,3 +5937,10 @@ pref("dom.events.asyncClipboard", true); pref("dom.events.asyncClipboard.dataTransfer", false); // Should only be enabled in tests pref("dom.events.testing.asyncClipboard", false); + +#ifdef NIGHTLY_BUILD +// Disable moz* APIs in DataTransfer +pref("dom.datatransfer.mozAtAPIs", false); +#else +pref("dom.datatransfer.mozAtAPIs", true); +#endif diff --git a/testing/mochitest/tests/SimpleTest/EventUtils.js b/testing/mochitest/tests/SimpleTest/EventUtils.js index 6fab6577d8ac..5707a4eb77aa 100644 --- a/testing/mochitest/tests/SimpleTest/EventUtils.js +++ b/testing/mochitest/tests/SimpleTest/EventUtils.js @@ -2584,7 +2584,7 @@ function synthesizeDragOver(aSrcElement, aDestElement, aDragData, aDropEffect, a for (var i = 0; i < aDragData.length; i++) { var item = aDragData[i]; for (var j = 0; j < item.length; j++) { - event.dataTransfer.mozSetDataAt(item[j].type, item[j].data, i); + _EU_maybeWrap(event.dataTransfer).mozSetDataAt(item[j].type, item[j].data, i); } } } From c1225b09670e4a989dc95e7359216c47a7b5d02a Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Thu, 16 Aug 2018 11:35:50 -0700 Subject: [PATCH 36/45] Bug 1483865: Make NotStackAllocated assertion a bit safer. r=froydnj This assertion was always meant to be a best-effort thing to catch obvious errors, but the cases where the assumptions it makes fail have been growing. I could remove it entirely, but I'd be happier keeping at least some basic sanity checks. This compromise continues allowing any address below the first argument pointer, and extends the assertion to also allow anything more than 2KiB above it. We could probably get away with stretching that to at least 4KiB, but 2 seems safer, and likely enough to catch the obvious cases. Differential Revision: https://phabricator.services.mozilla.com/D3542 --HG-- extra : source : a5f51c76930c49160bf5e909301d8e7f1a83e379 extra : amend_source : e3decc44bfb4bed6696a394980c378dc033e1021 --- xpcom/components/nsComponentManager.cpp | 33 +++++++++++++++++++------ 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/xpcom/components/nsComponentManager.cpp b/xpcom/components/nsComponentManager.cpp index 654f42099154..7486d6964508 100644 --- a/xpcom/components/nsComponentManager.cpp +++ b/xpcom/components/nsComponentManager.cpp @@ -480,13 +480,32 @@ template static void AssertNotStackAllocated(T* aPtr) { - // The main thread's stack should be allocated at the top of our address - // space. Anything stack allocated should be above us on the stack, and - // therefore above our first argument pointer. - // Only this is apparently not the case on Windows. -#if !(defined(XP_WIN) || defined(ANDROID)) - MOZ_ASSERT(NS_IsMainThread()); - MOZ_ASSERT(uintptr_t(aPtr) < uintptr_t(&aPtr)); + // On all of our supported platforms, the stack grows down. Any address + // located below the address of our argument is therefore guaranteed not to be + // stack-allocated by the caller. + // + // For addresses above our argument, things get trickier. The main thread + // stack is traditionally placed at the top of the program's address space, + // but that is becoming less reliable as more and more systems adopt address + // space layout randomization strategies, so we have to guess how much space + // above our argument pointer we need to care about. + // + // On most systems, we're guaranteed at least several KiB at the top of each + // stack for TLS. We'd probably be safe assuming at least 4KiB in the stack + // segment above our argument address, but safer is... well, safer. + // + // For threads with huge stacks, it's theoretically possible that we could + // wind up being passed a stack-allocated string from farther up the stack, + // but this is a best-effort thing, so we'll assume we only care about the + // immediate caller. For that case, max 2KiB per stack frame is probably a + // reasonable guess most of the time, and is less than the ~4KiB that we + // expect for TLS, so go with that to avoid the risk of bumping into heap + // data just above the stack. +#ifdef DEBUG + static constexpr size_t kFuzz = 2048; + + MOZ_ASSERT(uintptr_t(aPtr) < uintptr_t(&aPtr) || + uintptr_t(aPtr) > uintptr_t(&aPtr) + kFuzz); #endif } From 84013bba1440d0e7f28dcf37ad3f118dbc220eca Mon Sep 17 00:00:00 2001 From: Andreas Pehrson Date: Mon, 20 Aug 2018 10:44:49 +0200 Subject: [PATCH 37/45] Bug 1478575 - Unify CamerasChild shutdown paths. r=gcp --- ipc/glue/BackgroundChildImpl.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/ipc/glue/BackgroundChildImpl.cpp b/ipc/glue/BackgroundChildImpl.cpp index 5a9d9e65ff78..75612378b51c 100644 --- a/ipc/glue/BackgroundChildImpl.cpp +++ b/ipc/glue/BackgroundChildImpl.cpp @@ -420,6 +420,7 @@ BackgroundChildImpl::DeallocPCamerasChild(camera::PCamerasChild *aActor) RefPtr child = dont_AddRef(static_cast(aActor)); MOZ_ASSERT(aActor); + camera::Shutdown(); #endif return true; } From d5f46f31e0b07a21eb8feeece4c59cb6661fe027 Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Thu, 9 Aug 2018 15:29:37 -0500 Subject: [PATCH 38/45] Bug 1479429 - Add a range check for the argument to Debugger.Script.prototype.get{Predecessor,Successor}Offsets. r=bhackett --HG-- extra : rebase_source : 2f60dff4108774e5828d3afa957817c3cc85563b --- js/src/jit-test/tests/debug/bug1479429.js | 15 +++++++++++++++ js/src/vm/Debugger.cpp | 7 +++++++ 2 files changed, 22 insertions(+) create mode 100644 js/src/jit-test/tests/debug/bug1479429.js diff --git a/js/src/jit-test/tests/debug/bug1479429.js b/js/src/jit-test/tests/debug/bug1479429.js new file mode 100644 index 000000000000..0c31d68312ed --- /dev/null +++ b/js/src/jit-test/tests/debug/bug1479429.js @@ -0,0 +1,15 @@ +// Bug 1479429 - Methods throw on out-of-range bytecode offsets. + +load(libdir + "asserts.js"); + +var g = newGlobal(); +var dbg = Debugger(g); +dbg.onDebuggerStatement = function(frame) { + assertThrowsInstanceOf( + () => frame.script.getPredecessorOffsets(0x400000), + TypeError); + assertThrowsInstanceOf( + () => frame.script.getSuccessorOffsets(-1), + TypeError); +} +g.eval("debugger;"); diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index 28aa12bb123f..2d90cfcb6f14 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -6104,8 +6104,13 @@ class DebuggerScriptGetSuccessorOrPredecessorOffsetsMatcher bool successor, MutableHandleObject result) : cx_(cx), offset_(offset), successor_(successor), result_(result) { } + using ReturnType = bool; + ReturnType match(HandleScript script) { + if (!EnsureScriptOffsetIsValid(cx_, script, offset_)) + return false; + PcVector adjacent; if (successor_) { if (!GetSuccessorBytecodes(script->code() + offset_, adjacent)) { @@ -6129,12 +6134,14 @@ class DebuggerScriptGetSuccessorOrPredecessorOffsetsMatcher } return true; } + ReturnType match(Handle lazyScript) { RootedScript script(cx_, DelazifyScript(cx_, lazyScript)); if (!script) return false; return match(script); } + ReturnType match(Handle instance) { JS_ReportErrorASCII(cx_, "getSuccessorOrPredecessorOffsets NYI on wasm instances"); return false; From b457cc25bd66199fae771eff5345d73dfd185b8e Mon Sep 17 00:00:00 2001 From: Lee Salzman Date: Wed, 22 Aug 2018 15:19:33 -0400 Subject: [PATCH 39/45] Bug 1483120 - sum SkDashPath intervals instead of subtracting. r=rhunt --- gfx/skia/skia/src/utils/SkDashPath.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/gfx/skia/skia/src/utils/SkDashPath.cpp b/gfx/skia/skia/src/utils/SkDashPath.cpp index e4840c84b6ea..39a30941f9a1 100644 --- a/gfx/skia/skia/src/utils/SkDashPath.cpp +++ b/gfx/skia/skia/src/utils/SkDashPath.cpp @@ -375,12 +375,13 @@ bool SkDashPath::InternalFilter(SkPath* dst, const SkPath& src, SkStrokeRec* rec SkScalar pathLength = SkPathMeasure(src, false, rec->getResScale()).getLength(); SkScalar endPhase = SkScalarMod(pathLength + initialDashLength, intervalLength); int index = 0; - while (endPhase > intervals[index]) { - endPhase -= intervals[index++]; + SkScalar sum = 0; + while (endPhase > sum + intervals[index]) { + sum += intervals[index++]; SkASSERT(index <= count); } // if dash ends inside "on", or ends at beginning of "off" - if (is_even(index) == (endPhase > 0)) { + if (is_even(index) == (endPhase > sum)) { SkPoint midPoint = src.getPoint(0); // get vector at end of rect int last = src.countPoints() - 1; From fd926c1cd2e74dc45a495f061a84803136961ea6 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Wed, 8 Aug 2018 11:02:18 -0700 Subject: [PATCH 40/45] Bug 1481897 - Remove HashPolicy template param from WeakMap, force to use MovableCellHasher instead, r=jonco --HG-- extra : rebase_source : c6babd16996b56517988d577197b572989bab6f2 --- js/src/gc/WeakMap-inl.h | 58 +++++++++++++++++++++------------------- js/src/gc/WeakMap.h | 13 ++++----- js/src/gc/WeakMapPtr.cpp | 3 +-- js/src/vm/Debugger.h | 5 ++-- 4 files changed, 38 insertions(+), 41 deletions(-) diff --git a/js/src/gc/WeakMap-inl.h b/js/src/gc/WeakMap-inl.h index 60cbdf3ae59a..10affa40b699 100644 --- a/js/src/gc/WeakMap-inl.h +++ b/js/src/gc/WeakMap-inl.h @@ -25,8 +25,8 @@ static T* extractUnbarriered(T* v) return v; } -template -WeakMap::WeakMap(JSContext* cx, JSObject* memOf) +template +WeakMap::WeakMap(JSContext* cx, JSObject* memOf) : Base(cx->zone()), WeakMapBase(memOf, cx->zone()) { zone()->gcWeakMapList().insertFront(this); @@ -40,9 +40,9 @@ WeakMap::WeakMap(JSContext* cx, JSObject* memOf) // This implementation does not use 'markedCell'; it looks up origKey and checks // the mark bits on everything it cares about, one of which will be // markedCell. But a subclass might use it to optimize the liveness check. -template +template void -WeakMap::markEntry(GCMarker* marker, gc::Cell* markedCell, JS::GCCellPtr origKey) +WeakMap::markEntry(GCMarker* marker, gc::Cell* markedCell, JS::GCCellPtr origKey) { MOZ_ASSERT(marked); @@ -64,9 +64,9 @@ WeakMap::markEntry(GCMarker* marker, gc::Cell* markedCell, JS::GCCellP key.unsafeSet(nullptr); // Prevent destructor from running barriers. } -template +template void -WeakMap::trace(JSTracer* trc) +WeakMap::trace(JSTracer* trc) { MOZ_ASSERT_IF(JS::RuntimeHeapIsBusy(), isInList()); @@ -94,9 +94,9 @@ WeakMap::trace(JSTracer* trc) TraceEdge(trc, &r.front().value(), "WeakMap entry value"); } -template +template /* static */ void -WeakMap::addWeakEntry(GCMarker* marker, JS::GCCellPtr key, +WeakMap::addWeakEntry(GCMarker* marker, JS::GCCellPtr key, const gc::WeakMarkable& markable) { Zone* zone = key.asCell()->asTenured().zone(); @@ -114,9 +114,9 @@ WeakMap::addWeakEntry(GCMarker* marker, JS::GCCellPtr key, } } -template +template bool -WeakMap::markIteratively(GCMarker* marker) +WeakMap::markIteratively(GCMarker* marker) { MOZ_ASSERT(marked); @@ -152,10 +152,12 @@ WeakMap::markIteratively(GCMarker* marker) return markedAny; } -template +template inline JSObject* -WeakMap::getDelegate(JSObject* key) const +WeakMap::getDelegate(JSObject* key) const { + JS::AutoSuppressGCAnalysis nogc; + JSWeakmapKeyDelegateOp op = key->getClass()->extWeakmapKeyDelegateOp(); if (!op) return nullptr; @@ -168,23 +170,23 @@ WeakMap::getDelegate(JSObject* key) const return obj; } -template +template inline JSObject* -WeakMap::getDelegate(JSScript* script) const +WeakMap::getDelegate(JSScript* script) const { return nullptr; } -template +template inline JSObject* -WeakMap::getDelegate(LazyScript* script) const +WeakMap::getDelegate(LazyScript* script) const { return nullptr; } -template +template inline bool -WeakMap::keyNeedsMark(JSObject* key) const +WeakMap::keyNeedsMark(JSObject* key) const { JSObject* delegate = getDelegate(key); /* @@ -194,24 +196,24 @@ WeakMap::keyNeedsMark(JSObject* key) const return delegate && gc::IsMarkedUnbarriered(zone()->runtimeFromMainThread(), &delegate); } -template +template inline bool -WeakMap::keyNeedsMark(JSScript* script) const +WeakMap::keyNeedsMark(JSScript* script) const { return false; } -template +template inline bool -WeakMap::keyNeedsMark(LazyScript* script) const +WeakMap::keyNeedsMark(LazyScript* script) const { return false; } -template +template void -WeakMap::sweep() +WeakMap::sweep() { /* Remove all entries whose keys remain unmarked. */ for (Enum e(*this); !e.empty(); e.popFront()) { @@ -227,9 +229,9 @@ WeakMap::sweep() } /* memberOf can be nullptr, which means that the map is not part of a JSObject. */ -template +template void -WeakMap::traceMappings(WeakMapTracer* tracer) +WeakMap::traceMappings(WeakMapTracer* tracer) { for (Range r = Base::all(); !r.empty(); r.popFront()) { gc::Cell* key = gc::ToMarkable(r.front().key()); @@ -243,9 +245,9 @@ WeakMap::traceMappings(WeakMapTracer* tracer) } #if DEBUG -template +template void -WeakMap::assertEntriesNotAboutToBeFinalized() +WeakMap::assertEntriesNotAboutToBeFinalized() { for (Range r = Base::all(); !r.empty(); r.popFront()) { K k(r.front().key()); diff --git a/js/src/gc/WeakMap.h b/js/src/gc/WeakMap.h index 5d6e289d0394..176482fb0e88 100644 --- a/js/src/gc/WeakMap.h +++ b/js/src/gc/WeakMap.h @@ -111,13 +111,12 @@ class WeakMapBase : public mozilla::LinkedListElement bool marked; }; -template > -class WeakMap : public HashMap, +template +class WeakMap : public HashMap, ZoneAllocPolicy>, public WeakMapBase { public: - typedef HashMap Base; + typedef HashMap, ZoneAllocPolicy> Base; typedef typename Base::Enum Enum; typedef typename Base::Lookup Lookup; typedef typename Base::Entry Entry; @@ -191,13 +190,11 @@ class WeakMap : public HashMap, }; -class ObjectValueMap : public WeakMap, HeapPtr, - MovableCellHasher>> +class ObjectValueMap : public WeakMap, HeapPtr> { public: ObjectValueMap(JSContext* cx, JSObject* obj) - : WeakMap, HeapPtr, - MovableCellHasher>>(cx, obj) + : WeakMap(cx, obj) {} bool findZoneEdges() override; diff --git a/js/src/gc/WeakMapPtr.cpp b/js/src/gc/WeakMapPtr.cpp index f6e8b8ae7c7b..fb1313a3d7d3 100644 --- a/js/src/gc/WeakMapPtr.cpp +++ b/js/src/gc/WeakMapPtr.cpp @@ -41,9 +41,8 @@ template struct Utils { typedef typename DataType::BarrieredType KeyType; - typedef typename DataType::HasherType HasherType; typedef typename DataType::BarrieredType ValueType; - typedef WeakMap Type; + typedef WeakMap Type; typedef Type* PtrType; static PtrType cast(void* ptr) { return static_cast(ptr); } }; diff --git a/js/src/vm/Debugger.h b/js/src/vm/Debugger.h index df5bfc205730..ff8f18e4a6b6 100644 --- a/js/src/vm/Debugger.h +++ b/js/src/vm/Debugger.h @@ -125,8 +125,7 @@ CheckDebuggeeThing(JSObject* obj, bool invisibleOk); * transitions. */ template -class DebuggerWeakMap : private WeakMap, HeapPtr, - MovableCellHasher>> +class DebuggerWeakMap : private WeakMap, HeapPtr> { private: typedef HeapPtr Key; @@ -141,7 +140,7 @@ class DebuggerWeakMap : private WeakMap, HeapPtr> Base; + typedef WeakMap Base; explicit DebuggerWeakMap(JSContext* cx) : Base(cx), From 71c1e0f58f12d47d21bf579f37936c8dbd93cf6c Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Thu, 16 Aug 2018 16:49:25 -0700 Subject: [PATCH 41/45] Bug 1470921 - Re-check whether nursery strings are enabled after collecting during allocation, r=jonco --HG-- extra : topic : nursery.strings.fix extra : rebase_source : 4befad86d2f96fcbf1d314b23434d0d4f935acac --- js/src/gc/Allocator.cpp | 5 +++-- js/src/jit/CodeGenerator.cpp | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/js/src/gc/Allocator.cpp b/js/src/gc/Allocator.cpp index 220318804f3a..528372e7c59b 100644 --- a/js/src/gc/Allocator.cpp +++ b/js/src/gc/Allocator.cpp @@ -151,8 +151,9 @@ GCRuntime::tryNewNurseryString(JSContext* cx, size_t thingSize, AllocKind kind) if (allowGC && !cx->suppressGC) { cx->runtime()->gc.minorGC(JS::gcreason::OUT_OF_NURSERY); - // Exceeding gcMaxBytes while tenuring can disable the Nursery. - if (cx->nursery().isEnabled()) + // Exceeding gcMaxBytes while tenuring can disable the Nursery, and + // other heuristics can disable nursery strings for this zone. + if (cx->nursery().isEnabled() && cx->zone()->allocNurseryStrings) return static_cast(cx->nursery().allocateString(cx->zone(), thingSize, kind)); } return nullptr; diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index 34ff48056732..4b6740009721 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -8701,7 +8701,8 @@ JitRealm::generateStringConcatStub(JSContext* cx) // Ensure result length <= JSString::MAX_LENGTH. masm.branch32(Assembler::Above, temp2, Imm32(JSString::MAX_LENGTH), &failure); - // Allocate a new rope, guaranteed to be in the nursery. + // Allocate a new rope, guaranteed to be in the nursery if + // stringsCanBeInNursery. (As a result, no post barriers are needed below.) masm.newGCString(output, temp3, &failure, stringsCanBeInNursery); // Store rope length and flags. temp1 still holds the result of AND'ing the From 68810d750796153105f9dd368978de662c1c923a Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Wed, 8 Aug 2018 11:16:21 -0700 Subject: [PATCH 42/45] Bug 1481740 - Never do endianness swapping on pointers, r=lth --HG-- extra : topic : nursery.strings.fix extra : rebase_source : 59fc25493b07a621f92154668c88652f0a865ef1 --- js/src/vm/StructuredClone.cpp | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/js/src/vm/StructuredClone.cpp b/js/src/vm/StructuredClone.cpp index 0961f9a0e168..919780ac85b9 100644 --- a/js/src/vm/StructuredClone.cpp +++ b/js/src/vm/StructuredClone.cpp @@ -327,7 +327,6 @@ struct SCOutput { MOZ_MUST_USE bool writeBytes(const void* p, size_t nbytes); MOZ_MUST_USE bool writeChars(const Latin1Char* p, size_t nchars); MOZ_MUST_USE bool writeChars(const char16_t* p, size_t nchars); - MOZ_MUST_USE bool writePtr(const void*); template MOZ_MUST_USE bool writeArray(const T* p, size_t nbytes); @@ -839,10 +838,11 @@ SCInput::getPtr(uint64_t data, void** ptr) bool SCInput::readPtr(void** p) { + // See endianness comment in getPtr, above. uint64_t u; if (!readNativeEndian(&u)) return false; - *p = reinterpret_cast(NativeEndian::swapFromLittleEndian(u)); + *p = reinterpret_cast(u); return true; } @@ -953,12 +953,6 @@ SCOutput::writeChars(const Latin1Char* p, size_t nchars) return writeBytes(p, nchars); } -bool -SCOutput::writePtr(const void* p) -{ - return write(reinterpret_cast(p)); -} - void SCOutput::discardTransferables() { @@ -1793,7 +1787,7 @@ JSStructuredCloneWriter::writeTransferMap() // (and, if necessary, detaching this object if it's an ArrayBuffer). if (!out.writePair(SCTAG_TRANSFER_MAP_PENDING_ENTRY, JS::SCTAG_TMO_UNFILLED)) return false; - if (!out.writePtr(nullptr)) // Pointer to ArrayBuffer contents. + if (!out.write(0)) // Pointer to ArrayBuffer contents. return false; if (!out.write(0)) // extraData return false; @@ -1918,7 +1912,7 @@ JSStructuredCloneWriter::transferOwnership() point.write(NativeEndian::swapToLittleEndian(PairToUInt64(tag, ownership))); point.next(); - point.write(NativeEndian::swapToLittleEndian(reinterpret_cast(content))); + point.write(reinterpret_cast(content)); point.next(); point.write(NativeEndian::swapToLittleEndian(extraData)); point.next(); From 80b21bd2245fc393d1917e37fc66bf0e1c084f36 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Wed, 22 Aug 2018 12:22:30 -0400 Subject: [PATCH 43/45] Bug 1485182 - Part 2: Add a test case to ensure that http-on-modify-request will be dispatched by requests blocked by tracking protection; r=mayhemer --- .../antitracking/test/browser/browser.ini | 2 + ...RequestNotificationForTrackingResources.js | 60 +++++++++++++++++++ .../antitracking/test/browser/embedder.html | 2 + .../antitracking/test/browser/head.js | 2 + 4 files changed, 66 insertions(+) create mode 100644 toolkit/components/antitracking/test/browser/browser_onModifyRequestNotificationForTrackingResources.js create mode 100644 toolkit/components/antitracking/test/browser/embedder.html diff --git a/toolkit/components/antitracking/test/browser/browser.ini b/toolkit/components/antitracking/test/browser/browser.ini index 9153c97c8617..1cf1cde38780 100644 --- a/toolkit/components/antitracking/test/browser/browser.ini +++ b/toolkit/components/antitracking/test/browser/browser.ini @@ -1,5 +1,6 @@ [DEFAULT] support-files = + embedder.html head.js page.html 3rdParty.html @@ -22,6 +23,7 @@ support-files = server.sjs [browser_existingCookiesForSubresources.js] [browser_imageCache.js] support-files = image.sjs +[browser_onModifyRequestNotificationForTrackingResources.js] [browser_subResources.js] support-files = subResources.sjs [browser_script.js] diff --git a/toolkit/components/antitracking/test/browser/browser_onModifyRequestNotificationForTrackingResources.js b/toolkit/components/antitracking/test/browser/browser_onModifyRequestNotificationForTrackingResources.js new file mode 100644 index 000000000000..000b112ef361 --- /dev/null +++ b/toolkit/components/antitracking/test/browser/browser_onModifyRequestNotificationForTrackingResources.js @@ -0,0 +1,60 @@ +ChromeUtils.import("resource://gre/modules/Services.jsm"); + +/** + * This test ensures that http-on-modify-request is dispatched for channels that + * are blocked by tracking protection. It sets up a page with a third-party script + * resource on it that is blocked by TP, and sets up an http-on-modify-request + * observer which waits to be notified about that resource. The test would time out + * if the http-on-modify-request notification isn't dispatched before the channel is + * canceled. + */ + +async function onModifyRequest() { + return new Promise((resolve, reject) => { + Services.obs.addObserver(function observer(subject, topic, data) { + let httpChannel = subject.QueryInterface(Ci.nsIHttpChannel); + let spec = httpChannel.URI.spec; + info("Observed channel for " + spec); + if (httpChannel.URI.prePath + "/" != TEST_3RD_PARTY_DOMAIN_TP) { + return; + } + ok(spec.endsWith("empty.js"), "Correct resource observed"); + Services.obs.removeObserver(observer, "http-on-modify-request"); + resolve(); + }, "http-on-modify-request", false); + }); +} + +add_task(async function() { + info("Starting subResources test"); + + await SpecialPowers.flushPrefEnv(); + await SpecialPowers.pushPrefEnv({"set": [ + ["browser.contentblocking.enabled", true], + ["privacy.trackingprotection.enabled", true], + // the test doesn't open a private window, so we don't care about this pref's value + ["privacy.trackingprotection.pbmode.enabled", false], + // tracking annotations aren't needed in this test, only TP is needed + ["privacy.trackingprotection.annotate_channels", false], + // prevent the content blocking on-boarding UI to start mid-way through the test! + ["privacy.trackingprotection.introCount", ContentBlocking.MAX_INTROS], + ]}); + + await UrlClassifierTestUtils.addTestTrackers(); + + let promise = onModifyRequest(); + + info("Creating a new tab"); + let tab = BrowserTestUtils.addTab(gBrowser, TEST_EMBEDDER_PAGE); + gBrowser.selectedTab = tab; + + let browser = gBrowser.getBrowserForTab(tab); + await BrowserTestUtils.browserLoaded(browser); + + await promise; + + info("Removing the tab"); + BrowserTestUtils.removeTab(tab); + + UrlClassifierTestUtils.cleanupTestTrackers(); +}); diff --git a/toolkit/components/antitracking/test/browser/embedder.html b/toolkit/components/antitracking/test/browser/embedder.html new file mode 100644 index 000000000000..33d713794fdd --- /dev/null +++ b/toolkit/components/antitracking/test/browser/embedder.html @@ -0,0 +1,2 @@ + + diff --git a/toolkit/components/antitracking/test/browser/head.js b/toolkit/components/antitracking/test/browser/head.js index 8706cb4cec96..f587f5bea38c 100644 --- a/toolkit/components/antitracking/test/browser/head.js +++ b/toolkit/components/antitracking/test/browser/head.js @@ -1,9 +1,11 @@ const TEST_DOMAIN = "http://example.net/"; const TEST_3RD_PARTY_DOMAIN = "https://tracking.example.org/"; +const TEST_3RD_PARTY_DOMAIN_TP = "https://tracking.example.com/"; const TEST_PATH = "browser/toolkit/components/antitracking/test/browser/"; const TEST_TOP_PAGE = TEST_DOMAIN + TEST_PATH + "page.html"; +const TEST_EMBEDDER_PAGE = TEST_DOMAIN + TEST_PATH + "embedder.html"; const TEST_POPUP_PAGE = TEST_DOMAIN + TEST_PATH + "popup.html"; const TEST_3RD_PARTY_PAGE = TEST_3RD_PARTY_DOMAIN + TEST_PATH + "3rdParty.html"; const TEST_3RD_PARTY_PAGE_WO = TEST_3RD_PARTY_DOMAIN + TEST_PATH + "3rdPartyWO.html"; From f91879804f32164da2e5b43f0860900957f9530a Mon Sep 17 00:00:00 2001 From: Blake Kaplan Date: Wed, 22 Aug 2018 15:34:16 -0400 Subject: [PATCH 44/45] Bug 1480965 - Fix review nit. r=asuth --HG-- extra : rebase_source : 5403d4a11f6753d086fcc7a64314b72e31419a30 --- dom/serviceworkers/ServiceWorkerPrivate.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dom/serviceworkers/ServiceWorkerPrivate.cpp b/dom/serviceworkers/ServiceWorkerPrivate.cpp index 1e37079cb8d7..c9b81ce55884 100644 --- a/dom/serviceworkers/ServiceWorkerPrivate.cpp +++ b/dom/serviceworkers/ServiceWorkerPrivate.cpp @@ -1941,7 +1941,7 @@ ServiceWorkerPrivate::TerminateWorker() } Unused << NS_WARN_IF(!mWorkerPrivate->Cancel()); - mWorkerPrivate = nullptr; + RefPtr workerPrivate(mWorkerPrivate.forget()); mSupportsArray.Clear(); // Any pending events are never going to fire on this worker. Cancel From a67fd14d8fc907bda2477662de962e1392f3dc55 Mon Sep 17 00:00:00 2001 From: shindli Date: Wed, 22 Aug 2018 22:49:43 +0300 Subject: [PATCH 45/45] Backed out changeset 4bf7cca192e7 (bug 1485182) for ES linting failure on a CLOSED TREE --- .../antitracking/test/browser/browser.ini | 2 - ...RequestNotificationForTrackingResources.js | 60 ------------------- .../antitracking/test/browser/embedder.html | 2 - .../antitracking/test/browser/head.js | 2 - 4 files changed, 66 deletions(-) delete mode 100644 toolkit/components/antitracking/test/browser/browser_onModifyRequestNotificationForTrackingResources.js delete mode 100644 toolkit/components/antitracking/test/browser/embedder.html diff --git a/toolkit/components/antitracking/test/browser/browser.ini b/toolkit/components/antitracking/test/browser/browser.ini index 1cf1cde38780..9153c97c8617 100644 --- a/toolkit/components/antitracking/test/browser/browser.ini +++ b/toolkit/components/antitracking/test/browser/browser.ini @@ -1,6 +1,5 @@ [DEFAULT] support-files = - embedder.html head.js page.html 3rdParty.html @@ -23,7 +22,6 @@ support-files = server.sjs [browser_existingCookiesForSubresources.js] [browser_imageCache.js] support-files = image.sjs -[browser_onModifyRequestNotificationForTrackingResources.js] [browser_subResources.js] support-files = subResources.sjs [browser_script.js] diff --git a/toolkit/components/antitracking/test/browser/browser_onModifyRequestNotificationForTrackingResources.js b/toolkit/components/antitracking/test/browser/browser_onModifyRequestNotificationForTrackingResources.js deleted file mode 100644 index 000b112ef361..000000000000 --- a/toolkit/components/antitracking/test/browser/browser_onModifyRequestNotificationForTrackingResources.js +++ /dev/null @@ -1,60 +0,0 @@ -ChromeUtils.import("resource://gre/modules/Services.jsm"); - -/** - * This test ensures that http-on-modify-request is dispatched for channels that - * are blocked by tracking protection. It sets up a page with a third-party script - * resource on it that is blocked by TP, and sets up an http-on-modify-request - * observer which waits to be notified about that resource. The test would time out - * if the http-on-modify-request notification isn't dispatched before the channel is - * canceled. - */ - -async function onModifyRequest() { - return new Promise((resolve, reject) => { - Services.obs.addObserver(function observer(subject, topic, data) { - let httpChannel = subject.QueryInterface(Ci.nsIHttpChannel); - let spec = httpChannel.URI.spec; - info("Observed channel for " + spec); - if (httpChannel.URI.prePath + "/" != TEST_3RD_PARTY_DOMAIN_TP) { - return; - } - ok(spec.endsWith("empty.js"), "Correct resource observed"); - Services.obs.removeObserver(observer, "http-on-modify-request"); - resolve(); - }, "http-on-modify-request", false); - }); -} - -add_task(async function() { - info("Starting subResources test"); - - await SpecialPowers.flushPrefEnv(); - await SpecialPowers.pushPrefEnv({"set": [ - ["browser.contentblocking.enabled", true], - ["privacy.trackingprotection.enabled", true], - // the test doesn't open a private window, so we don't care about this pref's value - ["privacy.trackingprotection.pbmode.enabled", false], - // tracking annotations aren't needed in this test, only TP is needed - ["privacy.trackingprotection.annotate_channels", false], - // prevent the content blocking on-boarding UI to start mid-way through the test! - ["privacy.trackingprotection.introCount", ContentBlocking.MAX_INTROS], - ]}); - - await UrlClassifierTestUtils.addTestTrackers(); - - let promise = onModifyRequest(); - - info("Creating a new tab"); - let tab = BrowserTestUtils.addTab(gBrowser, TEST_EMBEDDER_PAGE); - gBrowser.selectedTab = tab; - - let browser = gBrowser.getBrowserForTab(tab); - await BrowserTestUtils.browserLoaded(browser); - - await promise; - - info("Removing the tab"); - BrowserTestUtils.removeTab(tab); - - UrlClassifierTestUtils.cleanupTestTrackers(); -}); diff --git a/toolkit/components/antitracking/test/browser/embedder.html b/toolkit/components/antitracking/test/browser/embedder.html deleted file mode 100644 index 33d713794fdd..000000000000 --- a/toolkit/components/antitracking/test/browser/embedder.html +++ /dev/null @@ -1,2 +0,0 @@ - - diff --git a/toolkit/components/antitracking/test/browser/head.js b/toolkit/components/antitracking/test/browser/head.js index f587f5bea38c..8706cb4cec96 100644 --- a/toolkit/components/antitracking/test/browser/head.js +++ b/toolkit/components/antitracking/test/browser/head.js @@ -1,11 +1,9 @@ const TEST_DOMAIN = "http://example.net/"; const TEST_3RD_PARTY_DOMAIN = "https://tracking.example.org/"; -const TEST_3RD_PARTY_DOMAIN_TP = "https://tracking.example.com/"; const TEST_PATH = "browser/toolkit/components/antitracking/test/browser/"; const TEST_TOP_PAGE = TEST_DOMAIN + TEST_PATH + "page.html"; -const TEST_EMBEDDER_PAGE = TEST_DOMAIN + TEST_PATH + "embedder.html"; const TEST_POPUP_PAGE = TEST_DOMAIN + TEST_PATH + "popup.html"; const TEST_3RD_PARTY_PAGE = TEST_3RD_PARTY_DOMAIN + TEST_PATH + "3rdParty.html"; const TEST_3RD_PARTY_PAGE_WO = TEST_3RD_PARTY_DOMAIN + TEST_PATH + "3rdPartyWO.html";