From ebd2c4d0103c82f16650b84341ab6c7c9cc93563 Mon Sep 17 00:00:00 2001 From: Ryan Hunt Date: Thu, 27 Apr 2017 18:32:08 -0400 Subject: [PATCH 01/14] Bug 1357880 - Add a telemetry probe for non-passive keyboard event listeners r=smaug, data-review=bsmedberg This commit adds a telemetry probe to track the percentage of pages that ever have a non-passive 'keydown' or 'keypress' event that could preventDefault() APZ key scrolling of the root of a page. A flag is added to each EventListenerManager to track whether it ever had a qualifying event listener, and then in nsGlobalWindow::FreeInnerObjects() the event targets that could preventDefault() a scroll are checked for this flag. This check is done at nsGlobalWindow::FreeInnerObjects() so that the DOM is still alive. MozReview-Commit-ID: EkK3vxehZA5 --HG-- extra : rebase_source : 4642189d0065254cf74dfe8475403f0bf8210bca --- dom/base/nsGlobalWindow.cpp | 15 +++++++++++++++ dom/events/EventListenerManager.cpp | 4 ++++ dom/events/EventListenerManager.h | 5 ++++- dom/events/EventTarget.cpp | 7 +++++++ dom/events/EventTarget.h | 1 + toolkit/components/telemetry/Histograms.json | 7 +++++++ 6 files changed, 38 insertions(+), 1 deletion(-) diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index 52830ba10a34..875445fee8ad 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -103,6 +103,7 @@ #include "nsIDocShell.h" #include "nsIDocCharset.h" #include "nsIDocument.h" +#include "nsIDocumentInlines.h" #include "Crypto.h" #include "nsIDOMDocument.h" #include "nsIDOMElement.h" @@ -2044,6 +2045,20 @@ nsGlobalWindow::FreeInnerObjects() { NS_ASSERTION(IsInnerWindow(), "Don't free inner objects on an outer window"); + if (mDoc && !nsContentUtils::IsSystemPrincipal(mDoc->NodePrincipal())) { + EventTarget* win = this; + EventTarget* html = mDoc->GetHtmlElement(); + EventTarget* body = mDoc->GetBodyElement(); + + bool keyboardAware = win->MayHaveAPZAwareKeyEventListener() || + mDoc->MayHaveAPZAwareKeyEventListener() || + (html && html->MayHaveAPZAwareKeyEventListener()) || + (body && body->MayHaveAPZAwareKeyEventListener()); + + Telemetry::Accumulate(Telemetry::APZ_AWARE_KEY_LISTENERS, + keyboardAware ? 1 : 0); + } + // Make sure that this is called before we null out the document and // other members that the window destroyed observers could // re-create. diff --git a/dom/events/EventListenerManager.cpp b/dom/events/EventListenerManager.cpp index 7ba38f83b62d..fa0c204e9db7 100644 --- a/dom/events/EventListenerManager.cpp +++ b/dom/events/EventListenerManager.cpp @@ -129,6 +129,7 @@ EventListenerManagerBase::EventListenerManagerBase() , mMayHaveTouchEventListener(false) , mMayHaveMouseEnterLeaveEventListener(false) , mMayHavePointerEnterLeaveEventListener(false) + , mMayHaveAPZAwareKeyEventListener(false) , mMayHaveKeyEventListener(false) , mMayHaveInputOrCompositionEventListener(false) , mClearingListeners(false) @@ -411,6 +412,9 @@ EventListenerManager::AddEventListenerInternal( if (!aFlags.mInSystemGroup) { mMayHaveKeyEventListener = true; } + if (!aFlags.mPassive && aTypeAtom != nsGkAtoms::onkeyup) { + mMayHaveAPZAwareKeyEventListener = true; + } } else if (aTypeAtom == nsGkAtoms::oncompositionend || aTypeAtom == nsGkAtoms::oncompositionstart || aTypeAtom == nsGkAtoms::oncompositionupdate || diff --git a/dom/events/EventListenerManager.h b/dom/events/EventListenerManager.h index dfd3007c5a2e..95944c959e92 100644 --- a/dom/events/EventListenerManager.h +++ b/dom/events/EventListenerManager.h @@ -163,11 +163,12 @@ protected: uint16_t mMayHaveTouchEventListener : 1; uint16_t mMayHaveMouseEnterLeaveEventListener : 1; uint16_t mMayHavePointerEnterLeaveEventListener : 1; + uint16_t mMayHaveAPZAwareKeyEventListener : 1; uint16_t mMayHaveKeyEventListener : 1; uint16_t mMayHaveInputOrCompositionEventListener : 1; uint16_t mClearingListeners : 1; uint16_t mIsMainThreadELM : 1; - // uint16_t mUnused : 5; + // uint16_t mUnused : 4; }; /* @@ -442,6 +443,8 @@ public: bool MayHaveMouseEnterLeaveEventListener() { return mMayHaveMouseEnterLeaveEventListener; } bool MayHavePointerEnterLeaveEventListener() { return mMayHavePointerEnterLeaveEventListener; } + bool MayHaveAPZAwareKeyEventListener() const { return mMayHaveAPZAwareKeyEventListener; } + /** * Returns true if there may be a key event listener (keydown, keypress, * or keyup) registered, or false if there definitely isn't. diff --git a/dom/events/EventTarget.cpp b/dom/events/EventTarget.cpp index fade6eca189f..54f31205c756 100644 --- a/dom/events/EventTarget.cpp +++ b/dom/events/EventTarget.cpp @@ -64,6 +64,13 @@ EventTarget::IsApzAware() const return elm && elm->HasApzAwareListeners(); } +bool +EventTarget::MayHaveAPZAwareKeyEventListener() const +{ + EventListenerManager* elm = GetExistingListenerManager(); + return elm && elm->MayHaveAPZAwareKeyEventListener(); +} + bool EventTarget::DispatchEvent(Event& aEvent, CallerType aCallerType, diff --git a/dom/events/EventTarget.h b/dom/events/EventTarget.h index 5c3f997b8b4c..50b7443d91bc 100644 --- a/dom/events/EventTarget.h +++ b/dom/events/EventTarget.h @@ -100,6 +100,7 @@ public: virtual void AsyncEventRunning(AsyncEventDispatcher* aEvent) {} virtual bool IsApzAware() const; + bool MayHaveAPZAwareKeyEventListener() const; protected: EventHandlerNonNull* GetEventHandler(nsIAtom* aType, diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 45fe44f4e6b9..c373f614676a 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -10435,6 +10435,13 @@ "releaseChannelCollection": "opt-out", "description": "Graphics Crash Reason (...)" }, + "APZ_AWARE_KEY_LISTENERS": { + "alert_emails": ["rhunt@mozilla.com"], + "bug_numbers": [1352654], + "expires_in_version": "58", + "kind": "boolean", + "description": "The percentage of pages with a non-passive key event on the path to the root scrolling element that would disable APZ key scrolling. This is tracked for non system principal windows, so it applies to toplevel windows and subframes/iframes, but not chrome windows. " + }, "SCROLL_INPUT_METHODS": { "alert_emails": ["botond@mozilla.com"], "bug_numbers": [1238137], From d5bf64b9d46e9fd4ee4c9f98f42400df43a132e4 Mon Sep 17 00:00:00 2001 From: Ryan Hunt Date: Wed, 26 Apr 2017 18:56:51 -0400 Subject: [PATCH 02/14] Bug 1357880 - Add a telemetry probe for mousemove event listeners r=smaug, data-review=bsmedberg This commit adds a telemetry probe to determine the percentage of pages that ever have a 'mousemove' event listener added to the DOM. This is for determining how often APZ key scrolling could handle interleaved mousemove events. A flag is added to nsPIDOMWindow to track whether a qualifying event listener was ever added to the DOM for this window, and is updated by EventListenerManager. There are several other similar flags to this. The probe is reported in nsGlobalWindow::FreeInnerObjects() so that it can be compared exactly with the non-passive keyboard listener APZ probe. MozReview-Commit-ID: DqqCfrdRCGp --HG-- extra : rebase_source : fad8159c28b587572a4191f7cbde1e97e166639c --- dom/base/nsGlobalWindow.cpp | 4 ++++ dom/base/nsNodeUtils.cpp | 3 +++ dom/base/nsPIDOMWindow.h | 19 +++++++++++++++++++ dom/events/EventListenerManager.cpp | 6 ++++++ dom/events/EventListenerManager.h | 5 ++++- toolkit/components/telemetry/Histograms.json | 7 +++++++ 6 files changed, 43 insertions(+), 1 deletion(-) diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index 875445fee8ad..0867d2835bd4 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -978,6 +978,7 @@ nsPIDOMWindow::nsPIDOMWindow(nsPIDOMWindowOuter *aOuterWindow) mIsHandlingResizeEvent(false), mIsInnerWindow(aOuterWindow != nullptr), mMayHavePaintEventListener(false), mMayHaveTouchEventListener(false), mMayHaveMouseEnterLeaveEventListener(false), + mMayHaveMouseMoveEventListener(false), mMayHavePointerEnterLeaveEventListener(false), mInnerObjectsFreed(false), mIsModalContentWindow(false), @@ -2050,11 +2051,14 @@ nsGlobalWindow::FreeInnerObjects() EventTarget* html = mDoc->GetHtmlElement(); EventTarget* body = mDoc->GetBodyElement(); + bool mouseAware = AsInner()->HasMouseMoveEventListeners(); bool keyboardAware = win->MayHaveAPZAwareKeyEventListener() || mDoc->MayHaveAPZAwareKeyEventListener() || (html && html->MayHaveAPZAwareKeyEventListener()) || (body && body->MayHaveAPZAwareKeyEventListener()); + Telemetry::Accumulate(Telemetry::APZ_AWARE_MOUSEMOVE_LISTENERS, + mouseAware ? 1 : 0); Telemetry::Accumulate(Telemetry::APZ_AWARE_KEY_LISTENERS, keyboardAware ? 1 : 0); } diff --git a/dom/base/nsNodeUtils.cpp b/dom/base/nsNodeUtils.cpp index d21dec431d68..77c703c9bad5 100644 --- a/dom/base/nsNodeUtils.cpp +++ b/dom/base/nsNodeUtils.cpp @@ -539,6 +539,9 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, bool aClone, bool aDeep, if (elm->MayHaveTouchEventListener()) { window->SetHasTouchEventListeners(); } + if (elm->MayHaveMouseMoveEventListener()) { + window->SetHasMouseMoveEventListeners(); + } if (elm->MayHaveMouseEnterLeaveEventListener()) { window->SetHasMouseEnterLeaveEventListeners(); } diff --git a/dom/base/nsPIDOMWindow.h b/dom/base/nsPIDOMWindow.h index 3b964826f5cd..5518a9270301 100644 --- a/dom/base/nsPIDOMWindow.h +++ b/dom/base/nsPIDOMWindow.h @@ -669,6 +669,7 @@ protected: bool mMayHavePaintEventListener; bool mMayHaveTouchEventListener; bool mMayHaveMouseEnterLeaveEventListener; + bool mMayHaveMouseMoveEventListener; bool mMayHavePointerEnterLeaveEventListener; // Used to detect whether we have called FreeInnerObjects() (e.g. to ensure @@ -814,6 +815,24 @@ public: mMutationBits |= aType; } + /** + * Call this to check whether some node (this window, its document, + * or content in that document) has or had a mousemove event listener. + */ + bool HasMouseMoveEventListeners() + { + return mMayHaveMouseMoveEventListener; + } + + /** + * Call this to indicate that some node (this window, its document, + * or content in that document) has or had a mousemove event listener. + */ + void SetHasMouseMoveEventListeners() + { + mMayHaveMouseMoveEventListener = true; + } + /** * Call this to check whether some node (this window, its document, * or content in that document) has a mouseenter/leave event listener. diff --git a/dom/events/EventListenerManager.cpp b/dom/events/EventListenerManager.cpp index fa0c204e9db7..65b97daf3f00 100644 --- a/dom/events/EventListenerManager.cpp +++ b/dom/events/EventListenerManager.cpp @@ -127,6 +127,7 @@ EventListenerManagerBase::EventListenerManagerBase() , mMayHaveCapturingListeners(false) , mMayHaveSystemGroupListeners(false) , mMayHaveTouchEventListener(false) + , mMayHaveMouseMoveEventListener(false) , mMayHaveMouseEnterLeaveEventListener(false) , mMayHavePointerEnterLeaveEventListener(false) , mMayHaveAPZAwareKeyEventListener(false) @@ -422,6 +423,11 @@ EventListenerManager::AddEventListenerInternal( if (!aFlags.mInSystemGroup) { mMayHaveInputOrCompositionEventListener = true; } + } else if (aTypeAtom == nsGkAtoms::onmousemove) { + mMayHaveMouseMoveEventListener = true; + if (nsPIDOMWindowInner* window = GetInnerWindowForTarget()) { + window->SetHasMouseMoveEventListeners(); + } } if (IsApzAwareListener(listener)) { diff --git a/dom/events/EventListenerManager.h b/dom/events/EventListenerManager.h index 95944c959e92..6d60343d09e2 100644 --- a/dom/events/EventListenerManager.h +++ b/dom/events/EventListenerManager.h @@ -161,6 +161,7 @@ protected: uint16_t mMayHaveCapturingListeners : 1; uint16_t mMayHaveSystemGroupListeners : 1; uint16_t mMayHaveTouchEventListener : 1; + uint16_t mMayHaveMouseMoveEventListener : 1; uint16_t mMayHaveMouseEnterLeaveEventListener : 1; uint16_t mMayHavePointerEnterLeaveEventListener : 1; uint16_t mMayHaveAPZAwareKeyEventListener : 1; @@ -168,7 +169,7 @@ protected: uint16_t mMayHaveInputOrCompositionEventListener : 1; uint16_t mClearingListeners : 1; uint16_t mIsMainThreadELM : 1; - // uint16_t mUnused : 4; + // uint16_t mUnused : 3; }; /* @@ -440,6 +441,8 @@ public: */ bool MayHaveTouchEventListener() { return mMayHaveTouchEventListener; } + bool MayHaveMouseMoveEventListener() { return mMayHaveMouseMoveEventListener; } + bool MayHaveMouseEnterLeaveEventListener() { return mMayHaveMouseEnterLeaveEventListener; } bool MayHavePointerEnterLeaveEventListener() { return mMayHavePointerEnterLeaveEventListener; } diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index c373f614676a..ef4589226a64 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -10442,6 +10442,13 @@ "kind": "boolean", "description": "The percentage of pages with a non-passive key event on the path to the root scrolling element that would disable APZ key scrolling. This is tracked for non system principal windows, so it applies to toplevel windows and subframes/iframes, but not chrome windows. " }, + "APZ_AWARE_MOUSEMOVE_LISTENERS": { + "alert_emails": ["rhunt@mozilla.com"], + "bug_numbers": [1352654], + "expires_in_version": "58", + "kind": "boolean", + "description": "The percentage of pages with a mousemove listener anywhere in the document that would disable APZ key scrolling. This is tracked for non system principal windows, so it applies to toplevel windows and subframes/iframes, but not chrome windows." + }, "SCROLL_INPUT_METHODS": { "alert_emails": ["botond@mozilla.com"], "bug_numbers": [1238137], From a20cb19dced9d69e5bf8627e9a42972c6b2307f8 Mon Sep 17 00:00:00 2001 From: Bill McCloskey Date: Thu, 27 Apr 2017 15:25:21 -0700 Subject: [PATCH 03/14] Bug 1360372 - Acquire cooperative lock when entering system zone group (r=bhackett) MozReview-Commit-ID: 92SgTKMD6xt --- js/src/gc/ZoneGroup.cpp | 9 +++++++-- js/src/gc/ZoneGroup.h | 9 ++++++++- js/src/jscntxt.h | 16 ++++++++++++++++ js/src/jscntxtinlines.h | 2 +- js/src/jsfriendapi.cpp | 13 +++++++++++++ js/src/jsfriendapi.h | 14 ++++++++++++++ js/src/jsgc.cpp | 3 ++- 7 files changed, 61 insertions(+), 5 deletions(-) diff --git a/js/src/gc/ZoneGroup.cpp b/js/src/gc/ZoneGroup.cpp index 464a62ccb3c3..11e53204100a 100644 --- a/js/src/gc/ZoneGroup.cpp +++ b/js/src/gc/ZoneGroup.cpp @@ -57,12 +57,17 @@ ZoneGroup::~ZoneGroup() } void -ZoneGroup::enter() +ZoneGroup::enter(JSContext* cx) { - JSContext* cx = TlsContext.get(); if (ownerContext().context() == cx) { MOZ_ASSERT(enterCount); } else { + if (useExclusiveLocking) { + MOZ_ASSERT(!usedByHelperThread); + while (ownerContext().context() != nullptr) { + cx->yieldToEmbedding(); + } + } MOZ_RELEASE_ASSERT(ownerContext().context() == nullptr); MOZ_ASSERT(enterCount == 0); ownerContext_ = CooperatingContext(cx); diff --git a/js/src/gc/ZoneGroup.h b/js/src/gc/ZoneGroup.h index f6b520034037..0ea926572345 100644 --- a/js/src/gc/ZoneGroup.h +++ b/js/src/gc/ZoneGroup.h @@ -41,11 +41,15 @@ class ZoneGroup // The number of times the context has entered this zone group. UnprotectedData enterCount; + // If this flag is true, then we may need to block before entering this zone + // group. Blocking happens using JSContext::yieldToEmbedding. + UnprotectedData useExclusiveLocking; + public: CooperatingContext& ownerContext() { return ownerContext_.ref(); } void* addressOfOwnerContext() { return &ownerContext_.ref().cx; } - void enter(); + void enter(JSContext* cx); void leave(); bool ownedByCurrentThread(); @@ -72,6 +76,9 @@ class ZoneGroup inline bool isCollecting(); inline bool isGCScheduled(); + // See the useExclusiveLocking field above. + void setUseExclusiveLocking() { useExclusiveLocking = true; } + #ifdef DEBUG private: // The number of possible bailing places encounters before forcefully bailing diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index 94f022945b65..62a9b3340216 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -303,10 +303,26 @@ struct JSContext : public JS::RootingContext, friend class js::jit::DebugModeOSRVolatileJitFrameIterator; friend void js::ReportOverRecursed(JSContext*, unsigned errorNumber); + // Returns to the embedding to allow other cooperative threads to run. We + // may do this if we need access to a ZoneGroup that is in use by another + // thread. + void yieldToEmbedding() { + (*yieldCallback_)(this); + } + + void setYieldCallback(js::YieldCallback callback) { + yieldCallback_ = callback; + } + private: static JS::Error reportedError; static JS::OOM reportedOOM; + // This callback is used to ask the embedding to allow other cooperative + // threads to run. We may do this if we need access to a ZoneGroup that is + // in use by another thread. + js::ThreadLocalData yieldCallback_; + public: inline JS::Result<> boolToResult(bool ok); diff --git a/js/src/jscntxtinlines.h b/js/src/jscntxtinlines.h index 478225f91345..85fa87936861 100644 --- a/js/src/jscntxtinlines.h +++ b/js/src/jscntxtinlines.h @@ -529,7 +529,7 @@ inline void JSContext::enterZoneGroup(js::ZoneGroup* group) { MOZ_ASSERT(this == js::TlsContext.get()); - group->enter(); + group->enter(this); } inline void diff --git a/js/src/jsfriendapi.cpp b/js/src/jsfriendapi.cpp index 387eb6aefe8c..f8cb5bca3938 100644 --- a/js/src/jsfriendapi.cpp +++ b/js/src/jsfriendapi.cpp @@ -1482,3 +1482,16 @@ js::SetCompartmentValidAccessPtr(JSContext* cx, JS::HandleObject global, bool* a { global->compartment()->setValidAccessPtr(accessp); } + +JS_FRIEND_API(void) +js::SetCooperativeYieldCallback(JSContext* cx, YieldCallback callback) +{ + cx->setYieldCallback(callback); +} + +JS_FRIEND_API(bool) +js::SystemZoneAvailable(JSContext* cx) +{ + CooperatingContext& owner = cx->runtime()->gc.systemZoneGroup->ownerContext(); + return owner.context() == cx || owner.context() == nullptr; +} diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index c7ec975970f0..63a8f432dadd 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -2963,6 +2963,20 @@ EnableAccessValidation(JSContext* cx, bool enabled); extern JS_FRIEND_API(void) SetCompartmentValidAccessPtr(JSContext* cx, JS::HandleObject global, bool* accessp); +// If the JS engine wants to block so that other cooperative threads can run, it +// will call the yield callback. It may do this if it needs to access a ZoneGroup +// that is held by another thread (such as the system zone group). +typedef void +(* YieldCallback)(JSContext* cx); + +extern JS_FRIEND_API(void) +SetCooperativeYieldCallback(JSContext* cx, YieldCallback callback); + +// Returns true if the system zone is available (i.e., if no cooperative contexts +// are using it now). +extern JS_FRIEND_API(bool) +SystemZoneAvailable(JSContext* cx); + } /* namespace js */ class NativeProfiler diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index fde7deb004c3..19e54f257f55 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -6971,7 +6971,7 @@ js::NewCompartment(JSContext* cx, JSPrincipals* principals, if (group) { // Take over ownership of the group while we create the compartment/zone. - group->enter(); + group->enter(cx); } else { MOZ_ASSERT(!zone); group = cx->new_(rt); @@ -7042,6 +7042,7 @@ js::NewCompartment(JSContext* cx, JSPrincipals* principals, if (zoneSpec == JS::SystemZone || zoneSpec == JS::NewZoneInSystemZoneGroup) { MOZ_RELEASE_ASSERT(!rt->gc.systemZoneGroup); rt->gc.systemZoneGroup = group; + group->setUseExclusiveLocking(); } } From bb2fc6efcebc306450ff11684b1aafa7e9dd6354 Mon Sep 17 00:00:00 2001 From: Bill McCloskey Date: Sun, 30 Apr 2017 20:23:59 -0700 Subject: [PATCH 04/14] Bug 1360372 - Avoid rooting hazard when entering atoms compartment (r=sfink) This patch avoids a rooting hazard involving the atoms compartment. There is code in jsatom.cpp that enters the atoms compartment while unrooted pointers are on the stack. Now that enterZoneGroup can potentially yield, this leads to a rooting hazard. This patch avoids the hazard by using a totally separate path that avoids enterZoneGroup when entering the atoms compartment. MozReview-Commit-ID: ExG1ofCbN8C --- js/src/jscntxt.h | 12 ++++++++---- js/src/jscntxtinlines.h | 24 +++++++++++++++++------- js/src/jscompartment.h | 5 ++++- js/src/jscompartmentinlines.h | 23 ++++++++++++++++++----- 4 files changed, 47 insertions(+), 17 deletions(-) diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index 62a9b3340216..faae8dab7fc2 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -201,10 +201,14 @@ struct JSContext : public JS::RootingContext, #endif private: - // If |c| or |oldCompartment| is the atoms compartment, the - // |exclusiveAccessLock| must be held. - inline void enterCompartment(JSCompartment* c, - const js::AutoLockForExclusiveAccess* maybeLock = nullptr); + // We distinguish between entering the atoms compartment and all other + // compartments. Entering the atoms compartment requires a lock. Also, we + // don't call enterZoneGroup when entering the atoms compartment since that + // can induce GC hazards. + inline void enterNonAtomsCompartment(JSCompartment* c); + inline void enterAtomsCompartment(JSCompartment* c, + const js::AutoLockForExclusiveAccess& lock); + friend class js::AutoCompartment; public: diff --git a/js/src/jscntxtinlines.h b/js/src/jscntxtinlines.h index 85fa87936861..4dad00a57c6f 100644 --- a/js/src/jscntxtinlines.h +++ b/js/src/jscntxtinlines.h @@ -451,17 +451,27 @@ JSContext::runningWithTrustedPrincipals() } inline void -JSContext::enterCompartment( - JSCompartment* c, - const js::AutoLockForExclusiveAccess* maybeLock /* = nullptr */) +JSContext::enterNonAtomsCompartment(JSCompartment* c) { enterCompartmentDepth_++; - if (!c->zone()->isAtomsZone()) - enterZoneGroup(c->zone()->group()); + MOZ_ASSERT(!c->zone()->isAtomsZone()); + enterZoneGroup(c->zone()->group()); c->enter(); - setCompartment(c, maybeLock); + setCompartment(c, nullptr); +} + +inline void +JSContext::enterAtomsCompartment(JSCompartment* c, + const js::AutoLockForExclusiveAccess& lock) +{ + enterCompartmentDepth_++; + + MOZ_ASSERT(c->zone()->isAtomsZone()); + + c->enter(); + setCompartment(c, &lock); } template @@ -469,7 +479,7 @@ inline void JSContext::enterCompartmentOf(const T& target) { MOZ_ASSERT(JS::CellIsNotGray(target)); - enterCompartment(target->compartment(), nullptr); + enterNonAtomsCompartment(target->compartment()); } inline void diff --git a/js/src/jscompartment.h b/js/src/jscompartment.h index 3cf989c5d408..375b5cd69731 100644 --- a/js/src/jscompartment.h +++ b/js/src/jscompartment.h @@ -1053,8 +1053,11 @@ class AutoCompartment JSCompartment* origin() const { return origin_; } protected: + inline AutoCompartment(JSContext* cx, JSCompartment* target); + + // Used only for entering the atoms compartment. inline AutoCompartment(JSContext* cx, JSCompartment* target, - AutoLockForExclusiveAccess* maybeLock = nullptr); + AutoLockForExclusiveAccess& lock); private: AutoCompartment(const AutoCompartment&) = delete; diff --git a/js/src/jscompartmentinlines.h b/js/src/jscompartmentinlines.h index 54c90f02e415..47280975127a 100644 --- a/js/src/jscompartmentinlines.h +++ b/js/src/jscompartmentinlines.h @@ -44,14 +44,27 @@ js::AutoCompartment::AutoCompartment(JSContext* cx, const T& target) cx_->enterCompartmentOf(target); } -// Protected constructor that bypasses assertions in enterCompartmentOf. +// Protected constructor that bypasses assertions in enterCompartmentOf. Used +// only for entering the atoms compartment. js::AutoCompartment::AutoCompartment(JSContext* cx, JSCompartment* target, - js::AutoLockForExclusiveAccess* maybeLock /* = nullptr */) + js::AutoLockForExclusiveAccess& lock) : cx_(cx), origin_(cx->compartment()), - maybeLock_(maybeLock) + maybeLock_(&lock) { - cx_->enterCompartment(target, maybeLock); + MOZ_ASSERT(target->isAtomsCompartment()); + cx_->enterAtomsCompartment(target, lock); +} + +// Protected constructor that bypasses assertions in enterCompartmentOf. Should +// not be used to enter the atoms compartment. +js::AutoCompartment::AutoCompartment(JSContext* cx, JSCompartment* target) + : cx_(cx), + origin_(cx->compartment()), + maybeLock_(nullptr) +{ + MOZ_ASSERT(!target->isAtomsCompartment()); + cx_->enterNonAtomsCompartment(target); } js::AutoCompartment::~AutoCompartment() @@ -61,7 +74,7 @@ js::AutoCompartment::~AutoCompartment() js::AutoAtomsCompartment::AutoAtomsCompartment(JSContext* cx, js::AutoLockForExclusiveAccess& lock) - : AutoCompartment(cx, cx->atomsCompartment(lock), &lock) + : AutoCompartment(cx, cx->atomsCompartment(lock), lock) {} js::AutoCompartmentUnchecked::AutoCompartmentUnchecked(JSContext* cx, JSCompartment* target) From f9391961968d23bddf428113a87d7532547e9756 Mon Sep 17 00:00:00 2001 From: philipp Date: Fri, 28 Apr 2017 08:44:00 -0400 Subject: [PATCH 05/14] Bug 1333486 - Add idmcchandler5.dll/idmcchandler5_64.dll to the blocklist. r=marco --- mozglue/build/WindowsDllBlocklist.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mozglue/build/WindowsDllBlocklist.cpp b/mozglue/build/WindowsDllBlocklist.cpp index 3ed0e7e49f46..e13c8daa39cb 100644 --- a/mozglue/build/WindowsDllBlocklist.cpp +++ b/mozglue/build/WindowsDllBlocklist.cpp @@ -233,6 +233,8 @@ static const DllBlockInfo sWindowsDllBlocklist[] = { // Crashes with Internet Download Manager, bug 1333486 { "idmcchandler7.dll", ALL_VERSIONS }, { "idmcchandler7_64.dll", ALL_VERSIONS }, + { "idmcchandler5.dll", ALL_VERSIONS }, + { "idmcchandler5_64.dll", ALL_VERSIONS }, // Nahimic 2 breaks applicaton update (bug 1356637) { "nahimic2devprops.dll", ALL_VERSIONS }, From 1810e88c32c15a1ab456b112d0752aad6ef46849 Mon Sep 17 00:00:00 2001 From: David Parks Date: Mon, 1 May 2017 14:04:55 -0700 Subject: [PATCH 06/14] Bug 1359129 - Use the most recent RootDocAccessible when delaying DocAccessibleChild messages. r=aklotz We sometimes briefly have more than one root DocAccessible associated with a TabChild, for example, while navigating links in a page. This patch makes sure that we use the correct accessible when delaying messages that we forward to the parent process. --- accessible/generic/DocAccessible.cpp | 3 +++ accessible/ipc/win/DocAccessibleChild.cpp | 14 ++------------ dom/ipc/TabChild.cpp | 3 +++ dom/ipc/TabChild.h | 16 ++++++++++++++++ 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/accessible/generic/DocAccessible.cpp b/accessible/generic/DocAccessible.cpp index 159b375e5454..78325b700da1 100644 --- a/accessible/generic/DocAccessible.cpp +++ b/accessible/generic/DocAccessible.cpp @@ -1481,6 +1481,9 @@ DocAccessible::DoInitialUpdate() if (RefPtr tabChild = dom::TabChild::GetFrom(docShell)) { DocAccessibleChild* ipcDoc = new DocAccessibleChild(this, tabChild); SetIPCDoc(ipcDoc); + if (IsRoot()) { + tabChild->SetTopLevelDocAccessibleChild(ipcDoc); + } #if defined(XP_WIN) IAccessibleHolder holder(CreateHolderFromAccessible(this)); diff --git a/accessible/ipc/win/DocAccessibleChild.cpp b/accessible/ipc/win/DocAccessibleChild.cpp index 9165f3ca9b92..c8170c1af95f 100644 --- a/accessible/ipc/win/DocAccessibleChild.cpp +++ b/accessible/ipc/win/DocAccessibleChild.cpp @@ -102,18 +102,8 @@ DocAccessibleChild::PushDeferredEvent(UniquePtr aEvent) return; } - nsTArray ipcDocAccs; - tabChild->ManagedPDocAccessibleChild(ipcDocAccs); - - // Look for the top-level DocAccessibleChild - there will only be one - // per TabChild. - for (uint32_t i = 0, l = ipcDocAccs.Length(); i < l; ++i) { - auto ipcDocAcc = static_cast(ipcDocAccs[i]); - if (ipcDocAcc->mDoc && ipcDocAcc->mDoc->IsRoot()) { - topLevelIPCDoc = ipcDocAcc; - break; - } - } + topLevelIPCDoc = + static_cast(tabChild->GetTopLevelDocAccessibleChild()); } if (topLevelIPCDoc) { diff --git a/dom/ipc/TabChild.cpp b/dom/ipc/TabChild.cpp index 564d80ad4b53..8f58fa860227 100644 --- a/dom/ipc/TabChild.cpp +++ b/dom/ipc/TabChild.cpp @@ -403,6 +403,9 @@ TabChild::TabChild(nsIContentChild* aManager, #if defined(XP_WIN) && defined(ACCESSIBILITY) , mNativeWindowHandle(0) #endif +#if defined(ACCESSIBILITY) + , mTopLevelDocAccessibleChild(nullptr) +#endif { nsWeakPtr weakPtrThis(do_GetWeakReference(static_cast(this))); // for capture by the lambda mSetAllowedTouchBehaviorCallback = [weakPtrThis](uint64_t aInputBlockId, diff --git a/dom/ipc/TabChild.h b/dom/ipc/TabChild.h index 266513131878..e76a17e8740b 100644 --- a/dom/ipc/TabChild.h +++ b/dom/ipc/TabChild.h @@ -675,6 +675,18 @@ public: mozilla::dom::TabGroup* TabGroup(); +#if defined(ACCESSIBILITY) + void SetTopLevelDocAccessibleChild(PDocAccessibleChild* aTopLevelChild) + { + mTopLevelDocAccessibleChild = aTopLevelChild; + } + + PDocAccessibleChild* GetTopLevelDocAccessibleChild() + { + return mTopLevelDocAccessibleChild; + } +#endif + protected: virtual ~TabChild(); @@ -843,6 +855,10 @@ private: uintptr_t mNativeWindowHandle; #endif // defined(XP_WIN) +#if defined(ACCESSIBILITY) + PDocAccessibleChild* mTopLevelDocAccessibleChild; +#endif + DISALLOW_EVIL_CONSTRUCTORS(TabChild); }; From fdc1a8031f10979d3e15443af7829dfb4535ac1f Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Tue, 2 May 2017 03:50:16 +0200 Subject: [PATCH 07/14] Bug 1359060 - [css-grid] Disallow fit-content() in repeat(auto-fill/fit) track sizes (per the CSS Grid spec). r=dholbert MozReview-Commit-ID: Eijlbr8lHjV --- layout/style/nsCSSParser.cpp | 4 ++++ layout/style/test/property_database.js | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/layout/style/nsCSSParser.cpp b/layout/style/nsCSSParser.cpp index 595054b2777d..2841c34f9bbf 100644 --- a/layout/style/nsCSSParser.cpp +++ b/layout/style/nsCSSParser.cpp @@ -8768,6 +8768,10 @@ CSSParserImpl::ParseGridTrackSize(nsCSSValue& aValue, return CSSParseResult::NotFound; } if (mToken.mIdent.LowerCaseEqualsLiteral("fit-content")) { + if (requireFixedSize) { + UngetToken(); + return CSSParseResult::Error; + } nsCSSValue::Array* func = aValue.InitFunction(eCSSKeyword_fit_content, 1); if (ParseGridTrackBreadth(func->Item(1)) == CSSParseResult::Ok && func->Item(1).IsLengthPercentCalcUnit() && diff --git a/layout/style/test/property_database.js b/layout/style/test/property_database.js index 2cfe3de841b1..a7d09d4fd187 100644 --- a/layout/style/test/property_database.js +++ b/layout/style/test/property_database.js @@ -6238,6 +6238,7 @@ if (IsCSSPropertyPrefEnabled("layout.css.grid.enabled")) { "repeat(auto-fill,minmax(1%,auto))", "repeat(auto-fill,minmax(1em,min-content)) minmax(min-content,0)", "repeat(auto-fill,minmax(max-content,1mm))", + "repeat(2, fit-content(1px))", "fit-content(1px) 1fr", "[a] fit-content(calc(1px - 99%)) [b]", "[a] fit-content(10%) [b c] fit-content(1em)", @@ -6282,6 +6283,8 @@ if (IsCSSPropertyPrefEnabled("layout.css.grid.enabled")) { "repeat(1, repeat(1, 20px))", "repeat(auto-fill, auto)", "repeat(auto-fit,auto)", + "repeat(auto-fill, fit-content(1px))", + "repeat(auto-fit, fit-content(1px))", "repeat(auto-fit,[])", "repeat(auto-fill, 0) repeat(auto-fit, 0) ", "repeat(auto-fit, 0) repeat(auto-fill, 0) ", @@ -6303,6 +6306,8 @@ if (IsCSSPropertyPrefEnabled("layout.css.grid.enabled")) { "fit-content(-1px)", "fit-content(auto)", "fit-content(min-content)", + "fit-content(1px) repeat(auto-fit, 1px)", + "fit-content(1px) repeat(auto-fill, 1px)", ], unbalanced_values: [ "(foo] 40px", From facb121186167319705dad96698d6c706214e5e0 Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Tue, 2 May 2017 03:50:16 +0200 Subject: [PATCH 08/14] Bug 1360867 - Make a few classes final to possibly devirtualize some calls. r=dholbert MozReview-Commit-ID: EyBsvXbfJvK --- layout/xul/grid/nsGridRowGroupFrame.h | 2 +- layout/xul/nsBoxFrame.cpp | 5 +++-- layout/xul/nsDeckFrame.h | 2 +- layout/xul/nsDocElementBoxFrame.cpp | 4 ++-- layout/xul/nsGroupBoxFrame.cpp | 6 ++++-- layout/xul/nsImageBoxFrame.h | 3 ++- layout/xul/nsListBoxLayout.h | 2 +- layout/xul/nsListItemFrame.h | 2 +- layout/xul/nsResizerFrame.h | 2 +- layout/xul/nsRootBoxFrame.cpp | 3 ++- layout/xul/nsScrollBoxFrame.cpp | 2 +- layout/xul/nsScrollbarButtonFrame.h | 2 +- layout/xul/nsScrollbarFrame.h | 2 +- layout/xul/nsSplitterFrame.h | 2 +- layout/xul/nsStackFrame.h | 2 +- layout/xul/nsTextBoxFrame.cpp | 3 ++- layout/xul/nsTextBoxFrame.h | 2 +- layout/xul/nsXULLabelFrame.h | 2 +- layout/xul/tree/nsTreeColFrame.cpp | 3 ++- layout/xul/tree/nsTreeColFrame.h | 2 +- 20 files changed, 30 insertions(+), 23 deletions(-) diff --git a/layout/xul/grid/nsGridRowGroupFrame.h b/layout/xul/grid/nsGridRowGroupFrame.h index f367e212168e..27cc4c74bb9d 100644 --- a/layout/xul/grid/nsGridRowGroupFrame.h +++ b/layout/xul/grid/nsGridRowGroupFrame.h @@ -23,7 +23,7 @@ * all the columns). However, multiple levels of groups are allowed, so * the parent or child could instead be another group. */ -class nsGridRowGroupFrame : public nsBoxFrame +class nsGridRowGroupFrame final : public nsBoxFrame { public: NS_DECL_FRAMEARENA_HELPERS diff --git a/layout/xul/nsBoxFrame.cpp b/layout/xul/nsBoxFrame.cpp index 395e1f3addd8..f699a7a295d8 100644 --- a/layout/xul/nsBoxFrame.cpp +++ b/layout/xul/nsBoxFrame.cpp @@ -2001,7 +2001,8 @@ nsBoxFrame::XULRelayoutChildAtOrdinal(nsIFrame* aChild) // REVIEW: This is roughly of what nsMenuFrame::GetFrameForPoint used to do. // I've made 'allowevents' affect child elements because that seems the only // reasonable thing to do. -class nsDisplayXULEventRedirector : public nsDisplayWrapList { +class nsDisplayXULEventRedirector final : public nsDisplayWrapList +{ public: nsDisplayXULEventRedirector(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame, nsDisplayItem* aItem, @@ -2053,7 +2054,7 @@ void nsDisplayXULEventRedirector::HitTest(nsDisplayListBuilder* aBuilder, } } -class nsXULEventRedirectorWrapper : public nsDisplayWrapper +class nsXULEventRedirectorWrapper final : public nsDisplayWrapper { public: explicit nsXULEventRedirectorWrapper(nsIFrame* aTargetFrame) diff --git a/layout/xul/nsDeckFrame.h b/layout/xul/nsDeckFrame.h index dfefd4af3d06..3f6783eac47a 100644 --- a/layout/xul/nsDeckFrame.h +++ b/layout/xul/nsDeckFrame.h @@ -17,7 +17,7 @@ #include "mozilla/Attributes.h" #include "nsBoxFrame.h" -class nsDeckFrame : public nsBoxFrame +class nsDeckFrame final : public nsBoxFrame { public: NS_DECL_QUERYFRAME_TARGET(nsDeckFrame) diff --git a/layout/xul/nsDocElementBoxFrame.cpp b/layout/xul/nsDocElementBoxFrame.cpp index 4d978617a423..076e8f2270b3 100644 --- a/layout/xul/nsDocElementBoxFrame.cpp +++ b/layout/xul/nsDocElementBoxFrame.cpp @@ -26,8 +26,8 @@ using namespace mozilla::dom; -class nsDocElementBoxFrame : public nsBoxFrame, - public nsIAnonymousContentCreator +class nsDocElementBoxFrame final : public nsBoxFrame + , public nsIAnonymousContentCreator { public: virtual void DestroyFrom(nsIFrame* aDestructRoot) override; diff --git a/layout/xul/nsGroupBoxFrame.cpp b/layout/xul/nsGroupBoxFrame.cpp index 514287a24a2d..03fc4b7b2502 100644 --- a/layout/xul/nsGroupBoxFrame.cpp +++ b/layout/xul/nsGroupBoxFrame.cpp @@ -18,7 +18,8 @@ using namespace mozilla; using namespace mozilla::gfx; using namespace mozilla::image; -class nsGroupBoxFrame : public nsBoxFrame { +class nsGroupBoxFrame final : public nsBoxFrame +{ public: NS_DECL_FRAMEARENA_HELPERS @@ -83,7 +84,8 @@ NS_NewGroupBoxFrame(nsIPresShell* aPresShell, nsStyleContext* aContext) NS_IMPL_FRAMEARENA_HELPERS(nsGroupBoxFrame) -class nsDisplayXULGroupBorder : public nsDisplayItem { +class nsDisplayXULGroupBorder final : public nsDisplayItem +{ public: nsDisplayXULGroupBorder(nsDisplayListBuilder* aBuilder, nsGroupBoxFrame* aFrame) : diff --git a/layout/xul/nsImageBoxFrame.h b/layout/xul/nsImageBoxFrame.h index fc6b3dbadb2b..9a37ee85ea07 100644 --- a/layout/xul/nsImageBoxFrame.h +++ b/layout/xul/nsImageBoxFrame.h @@ -126,7 +126,8 @@ private: bool mSuppressStyleCheck; }; // class nsImageBoxFrame -class nsDisplayXULImage : public nsDisplayImageContainer { +class nsDisplayXULImage final : public nsDisplayImageContainer +{ public: nsDisplayXULImage(nsDisplayListBuilder* aBuilder, nsImageBoxFrame* aFrame) : diff --git a/layout/xul/nsListBoxLayout.h b/layout/xul/nsListBoxLayout.h index d1f9788433f5..4aa276be956c 100644 --- a/layout/xul/nsListBoxLayout.h +++ b/layout/xul/nsListBoxLayout.h @@ -13,7 +13,7 @@ class nsIFrame; typedef class nsIFrame nsIFrame; class nsBoxLayoutState; -class nsListBoxLayout : public nsGridRowGroupLayout +class nsListBoxLayout final : public nsGridRowGroupLayout { public: nsListBoxLayout(); diff --git a/layout/xul/nsListItemFrame.h b/layout/xul/nsListItemFrame.h index 40e731efa5b5..3ef0ad79cf71 100644 --- a/layout/xul/nsListItemFrame.h +++ b/layout/xul/nsListItemFrame.h @@ -9,7 +9,7 @@ nsIFrame* NS_NewListItemFrame(nsIPresShell* aPresShell, nsStyleContext *aContext); -class nsListItemFrame : public nsGridRowLeafFrame +class nsListItemFrame final : public nsGridRowLeafFrame { public: NS_DECL_FRAMEARENA_HELPERS diff --git a/layout/xul/nsResizerFrame.h b/layout/xul/nsResizerFrame.h index 92656bc764bb..eaa5cbf4fc90 100644 --- a/layout/xul/nsResizerFrame.h +++ b/layout/xul/nsResizerFrame.h @@ -11,7 +11,7 @@ class nsIBaseWindow; -class nsResizerFrame : public nsTitleBarFrame +class nsResizerFrame final : public nsTitleBarFrame { protected: struct Direction { diff --git a/layout/xul/nsRootBoxFrame.cpp b/layout/xul/nsRootBoxFrame.cpp index 2e53d210f5ee..7565a3560046 100644 --- a/layout/xul/nsRootBoxFrame.cpp +++ b/layout/xul/nsRootBoxFrame.cpp @@ -41,7 +41,8 @@ nsIRootBox::GetRootBox(nsIPresShell* aShell) return rootBox; } -class nsRootBoxFrame : public nsBoxFrame, public nsIRootBox { +class nsRootBoxFrame final : public nsBoxFrame, public nsIRootBox +{ public: friend nsIFrame* NS_NewBoxFrame(nsIPresShell* aPresShell, nsStyleContext* aContext); diff --git a/layout/xul/nsScrollBoxFrame.cpp b/layout/xul/nsScrollBoxFrame.cpp index e0efa98d74e2..b7dea7569a65 100644 --- a/layout/xul/nsScrollBoxFrame.cpp +++ b/layout/xul/nsScrollBoxFrame.cpp @@ -14,7 +14,7 @@ using namespace mozilla; -class nsAutoRepeatBoxFrame : public nsButtonBoxFrame +class nsAutoRepeatBoxFrame final : public nsButtonBoxFrame { public: NS_DECL_FRAMEARENA_HELPERS diff --git a/layout/xul/nsScrollbarButtonFrame.h b/layout/xul/nsScrollbarButtonFrame.h index 6bce0e36c96f..3dd3768264d4 100644 --- a/layout/xul/nsScrollbarButtonFrame.h +++ b/layout/xul/nsScrollbarButtonFrame.h @@ -18,7 +18,7 @@ #include "nsITimer.h" #include "nsRepeatService.h" -class nsScrollbarButtonFrame : public nsButtonBoxFrame +class nsScrollbarButtonFrame final : public nsButtonBoxFrame { public: NS_DECL_FRAMEARENA_HELPERS diff --git a/layout/xul/nsScrollbarFrame.h b/layout/xul/nsScrollbarFrame.h index 0bf319c15628..738ed5d0bddd 100644 --- a/layout/xul/nsScrollbarFrame.h +++ b/layout/xul/nsScrollbarFrame.h @@ -17,7 +17,7 @@ class nsIScrollbarMediator; nsIFrame* NS_NewScrollbarFrame(nsIPresShell* aPresShell, nsStyleContext* aContext); -class nsScrollbarFrame : public nsBoxFrame +class nsScrollbarFrame final : public nsBoxFrame { public: explicit nsScrollbarFrame(nsStyleContext* aContext) diff --git a/layout/xul/nsSplitterFrame.h b/layout/xul/nsSplitterFrame.h index df8872255ef2..b50091d39754 100644 --- a/layout/xul/nsSplitterFrame.h +++ b/layout/xul/nsSplitterFrame.h @@ -18,7 +18,7 @@ class nsSplitterFrameInner; nsIFrame* NS_NewSplitterFrame(nsIPresShell* aPresShell, nsStyleContext* aContext); -class nsSplitterFrame : public nsBoxFrame +class nsSplitterFrame final : public nsBoxFrame { public: NS_DECL_FRAMEARENA_HELPERS diff --git a/layout/xul/nsStackFrame.h b/layout/xul/nsStackFrame.h index b90a16b213c2..d94f96b8b581 100644 --- a/layout/xul/nsStackFrame.h +++ b/layout/xul/nsStackFrame.h @@ -17,7 +17,7 @@ #include "mozilla/Attributes.h" #include "nsBoxFrame.h" -class nsStackFrame : public nsBoxFrame +class nsStackFrame final : public nsBoxFrame { public: NS_DECL_FRAMEARENA_HELPERS diff --git a/layout/xul/nsTextBoxFrame.cpp b/layout/xul/nsTextBoxFrame.cpp index f2e41ba543ce..779473036c1b 100644 --- a/layout/xul/nsTextBoxFrame.cpp +++ b/layout/xul/nsTextBoxFrame.cpp @@ -277,7 +277,8 @@ nsTextBoxFrame::UpdateAttributes(nsIAtom* aAttribute, } -class nsDisplayXULTextBox : public nsDisplayItem { +class nsDisplayXULTextBox final : public nsDisplayItem +{ public: nsDisplayXULTextBox(nsDisplayListBuilder* aBuilder, nsTextBoxFrame* aFrame) : diff --git a/layout/xul/nsTextBoxFrame.h b/layout/xul/nsTextBoxFrame.h index ece1d4b4db82..c74df5d9e6a3 100644 --- a/layout/xul/nsTextBoxFrame.h +++ b/layout/xul/nsTextBoxFrame.h @@ -12,7 +12,7 @@ class nsAccessKeyInfo; class nsAsyncAccesskeyUpdate; class nsFontMetrics; -class nsTextBoxFrame : public nsLeafBoxFrame +class nsTextBoxFrame final : public nsLeafBoxFrame { public: NS_DECL_QUERYFRAME_TARGET(nsTextBoxFrame) diff --git a/layout/xul/nsXULLabelFrame.h b/layout/xul/nsXULLabelFrame.h index 1f0d295f693a..b5c3a197b47b 100644 --- a/layout/xul/nsXULLabelFrame.h +++ b/layout/xul/nsXULLabelFrame.h @@ -15,7 +15,7 @@ #error "This file should not be included" #endif -class nsXULLabelFrame : public nsBlockFrame +class nsXULLabelFrame final : public nsBlockFrame { public: NS_DECL_FRAMEARENA_HELPERS diff --git a/layout/xul/tree/nsTreeColFrame.cpp b/layout/xul/tree/nsTreeColFrame.cpp index cab223e49cf1..70e114da2090 100644 --- a/layout/xul/tree/nsTreeColFrame.cpp +++ b/layout/xul/tree/nsTreeColFrame.cpp @@ -53,7 +53,8 @@ nsTreeColFrame::DestroyFrom(nsIFrame* aDestructRoot) nsBoxFrame::DestroyFrom(aDestructRoot); } -class nsDisplayXULTreeColSplitterTarget : public nsDisplayItem { +class nsDisplayXULTreeColSplitterTarget final : public nsDisplayItem +{ public: nsDisplayXULTreeColSplitterTarget(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame) : diff --git a/layout/xul/tree/nsTreeColFrame.h b/layout/xul/tree/nsTreeColFrame.h index 8fc3219d500f..af1f351caa1a 100644 --- a/layout/xul/tree/nsTreeColFrame.h +++ b/layout/xul/tree/nsTreeColFrame.h @@ -11,7 +11,7 @@ class nsITreeBoxObject; nsIFrame* NS_NewTreeColFrame(nsIPresShell* aPresShell, nsStyleContext* aContext); -class nsTreeColFrame : public nsBoxFrame +class nsTreeColFrame final : public nsBoxFrame { public: NS_DECL_FRAMEARENA_HELPERS From f7b2b2533f3f9a2f4db91f1dafaa0803bef9d419 Mon Sep 17 00:00:00 2001 From: David Major Date: Mon, 1 May 2017 22:56:47 -0400 Subject: [PATCH 09/14] Bug 1360575: Use PR_SetCurrentThreadName to set the sampler thread's name on all platforms. r=mstange --- tools/profiler/core/platform-linux-android.cpp | 2 -- tools/profiler/core/platform-macos.cpp | 17 ----------------- tools/profiler/core/platform.cpp | 2 ++ 3 files changed, 2 insertions(+), 19 deletions(-) diff --git a/tools/profiler/core/platform-linux-android.cpp b/tools/profiler/core/platform-linux-android.cpp index 1ee418e47c2c..7451787ff1b0 100644 --- a/tools/profiler/core/platform-linux-android.cpp +++ b/tools/profiler/core/platform-linux-android.cpp @@ -40,7 +40,6 @@ #include #include #include -#include // set name #include #include #include @@ -273,7 +272,6 @@ static void* ThreadEntry(void* aArg) { auto thread = static_cast(aArg); - prctl(PR_SET_NAME, "SamplerThread", 0, 0, 0); thread->mSamplerTid = gettid(); thread->Run(); return nullptr; diff --git a/tools/profiler/core/platform-macos.cpp b/tools/profiler/core/platform-macos.cpp index b064666b96a1..b471fa39b768 100644 --- a/tools/profiler/core/platform-macos.cpp +++ b/tools/profiler/core/platform-macos.cpp @@ -4,11 +4,9 @@ * 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 #include #include #include -#include #include #include @@ -78,25 +76,10 @@ private: //////////////////////////////////////////////////////////////////////// // BEGIN SamplerThread target specifics -static void -SetThreadName() -{ - // pthread_setname_np is only available in 10.6 or later, so test - // for it at runtime. - int (*dynamic_pthread_setname_np)(const char*); - *reinterpret_cast(&dynamic_pthread_setname_np) = - dlsym(RTLD_DEFAULT, "pthread_setname_np"); - if (!dynamic_pthread_setname_np) - return; - - dynamic_pthread_setname_np("SamplerThread"); -} - static void* ThreadEntry(void* aArg) { auto thread = static_cast(aArg); - SetThreadName(); thread->Run(); return nullptr; } diff --git a/tools/profiler/core/platform.cpp b/tools/profiler/core/platform.cpp index ebae6486fc6a..4be311fdc011 100644 --- a/tools/profiler/core/platform.cpp +++ b/tools/profiler/core/platform.cpp @@ -1774,6 +1774,8 @@ NewSamplerThread(PSLockRef aLock, uint32_t aGeneration, double aInterval) void SamplerThread::Run() { + PR_SetCurrentThreadName("SamplerThread"); + // This will be positive if we are running behind schedule (sampling less // frequently than desired) and negative if we are ahead of schedule. TimeDuration lastSleepOvershoot = 0; From 2a35142b62b62ca9b12f29e97314ce904d5d4ee6 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Fri, 21 Apr 2017 14:02:15 +1200 Subject: [PATCH 10/14] Bug 1349418 - Remove checkerboarding code and just use an opaque background color behind root scroll frames. r=kats --- gfx/layers/LayerAttributes.h | 14 +-------- gfx/layers/Layers.cpp | 6 +--- gfx/layers/Layers.h | 17 ----------- gfx/layers/client/ClientTiledPaintedLayer.cpp | 2 +- gfx/layers/client/TiledContentClient.cpp | 2 +- .../composite/ContainerLayerComposite.cpp | 21 +------------- .../composite/LayerManagerComposite.cpp | 29 ------------------- gfx/layers/composite/LayerManagerComposite.h | 8 ----- layout/base/PresShell.cpp | 17 ++++++++--- layout/painting/FrameLayerBuilder.cpp | 14 +-------- .../async-scrolling/checkerboard-2-ref.html | 1 + .../async-scrolling/checkerboard-3-ref.html | 1 + 12 files changed, 21 insertions(+), 111 deletions(-) diff --git a/gfx/layers/LayerAttributes.h b/gfx/layers/LayerAttributes.h index 7e5c2844d6b5..be25841525d5 100644 --- a/gfx/layers/LayerAttributes.h +++ b/gfx/layers/LayerAttributes.h @@ -44,13 +44,6 @@ public: // All set methods return true if values changed, false otherwise. // - bool SetLayerBounds(const gfx::IntRect& aLayerBounds) { - if (mLayerBounds.IsEqualEdges(aLayerBounds)) { - return false; - } - mLayerBounds = aLayerBounds; - return true; - } bool SetPostScale(float aXScale, float aYScale) { if (mPostXScale == aXScale && mPostYScale == aYScale) { return false; @@ -195,9 +188,6 @@ public: // Getters. // - const gfx::IntRect& LayerBounds() const { - return mLayerBounds; - } float PostXScale() const { return mPostXScale; } @@ -265,8 +255,7 @@ public: } bool operator ==(const SimpleLayerAttributes& aOther) const { - return mLayerBounds == aOther.mLayerBounds && - mTransform == aOther.mTransform && + return mTransform == aOther.mTransform && mTransformIsPerspective == aOther.mTransformIsPerspective && mScrolledClip == aOther.mScrolledClip && mPostXScale == aOther.mPostXScale && @@ -283,7 +272,6 @@ public: } private: - gfx::IntRect mLayerBounds; gfx::Matrix4x4 mTransform; bool mTransformIsPerspective; Maybe mScrolledClip; diff --git a/gfx/layers/Layers.cpp b/gfx/layers/Layers.cpp index 74c738edf4ed..858b29b806ea 100644 --- a/gfx/layers/Layers.cpp +++ b/gfx/layers/Layers.cpp @@ -576,8 +576,7 @@ Layer::CalculateScissorRect(const RenderTargetIntRect& aCurrentScissorRect) return currentClip; } - if (GetLocalVisibleRegion().IsEmpty() && - !(AsHostLayer() && AsHostLayer()->NeedToDrawCheckerboarding())) { + if (GetLocalVisibleRegion().IsEmpty()) { // When our visible region is empty, our parent may not have created the // intermediate surface that we would require for correct clipping; however, // this does not matter since we are invisible. @@ -1850,9 +1849,6 @@ Layer::PrintInfo(std::stringstream& aStream, const char* aPrefix) if (GetTransformIsPerspective()) { aStream << " [perspective]"; } - if (!GetLayerBounds().IsEmpty()) { - AppendToString(aStream, GetLayerBounds(), " [bounds=", "]"); - } if (!mVisibleRegion.IsEmpty()) { AppendToString(aStream, mVisibleRegion.ToUnknownRegion(), " [visible=", "]"); } else { diff --git a/gfx/layers/Layers.h b/gfx/layers/Layers.h index 74a5f7c77070..4ee3872ef857 100644 --- a/gfx/layers/Layers.h +++ b/gfx/layers/Layers.h @@ -898,22 +898,6 @@ public: } } - /** - * CONSTRUCTION PHASE ONLY - * The union of the bounds of all the display item that got flattened - * into this layer. This is intended to be an approximation to the - * size of the layer if the nearest scrollable ancestor had an infinitely - * large displayport. Computing this more exactly is too expensive, - * but this approximation is sufficient for what we need to use it for. - */ - virtual void SetLayerBounds(const gfx::IntRect& aLayerBounds) - { - if (mSimpleAttrs.SetLayerBounds(aLayerBounds)) { - MOZ_LAYERS_LOG_IF_SHADOWABLE(this, ("Layer::Mutated(%p) LayerBounds", this)); - MutatedSimple(); - } - } - /** * CONSTRUCTION PHASE ONLY * Tell this layer which region will be visible. The visible region @@ -1340,7 +1324,6 @@ public: const Maybe& GetScrolledClip() const { return mSimpleAttrs.ScrolledClip(); } Maybe GetScrolledClipRect() const; uint32_t GetContentFlags() { return mSimpleAttrs.ContentFlags(); } - const gfx::IntRect& GetLayerBounds() const { return mSimpleAttrs.LayerBounds(); } const LayerIntRegion& GetVisibleRegion() const { return mVisibleRegion; } const ScrollMetadata& GetScrollMetadata(uint32_t aIndex) const; const FrameMetrics& GetFrameMetrics(uint32_t aIndex) const; diff --git a/gfx/layers/client/ClientTiledPaintedLayer.cpp b/gfx/layers/client/ClientTiledPaintedLayer.cpp index 7ed5c84f25b3..4873f09da604 100644 --- a/gfx/layers/client/ClientTiledPaintedLayer.cpp +++ b/gfx/layers/client/ClientTiledPaintedLayer.cpp @@ -169,7 +169,7 @@ ClientTiledPaintedLayer::BeginPaint() ParentLayerToLayerMatrix4x4 transformDisplayPortToLayer = GetTransformToAncestorsParentLayer(this, displayPortAncestor).Inverse(); - LayerRect layerBounds = ViewAs(Rect(GetLayerBounds())); + LayerRect layerBounds(GetVisibleRegion().GetBounds()); // Compute the critical display port that applies to this layer in the // LayoutDevice space of this layer, but only if there is no OMT animation diff --git a/gfx/layers/client/TiledContentClient.cpp b/gfx/layers/client/TiledContentClient.cpp index 3335607f8c39..ccb20c0c9397 100644 --- a/gfx/layers/client/TiledContentClient.cpp +++ b/gfx/layers/client/TiledContentClient.cpp @@ -1218,7 +1218,7 @@ ClientMultiTiledLayerBuffer::ComputeProgressiveUpdateRegion(const nsIntRegion& a GetCompositorSideCompositionBounds(scrollAncestor, aPaintData->mTransformToCompBounds, viewTransform, - ViewAs(Rect(mPaintedLayer.GetLayerBounds()))); + LayerRect(mPaintedLayer.GetVisibleRegion().GetBounds())); if (!transformedCompositionBounds) { aPaintData->mPaintFinished = true; diff --git a/gfx/layers/composite/ContainerLayerComposite.cpp b/gfx/layers/composite/ContainerLayerComposite.cpp index 83a187a8295d..084d95829448 100755 --- a/gfx/layers/composite/ContainerLayerComposite.cpp +++ b/gfx/layers/composite/ContainerLayerComposite.cpp @@ -212,8 +212,7 @@ ContainerPrepare(ContainerT* aContainer, // We don't want to skip container layers because otherwise their mPrepared // may be null which is not allowed. if (!layerToRender->GetLayer()->AsContainerLayer()) { - if (!layerToRender->GetLayer()->IsVisible() && - !layerToRender->NeedToDrawCheckerboarding(nullptr)) { + if (!layerToRender->GetLayer()->IsVisible()) { CULLING_LOG("Sublayer %p has no effective visible region\n", layerToRender->GetLayer()); continue; } @@ -423,24 +422,6 @@ RenderLayers(ContainerT* aContainer, LayerManagerComposite* aManager, } } - Color color; - if (layerToRender->NeedToDrawCheckerboarding(&color)) { - if (gfxPrefs::APZHighlightCheckerboardedAreas()) { - color = Color(255 / 255.f, 188 / 255.f, 217 / 255.f, 1.f); // "Cotton Candy" - } - // Ideally we would want to intersect the checkerboard region from the APZ with the layer bounds - // and only fill in that area. However the layer bounds takes into account the base translation - // for the painted layer whereas the checkerboard region does not. One does not simply - // intersect areas in different coordinate spaces. So we do this a little more permissively - // and only fill in the background when we know there is checkerboard, which in theory - // should only occur transiently. - EffectChain effectChain(layer); - effectChain.mPrimaryEffect = new EffectSolidColor(color); - aManager->GetCompositor()->DrawGeometry(gfx::Rect(layer->GetLayerBounds()), clipRect, - effectChain, layer->GetEffectiveOpacity(), - layer->GetEffectiveTransform(), Nothing()); - } - if (layerToRender->HasLayerBeenComposited()) { // Composer2D will compose this layer so skip GPU composition // this time. The flag will be reset for the next composition phase diff --git a/gfx/layers/composite/LayerManagerComposite.cpp b/gfx/layers/composite/LayerManagerComposite.cpp index 552cdad462a8..13428718a4b3 100644 --- a/gfx/layers/composite/LayerManagerComposite.cpp +++ b/gfx/layers/composite/LayerManagerComposite.cpp @@ -1501,35 +1501,6 @@ LayerComposite::HasStaleCompositor() const return mCompositeManager->GetCompositor() != mCompositor; } -static bool -LayerHasCheckerboardingAPZC(Layer* aLayer, Color* aOutColor) -{ - bool answer = false; - for (LayerMetricsWrapper i(aLayer, LayerMetricsWrapper::StartAt::BOTTOM); i; i = i.GetParent()) { - if (!i.Metrics().IsScrollable()) { - continue; - } - if (i.GetApzc() && i.GetApzc()->IsCurrentlyCheckerboarding()) { - if (aOutColor) { - *aOutColor = i.Metadata().GetBackgroundColor(); - } - answer = true; - break; - } - break; - } - return answer; -} - -bool -LayerComposite::NeedToDrawCheckerboarding(gfx::Color* aOutCheckerboardingColor) -{ - return GetLayer()->Manager()->AsyncPanZoomEnabled() && - (GetLayer()->GetContentFlags() & Layer::CONTENT_OPAQUE) && - GetLayer()->IsOpaqueForVisibility() && - LayerHasCheckerboardingAPZC(GetLayer(), aOutCheckerboardingColor); -} - #ifndef MOZ_HAVE_PLATFORM_SPECIFIC_LAYER_BUFFERS /*static*/ bool diff --git a/gfx/layers/composite/LayerManagerComposite.h b/gfx/layers/composite/LayerManagerComposite.h index 986b7f609c46..20f7cc6d6cea 100644 --- a/gfx/layers/composite/LayerManagerComposite.h +++ b/gfx/layers/composite/LayerManagerComposite.h @@ -589,12 +589,6 @@ public: bool GetShadowTransformSetByAnimation() { return mShadowTransformSetByAnimation; } bool GetShadowOpacitySetByAnimation() { return mShadowOpacitySetByAnimation; } - /** - * Return true if a checkerboarding background color needs to be drawn - * for this layer. - */ - virtual bool NeedToDrawCheckerboarding(gfx::Color* aOutCheckerboardingColor = nullptr) { return false; } - protected: HostLayerManager* mCompositorManager; @@ -694,8 +688,6 @@ public: */ virtual nsIntRegion GetFullyRenderedRegion(); - virtual bool NeedToDrawCheckerboarding(gfx::Color* aOutCheckerboardingColor = nullptr); - protected: LayerManagerComposite* mCompositeManager; diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp index b0e29c85f6ff..10530d7bc73d 100644 --- a/layout/base/PresShell.cpp +++ b/layout/base/PresShell.cpp @@ -5313,20 +5313,29 @@ PresShell::AddCanvasBackgroundColorItem(nsDisplayListBuilder& aBuilder, // color background behind a scrolled transparent background. Instead, // we'll try to move the color background into the scrolled content // by making nsDisplayCanvasBackground paint it. + bool addedScrollingBackgroundColor = false; if (!aFrame->GetParent()) { nsIScrollableFrame* sf = aFrame->PresContext()->PresShell()->GetRootScrollFrameAsScrollable(); if (sf) { nsCanvasFrame* canvasFrame = do_QueryFrame(sf->GetScrolledFrame()); if (canvasFrame && canvasFrame->IsVisibleForPainting(&aBuilder)) { - if (AddCanvasBackgroundColor(aList, canvasFrame, bgcolor, mHasCSSBackgroundColor)) - return; + addedScrollingBackgroundColor = + AddCanvasBackgroundColor(aList, canvasFrame, bgcolor, mHasCSSBackgroundColor); } } } - aList.AppendNewToBottom( - new (&aBuilder) nsDisplaySolidColor(&aBuilder, aFrame, aBounds, bgcolor)); + if (!addedScrollingBackgroundColor || + (nsLayoutUtils::UsesAsyncScrolling(aFrame) && NS_GET_A(bgcolor) == 255)) { + // With async scrolling, we'd like to have two instances of the background + // color: one that scrolls with the content (for the reasons stated above), + // and one underneath which does not scroll with the content, but which can + // be shown during checkerboarding and overscroll. + // We can only do that if the color is opaque. + aList.AppendNewToBottom( + new (&aBuilder) nsDisplaySolidColor(&aBuilder, aFrame, aBounds, bgcolor)); + } } static bool IsTransparentContainerElement(nsPresContext* aPresContext) diff --git a/layout/painting/FrameLayerBuilder.cpp b/layout/painting/FrameLayerBuilder.cpp index b5362cc77892..426b9704153a 100644 --- a/layout/painting/FrameLayerBuilder.cpp +++ b/layout/painting/FrameLayerBuilder.cpp @@ -660,10 +660,6 @@ public: * in mItemClip). */ void UpdateCommonClipCount(const DisplayItemClip& aCurrentClip); - /** - * The union of all the bounds of the display items in this layer. - */ - nsIntRect mBounds; /** * The region of visible content above the layer and below the * next PaintedLayerData currently in the stack, if any. @@ -3241,10 +3237,6 @@ void ContainerState::FinishPaintedLayerData(PaintedLayerData& aData, FindOpaqueB SetOuterVisibleRegionForLayer(layer, data->mVisibleRegion); } - nsIntRect layerBounds = data->mBounds; - layerBounds.MoveBy(-GetTranslationForPaintedLayer(data->mLayer)); - layer->SetLayerBounds(layerBounds); - #ifdef MOZ_DUMP_PAINTING if (!data->mLog.IsEmpty()) { if (PaintedLayerData* containingPld = mLayerBuilder->GetContainingPaintedLayerData()) { @@ -3441,10 +3433,6 @@ PaintedLayerData::Accumulate(ContainerState* aState, { FLB_LOG_PAINTED_LAYER_DECISION(this, "Accumulating dp=%s(%p), f=%p against pld=%p\n", aItem->Name(), aItem, aItem->Frame(), this); - bool snap; - nsRect itemBounds = aItem->GetBounds(aState->mBuilder, &snap); - mBounds = mBounds.Union(aState->ScaleToOutsidePixels(itemBounds, snap)); - if (aState->mBuilder->NeedToForceTransparentSurfaceForItem(aItem)) { mForceTransparentSurface = true; } @@ -3766,7 +3754,7 @@ ContainerState::GetDisplayPortForAnimatedGeometryRoot(AnimatedGeometryRoot* aAni mLastDisplayPortAGR = aAnimatedGeometryRoot; nsIScrollableFrame* sf = nsLayoutUtils::GetScrollableFrameFor(*aAnimatedGeometryRoot); - if (sf == nullptr) { + if (sf == nullptr || nsLayoutUtils::UsesAsyncScrolling(*aAnimatedGeometryRoot)) { mLastDisplayPortRect = nsRect(); return mLastDisplayPortRect; } diff --git a/layout/reftests/async-scrolling/checkerboard-2-ref.html b/layout/reftests/async-scrolling/checkerboard-2-ref.html index d1927e43d294..6a12210c0542 100644 --- a/layout/reftests/async-scrolling/checkerboard-2-ref.html +++ b/layout/reftests/async-scrolling/checkerboard-2-ref.html @@ -1,6 +1,7 @@ +
diff --git a/layout/reftests/async-scrolling/checkerboard-3-ref.html b/layout/reftests/async-scrolling/checkerboard-3-ref.html index bba831e93dcb..d00946ada6c4 100644 --- a/layout/reftests/async-scrolling/checkerboard-3-ref.html +++ b/layout/reftests/async-scrolling/checkerboard-3-ref.html @@ -1,5 +1,6 @@ +
From c95a73cfc69bbc85930b33a52a71b0f03ec8ff23 Mon Sep 17 00:00:00 2001 From: Matt Woodrow Date: Fri, 21 Apr 2017 14:03:40 +1200 Subject: [PATCH 11/14] Bug 1349418 - Require ASRs to match before allow occlusions between layers. r=mstange --- layout/painting/FrameLayerBuilder.cpp | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/layout/painting/FrameLayerBuilder.cpp b/layout/painting/FrameLayerBuilder.cpp index 426b9704153a..9c3a47debfcf 100644 --- a/layout/painting/FrameLayerBuilder.cpp +++ b/layout/painting/FrameLayerBuilder.cpp @@ -1325,6 +1325,7 @@ protected: */ nsIntRegion ComputeOpaqueRect(nsDisplayItem* aItem, AnimatedGeometryRoot* aAnimatedGeometryRoot, + const ActiveScrolledRoot* aASR, const DisplayItemClip& aClip, nsDisplayList* aList, bool* aHideAllLayersBelow, @@ -3775,6 +3776,7 @@ ContainerState::GetDisplayPortForAnimatedGeometryRoot(AnimatedGeometryRoot* aAni nsIntRegion ContainerState::ComputeOpaqueRect(nsDisplayItem* aItem, AnimatedGeometryRoot* aAnimatedGeometryRoot, + const ActiveScrolledRoot* aASR, const DisplayItemClip& aClip, nsDisplayList* aList, bool* aHideAllLayersBelow, @@ -3793,6 +3795,7 @@ ContainerState::ComputeOpaqueRect(nsDisplayItem* aItem, aClip.ApproximateIntersectInward(iter.Get())); } if (aAnimatedGeometryRoot == mContainerAnimatedGeometryRoot && + aASR == mContainerASR && opaqueClipped.Contains(mContainerBounds)) { *aHideAllLayersBelow = true; aList->SetIsOpaque(); @@ -4377,7 +4380,7 @@ ContainerState::ProcessDisplayItems(nsDisplayList* aList) newLayerEntry->mVisibleRegion = itemVisibleRegion; } newLayerEntry->mOpaqueRegion = ComputeOpaqueRect(item, - animatedGeometryRoot, itemClip, aList, + animatedGeometryRoot, itemASR, itemClip, aList, &newLayerEntry->mHideAllLayersBelow, &newLayerEntry->mOpaqueForAnimatedGeometryRootParent); } else { @@ -4433,7 +4436,7 @@ ContainerState::ProcessDisplayItems(nsDisplayList* aList) paintedLayerData->UpdateCommonClipCount(itemClip); } nsIntRegion opaquePixels = ComputeOpaqueRect(item, - animatedGeometryRoot, itemClip, aList, + animatedGeometryRoot, itemASR, itemClip, aList, &paintedLayerData->mHideAllLayersBelow, &paintedLayerData->mOpaqueForAnimatedGeometryRootParent); MOZ_ASSERT(nsIntRegion(itemDrawRect).Contains(opaquePixels)); @@ -4916,16 +4919,19 @@ ContainerState::CollectOldLayers() struct OpaqueRegionEntry { AnimatedGeometryRoot* mAnimatedGeometryRoot; + const ActiveScrolledRoot* mASR; nsIntRegion mOpaqueRegion; }; static OpaqueRegionEntry* FindOpaqueRegionEntry(nsTArray& aEntries, - AnimatedGeometryRoot* aAnimatedGeometryRoot) + AnimatedGeometryRoot* aAnimatedGeometryRoot, + const ActiveScrolledRoot* aASR) { for (uint32_t i = 0; i < aEntries.Length(); ++i) { OpaqueRegionEntry* d = &aEntries[i]; - if (d->mAnimatedGeometryRoot == aAnimatedGeometryRoot) { + if (d->mAnimatedGeometryRoot == aAnimatedGeometryRoot && + d->mASR == aASR) { return d; } } @@ -5155,7 +5161,7 @@ ContainerState::PostprocessRetainedLayers(nsIntRegion* aOpaqueRegionForContainer continue; } - OpaqueRegionEntry* data = FindOpaqueRegionEntry(opaqueRegions, e->mAnimatedGeometryRoot); + OpaqueRegionEntry* data = FindOpaqueRegionEntry(opaqueRegions, e->mAnimatedGeometryRoot, e->mASR); SetupScrollingMetadata(e); @@ -5178,19 +5184,23 @@ ContainerState::PostprocessRetainedLayers(nsIntRegion* aOpaqueRegionForContainer if (!e->mOpaqueRegion.IsEmpty()) { AnimatedGeometryRoot* animatedGeometryRootToCover = e->mAnimatedGeometryRoot; + const ActiveScrolledRoot* asrToCover = e->mASR; if (e->mOpaqueForAnimatedGeometryRootParent && e->mAnimatedGeometryRoot->mParentAGR == mContainerAnimatedGeometryRoot) { animatedGeometryRootToCover = mContainerAnimatedGeometryRoot; - data = FindOpaqueRegionEntry(opaqueRegions, animatedGeometryRootToCover); + asrToCover = mContainerASR; + data = FindOpaqueRegionEntry(opaqueRegions, animatedGeometryRootToCover, asrToCover); } if (!data) { - if (animatedGeometryRootToCover == mContainerAnimatedGeometryRoot) { + if (animatedGeometryRootToCover == mContainerAnimatedGeometryRoot && + asrToCover == mContainerASR) { NS_ASSERTION(opaqueRegionForContainer == -1, "Already found it?"); opaqueRegionForContainer = opaqueRegions.Length(); } data = opaqueRegions.AppendElement(); data->mAnimatedGeometryRoot = animatedGeometryRootToCover; + data->mASR = asrToCover; } nsIntRegion clippedOpaque = e->mOpaqueRegion; From 82005900e6725c881f222c04ebb1798a4dc39273 Mon Sep 17 00:00:00 2001 From: Matt Woodrow Date: Fri, 28 Apr 2017 12:45:36 +1200 Subject: [PATCH 12/14] Bug 1349418 - Put the unscrolled item in the right place when we're doing container scrolling. r=mstange --- layout/base/PresShell.cpp | 28 ++++++++++++++++++--------- layout/base/nsIPresShell.h | 14 +++++++++++++- layout/generic/nsSubDocumentFrame.cpp | 25 +++++++++++++++++++++++- 3 files changed, 56 insertions(+), 11 deletions(-) diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp index 10530d7bc73d..dbc721083d70 100644 --- a/layout/base/PresShell.cpp +++ b/layout/base/PresShell.cpp @@ -5313,8 +5313,10 @@ PresShell::AddCanvasBackgroundColorItem(nsDisplayListBuilder& aBuilder, // color background behind a scrolled transparent background. Instead, // we'll try to move the color background into the scrolled content // by making nsDisplayCanvasBackground paint it. - bool addedScrollingBackgroundColor = false; - if (!aFrame->GetParent()) { + // If we're only adding an unscrolled item, then pretend that we've + // already done it. + bool addedScrollingBackgroundColor = (aFlags & APPEND_UNSCROLLED_ONLY); + if (!aFrame->GetParent() && !addedScrollingBackgroundColor) { nsIScrollableFrame* sf = aFrame->PresContext()->PresShell()->GetRootScrollFrameAsScrollable(); if (sf) { @@ -5326,13 +5328,21 @@ PresShell::AddCanvasBackgroundColorItem(nsDisplayListBuilder& aBuilder, } } - if (!addedScrollingBackgroundColor || - (nsLayoutUtils::UsesAsyncScrolling(aFrame) && NS_GET_A(bgcolor) == 255)) { - // With async scrolling, we'd like to have two instances of the background - // color: one that scrolls with the content (for the reasons stated above), - // and one underneath which does not scroll with the content, but which can - // be shown during checkerboarding and overscroll. - // We can only do that if the color is opaque. + // With async scrolling, we'd like to have two instances of the background + // color: one that scrolls with the content (for the reasons stated above), + // and one underneath which does not scroll with the content, but which can + // be shown during checkerboarding and overscroll. + // We can only do that if the color is opaque. + bool forceUnscrolledItem = nsLayoutUtils::UsesAsyncScrolling(aFrame) && + NS_GET_A(bgcolor) == 255; + if ((aFlags & ADD_FOR_SUBDOC) && gfxPrefs::LayoutUseContainersForRootFrames()) { + // If we're using ContainerLayers for a subdoc, then any items we add here will + // still be scrolled (since we're inside the container at this point), so don't + // bother and we will do it manually later. + forceUnscrolledItem = false; + } + + if (!addedScrollingBackgroundColor || forceUnscrolledItem) { aList.AppendNewToBottom( new (&aBuilder) nsDisplaySolidColor(&aBuilder, aFrame, aBounds, bgcolor)); } diff --git a/layout/base/nsIPresShell.h b/layout/base/nsIPresShell.h index bf7a2c5ba76d..8c3df5369dc6 100644 --- a/layout/base/nsIPresShell.h +++ b/layout/base/nsIPresShell.h @@ -1272,9 +1272,21 @@ public: * canvas frame (if the FORCE_DRAW flag is passed then this check is skipped). * aBackstopColor is composed behind the background color of the canvas, it is * transparent by default. + * We attempt to make the background color part of the scrolled canvas (to reduce + * transparent layers), and if async scrolling is enabled (and the background + * is opaque) then we add a second, unscrolled item to handle the checkerboarding + * case. + * ADD_FOR_SUBDOC shoud be specified when calling this for a subdocument, and + * LayoutUseContainersForRootFrame might cause the whole list to be scrolled. In + * that case the second unscrolled item will be elided. + * APPEND_UNSCROLLED_ONLY only attempts to add the unscrolled item, so that we + * can add it manually after LayoutUseContainersForRootFrame has built the + * scrolling ContainerLayer. */ enum { - FORCE_DRAW = 0x01 + FORCE_DRAW = 0x01, + ADD_FOR_SUBDOC = 0x02, + APPEND_UNSCROLLED_ONLY = 0x04, }; virtual void AddCanvasBackgroundColorItem(nsDisplayListBuilder& aBuilder, nsDisplayList& aList, diff --git a/layout/generic/nsSubDocumentFrame.cpp b/layout/generic/nsSubDocumentFrame.cpp index 75a71f542a2d..bcf506d60025 100644 --- a/layout/generic/nsSubDocumentFrame.cpp +++ b/layout/generic/nsSubDocumentFrame.cpp @@ -494,7 +494,7 @@ nsSubDocumentFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, // Add the canvas background color to the bottom of the list. This // happens after we've built the list so that AddCanvasBackgroundColorItem // can monkey with the contents if necessary. - uint32_t flags = nsIPresShell::FORCE_DRAW; + uint32_t flags = nsIPresShell::FORCE_DRAW | nsIPresShell::ADD_FOR_SUBDOC; presShell->AddCanvasBackgroundColorItem( *aBuilder, childItems, frame, bounds, NS_RGBA(0,0,0,0), flags); } @@ -548,6 +548,29 @@ nsSubDocumentFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, childItems.AppendToTop(layerItem); } + // If we're using containers for root frames, then the earlier call + // to AddCanvasBackgroundColorItem won't have been able to add an + // unscrolled color item for overscroll. Try again now that we're + // outside the scrolled ContainerLayer. + if (!aBuilder->IsForEventDelivery() && + gfxPrefs::LayoutUseContainersForRootFrames() && + !nsLayoutUtils::NeedsPrintPreviewBackground(presContext)) { + nsRect bounds = GetContentRectRelativeToSelf() + + aBuilder->ToReferenceFrame(this); + + // Invoke AutoBuildingDisplayList to ensure that the correct dirty rect + // is used to compute the visible rect if AddCanvasBackgroundColorItem + // creates a display item. + nsDisplayListBuilder::AutoBuildingDisplayList + building(aBuilder, this, dirty, true); + // Add the canvas background color to the bottom of the list. This + // happens after we've built the list so that AddCanvasBackgroundColorItem + // can monkey with the contents if necessary. + uint32_t flags = nsIPresShell::FORCE_DRAW | nsIPresShell::APPEND_UNSCROLLED_ONLY; + presShell->AddCanvasBackgroundColorItem( + *aBuilder, childItems, this, bounds, NS_RGBA(0,0,0,0), flags); + } + if (aBuilder->IsForFrameVisibility()) { // We don't add the childItems to the return list as we're dealing with them here. presShell->RebuildApproximateFrameVisibilityDisplayList(childItems); From be4450a99664d9e9df355903e6f72655765dca7d Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 2 May 2017 01:55:30 -0400 Subject: [PATCH 13/14] Bug 1357206 followup: fix the Windows key issues in devtools tests and reenable the new behavior. --- .../client/debugger/new/test/mochitest/head.js | 14 ++++++++++++-- modules/libpref/init/all.js | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/devtools/client/debugger/new/test/mochitest/head.js b/devtools/client/debugger/new/test/mochitest/head.js index 7721fab322d9..355e30cc46c4 100644 --- a/devtools/client/debugger/new/test/mochitest/head.js +++ b/devtools/client/debugger/new/test/mochitest/head.js @@ -553,7 +553,17 @@ function invokeInTab(fnc) { } const isLinux = Services.appinfo.OS === "Linux"; +const isMac = Services.appinfo.OS === "Darwin"; const cmdOrCtrl = isLinux ? { ctrlKey: true } : { metaKey: true }; +// On Mac, going to beginning/end only works with meta+left/right. On +// Windows, it only works with home/end. On Linux, apparently, either +// ctrl+left/right or home/end work. +const endKey = isMac ? + { code: "VK_RIGHT", modifiers: cmdOrCtrl } : + { code: "VK_END" }; +const startKey = isMac ? + { code: "VK_LEFT", modifiers: cmdOrCtrl } : + { code: "VK_HOME" }; const keyMappings = { sourceSearch: { code: "p", modifiers: cmdOrCtrl }, fileSearch: { code: "f", modifiers: cmdOrCtrl }, @@ -562,8 +572,8 @@ const keyMappings = { Down: { code: "VK_DOWN" }, Right: { code: "VK_RIGHT" }, Left: { code: "VK_LEFT" }, - End: { code: "VK_RIGHT", modifiers: cmdOrCtrl }, - Start: { code: "VK_LEFT", modifiers: cmdOrCtrl }, + End: endKey, + Start: startKey, Tab: { code: "VK_TAB" }, Escape: { code: "VK_ESCAPE" }, pauseKey: { code: "VK_F8" }, diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 30ca916325ed..fb86122ba3d6 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -1253,7 +1253,7 @@ pref("dom.input.dirpicker", false); // Enable not moving the cursor to end when a text input or textarea has .value // set to the value it already has. By default, enabled. -pref("dom.input.skip_cursor_move_for_same_value_set", false); +pref("dom.input.skip_cursor_move_for_same_value_set", true); // Enables system messages and activities pref("dom.sysmsg.enabled", false); From 45a20c40748b3bb7ba23b83f9ac5b641c5916e14 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 2 May 2017 01:55:47 -0400 Subject: [PATCH 14/14] Bug 1358596. Restore check for sanity of slot indices on DOM objects that got lost. r=qdot --- dom/bindings/Codegen.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index d91b6e917036..09e6dcc2830d 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -3990,11 +3990,15 @@ class CGUpdateMemberSlotsMethod(CGAbstractStaticMethod): body += fill( """ + static_assert(${slot} < js::shadow::Object::MAX_FIXED_SLOTS, + "Not enough fixed slots to fit '${interface}.${member}. Ion's visitGetDOMMemberV/visitGetDOMMemberT assume StoreInSlot things are all in fixed slots."); if (!get_${member}(aCx, aWrapper, aObject, args)) { return false; } // Getter handled setting our reserved slots """, + slot=memberReservedSlot(m, self.descriptor), + interface=self.descriptor.interface.identifier.name, member=m.identifier.name) body += "\nreturn true;\n"