From 302fa305adeca1e81bb56a6d9ba3b810d5c01627 Mon Sep 17 00:00:00 2001 From: Neil Deakin Date: Tue, 4 Jun 2019 12:45:55 -0400 Subject: [PATCH 1/8] Bug 1519948, remove XUL box objects, r=bzbarsky --HG-- extra : rebase_source : 4af617fecf06512aed5882831e64e38d4e7e5e94 --- dom/base/Document.cpp | 78 --- dom/base/Document.h | 21 - dom/base/Element.cpp | 3 - dom/base/FragmentOrElement.cpp | 2 - dom/base/nsINode.cpp | 1 - dom/base/nsNodeUtils.cpp | 7 +- .../checkout/deqp/temp_externs/gecko_dom.js | 29 - dom/chrome-webidl/XULTreeElement.webidl | 4 +- dom/webidl/BoxObject.webidl | 33 -- dom/webidl/Document.webidl | 2 - dom/webidl/XULElement.webidl | 2 - dom/webidl/moz.build | 1 - dom/xul/XULDocument.cpp | 3 - dom/xul/nsXULElement.cpp | 8 - dom/xul/nsXULElement.h | 5 - layout/base/nsCSSFrameConstructor.cpp | 2 - layout/xul/BoxObject.cpp | 526 ------------------ layout/xul/BoxObject.h | 98 ---- layout/xul/moz.build | 12 - layout/xul/nsIBoxObject.idl | 27 - layout/xul/nsPIBoxObject.h | 42 -- layout/xul/tree/nsTreeColumns.cpp | 1 - layout/xul/tree/nsTreeSelection.cpp | 1 - 23 files changed, 4 insertions(+), 904 deletions(-) delete mode 100644 dom/webidl/BoxObject.webidl delete mode 100644 layout/xul/BoxObject.cpp delete mode 100644 layout/xul/BoxObject.h delete mode 100644 layout/xul/nsIBoxObject.idl delete mode 100644 layout/xul/nsPIBoxObject.h diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp index e3809626c676..0d949663f692 100644 --- a/dom/base/Document.cpp +++ b/dom/base/Document.cpp @@ -298,7 +298,6 @@ # include "nsXULPopupManager.h" # include "nsIDocShellTreeOwner.h" #endif -#include "mozilla/dom/BoxObject.h" #include "mozilla/DocLoadingTimelineMarker.h" @@ -1247,7 +1246,6 @@ Document::Document(const char* aContentType) mAutoFocusFired(false), mScrolledToRefAlready(false), mChangeScrollPosWhenScrollingToRef(false), - mHasWarnedAboutBoxObjects(false), mDelayFrameLoaderInitialization(false), mSynchronousDOMContentLoaded(false), mMaybeServiceWorkerControlled(false), @@ -1315,7 +1313,6 @@ Document::Document(const char* aContentType) mFlashClassification(FlashClassification::Unknown), mScrollAnchorAdjustmentLength(0), mScrollAnchorAdjustmentCount(0), - mBoxObjectTable(nullptr), mCurrentOrientationAngle(0), mCurrentOrientationType(OrientationType::Portrait_primary), mServoRestyleRootDirtyBits(0), @@ -1471,19 +1468,6 @@ void Document::GetFailedCertSecurityInfo( aInfo.mSubjectAltNames = subjectAltNames; } -void Document::ClearAllBoxObjects() { - if (mBoxObjectTable) { - for (auto iter = mBoxObjectTable->Iter(); !iter.Done(); iter.Next()) { - nsPIBoxObject* boxObject = iter.UserData(); - if (boxObject) { - boxObject->Clear(); - } - } - delete mBoxObjectTable; - mBoxObjectTable = nullptr; - } -} - bool Document::IsAboutPage() const { nsCOMPtr principal = NodePrincipal(); nsCOMPtr uri; @@ -1679,8 +1663,6 @@ Document::~Document() { delete mHeaderData; - ClearAllBoxObjects(); - mPendingTitleChangeEvent.Revoke(); mPlugins.Clear(); @@ -1817,15 +1799,6 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INTERNAL(Document) DocumentOrShadowRoot::Traverse(tmp, cb); - // The boxobject for an element will only exist as long as it's in the - // document, so we'll traverse the table here instead of from the element. - if (tmp->mBoxObjectTable) { - for (auto iter = tmp->mBoxObjectTable->Iter(); !iter.Done(); iter.Next()) { - NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mBoxObjectTable entry"); - cb.NoteXPCOMChild(iter.UserData()); - } - } - NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mChannel) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLayoutHistoryState) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mOnloadBlocker) @@ -1956,8 +1929,6 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Document) NS_IMPL_CYCLE_COLLECTION_UNLINK(mIntersectionObservers) - tmp->ClearAllBoxObjects(); - if (tmp->mListenerManager) { tmp->mListenerManager->Disconnect(); tmp->UnsetFlags(NODE_HAS_LISTENERMANAGER); @@ -7696,55 +7667,6 @@ void Document::DoNotifyPossibleTitleChange() { CanBubble::eYes, Cancelable::eYes); } -already_AddRefed Document::GetBoxObjectFor(Element* aElement, - ErrorResult& aRv) { - if (!aElement) { - aRv.Throw(NS_ERROR_UNEXPECTED); - return nullptr; - } - - Document* doc = aElement->OwnerDoc(); - if (doc != this) { - aRv.Throw(NS_ERROR_DOM_WRONG_DOCUMENT_ERR); - return nullptr; - } - - if (!mHasWarnedAboutBoxObjects && !aElement->IsXULElement()) { - mHasWarnedAboutBoxObjects = true; - nsContentUtils::ReportToConsole( - nsIScriptError::warningFlag, NS_LITERAL_CSTRING("BoxObjects"), this, - nsContentUtils::eDOM_PROPERTIES, "UseOfGetBoxObjectForWarning"); - } - - if (!mBoxObjectTable) { - mBoxObjectTable = - new nsRefPtrHashtable, BoxObject>(6); - } - - RefPtr boxObject; - auto entry = mBoxObjectTable->LookupForAdd(aElement); - if (entry) { - boxObject = entry.Data(); - return boxObject.forget(); - } - - boxObject = new BoxObject(); - boxObject->Init(aElement); - entry.OrInsert([&boxObject]() { return boxObject; }); - - return boxObject.forget(); -} - -void Document::ClearBoxObjectFor(nsIContent* aContent) { - if (mBoxObjectTable) { - if (auto entry = mBoxObjectTable->Lookup(aContent)) { - nsPIBoxObject* boxObject = entry.Data(); - boxObject->Clear(); - entry.Remove(); - } - } -} - already_AddRefed Document::MatchMedia( const nsAString& aMediaQueryList, CallerType aCallerType) { RefPtr result = diff --git a/dom/base/Document.h b/dom/base/Document.h index d0b6ebadeca9..b308f157cd82 100644 --- a/dom/base/Document.h +++ b/dom/base/Document.h @@ -159,7 +159,6 @@ namespace dom { class Animation; class AnonymousContent; class Attr; -class BoxObject; class XULBroadcastManager; class XULPersist; class ClientInfo; @@ -1623,8 +1622,6 @@ class Document : public nsINode, void DoUnblockOnload(); - void ClearAllBoxObjects(); - void MaybeEndOutermostXBLUpdate(); void RetrieveRelevantHeaders(nsIChannel* aChannel); @@ -2532,20 +2529,6 @@ class Document : public nsINode, // Refreshes the hrefs of all the links in the document. void RefreshLinkHrefs(); - /** - * Resets and removes a box object from the document's box object cache - * - * @param aElement canonical nsIContent pointer of the box object's element - */ - void ClearBoxObjectFor(nsIContent* aContent); - - /** - * Get the box object for an element. This is not exposed through a - * scriptable interface except for XUL documents. - */ - already_AddRefed GetBoxObjectFor(Element* aElement, - ErrorResult& aRv); - /** * Support for window.matchMedia() */ @@ -4483,8 +4466,6 @@ class Document : public nsINode, bool mScrolledToRefAlready : 1; bool mChangeScrollPosWhenScrollingToRef : 1; - bool mHasWarnedAboutBoxObjects : 1; - bool mDelayFrameLoaderInitialization : 1; bool mSynchronousDOMContentLoaded : 1; @@ -4879,8 +4860,6 @@ class Document : public nsINode, RefPtr mScriptLoader; - nsRefPtrHashtable, BoxObject>* mBoxObjectTable; - // Tracker for animations that are waiting to start. // nullptr until GetOrCreatePendingAnimationTracker is called. RefPtr mPendingAnimationTracker; diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index 25092c5a36fd..a30cd3a30eac 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -94,7 +94,6 @@ #include "nsBindingManager.h" #include "nsXBLBinding.h" #include "nsPIDOMWindow.h" -#include "nsPIBoxObject.h" #include "mozilla/dom/DOMRect.h" #include "nsSVGUtils.h" #include "nsLayoutUtils.h" @@ -1942,8 +1941,6 @@ void Element::UnbindFromTree(bool aNullParent) { } } - document->ClearBoxObjectFor(this); - // Disconnected must be enqueued whenever a connected custom element becomes // disconnected. CustomElementData* data = GetCustomElementData(); diff --git a/dom/base/FragmentOrElement.cpp b/dom/base/FragmentOrElement.cpp index 10f027e35397..8e0597cbb312 100644 --- a/dom/base/FragmentOrElement.cpp +++ b/dom/base/FragmentOrElement.cpp @@ -75,7 +75,6 @@ #include "nsFrameLoader.h" #include "nsXBLBinding.h" #include "nsPIDOMWindow.h" -#include "nsPIBoxObject.h" #include "nsSVGUtils.h" #include "nsLayoutUtils.h" #include "nsGkAtoms.h" @@ -1173,7 +1172,6 @@ void FragmentOrElement::DestroyContent() { document->BindingManager()->RemovedFromDocument(this, document, nsBindingManager::eRunDtor); - document->ClearBoxObjectFor(this); #ifdef DEBUG uint32_t oldChildCount = GetChildCount(); diff --git a/dom/base/nsINode.cpp b/dom/base/nsINode.cpp index 64a6d8ffc953..13f3982029e6 100644 --- a/dom/base/nsINode.cpp +++ b/dom/base/nsINode.cpp @@ -84,7 +84,6 @@ #include "nsNameSpaceManager.h" #include "nsNodeInfoManager.h" #include "nsNodeUtils.h" -#include "nsPIBoxObject.h" #include "nsPIDOMWindow.h" #include "nsPresContext.h" #include "nsString.h" diff --git a/dom/base/nsNodeUtils.cpp b/dom/base/nsNodeUtils.cpp index 5dfcf3e766a9..27b94ec2396c 100644 --- a/dom/base/nsNodeUtils.cpp +++ b/dom/base/nsNodeUtils.cpp @@ -320,10 +320,8 @@ void nsNodeUtils::LastRelease(nsINode* aNode) { aNode->UnsetFlags(NODE_HAS_LISTENERMANAGER); } - if (Element* element = Element::FromNode(aNode)) { - element->OwnerDoc()->ClearBoxObjectFor(element); - NS_ASSERTION(!element->GetXBLBinding(), "Node has binding on destruction"); - } + NS_ASSERTION(!Element::FromNode(aNode) || + !Element::FromNode(aNode)->GetXBLBinding(), "Node has binding on destruction"); aNode->ReleaseWrapper(aNode); @@ -437,7 +435,6 @@ already_AddRefed nsNodeUtils::CloneAndAdopt( Document* oldDoc = aNode->OwnerDoc(); bool wasRegistered = false; if (elem) { - oldDoc->ClearBoxObjectFor(elem); wasRegistered = oldDoc->UnregisterActivityObserver(elem); } diff --git a/dom/canvas/test/webgl-conf/checkout/deqp/temp_externs/gecko_dom.js b/dom/canvas/test/webgl-conf/checkout/deqp/temp_externs/gecko_dom.js index a3314d812342..d518d2701c74 100644 --- a/dom/canvas/test/webgl-conf/checkout/deqp/temp_externs/gecko_dom.js +++ b/dom/canvas/test/webgl-conf/checkout/deqp/temp_externs/gecko_dom.js @@ -561,14 +561,6 @@ Document.prototype.writeln = function(text) {}; Document.prototype.ononline; Document.prototype.onoffline; -// XUL -/** - * @see http://developer.mozilla.org/en/DOM/document.getBoxObjectFor - * @return {BoxObject} - * @nosideeffects - */ -Document.prototype.getBoxObjectFor = function(element) {}; - // From: // http://lxr.mozilla.org/mozilla1.8/source/dom/public/idl/range/nsIDOMNSRange.idl @@ -1076,27 +1068,6 @@ Plugin.prototype.length; /** @type {string} */ Plugin.prototype.name; -/** @constructor */ -function BoxObject() {} - -/** @type {Element} */ -BoxObject.prototype.element; - -/** @type {number} */ -BoxObject.prototype.screenX; - -/** @type {number} */ -BoxObject.prototype.screenY; - -/** @type {number} */ -BoxObject.prototype.x; - -/** @type {number} */ -BoxObject.prototype.y; - -/** @type {number} */ -BoxObject.prototype.width; - /** * @type {number} diff --git a/dom/chrome-webidl/XULTreeElement.webidl b/dom/chrome-webidl/XULTreeElement.webidl index 9f9d3a4a89a9..b9dd51185fa1 100644 --- a/dom/chrome-webidl/XULTreeElement.webidl +++ b/dom/chrome-webidl/XULTreeElement.webidl @@ -114,7 +114,7 @@ interface XULTreeElement : XULElement * returns -1 for invalid mouse coordinates. * * The coordinate system is the client coordinate system for the - * document this boxObject lives in, and the units are CSS pixels. + * document this tree lives in, and the units are CSS pixels. */ long getRowAt(long x, long y); @@ -126,7 +126,7 @@ interface XULTreeElement : XULElement * "cell", "twisty", "image", and "text". * * The coordinate system is the client coordinate system for the - * document this boxObject lives in, and the units are CSS pixels. + * document this tree lives in, and the units are CSS pixels. */ [Throws] TreeCellInfo getCellAt(long x, long y); diff --git a/dom/webidl/BoxObject.webidl b/dom/webidl/BoxObject.webidl deleted file mode 100644 index 8bebd6c9eb3e..000000000000 --- a/dom/webidl/BoxObject.webidl +++ /dev/null @@ -1,33 +0,0 @@ -/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* 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/. - */ - -[Func="IsChromeOrXBL"] -interface BoxObject { - readonly attribute Element? element; - - readonly attribute long x; - readonly attribute long y; - [Throws] - readonly attribute long screenX; - [Throws] - readonly attribute long screenY; - readonly attribute long width; - readonly attribute long height; - - nsISupports? getPropertyAsSupports(DOMString propertyName); - void setPropertyAsSupports(DOMString propertyName, nsISupports value); - [Throws] - DOMString? getProperty(DOMString propertyName); - void setProperty(DOMString propertyName, DOMString propertyValue); - void removeProperty(DOMString propertyName); - - // for stepping through content in the expanded dom with box-ordinal-group order - readonly attribute Element? parentBox; - readonly attribute Element? firstChild; - readonly attribute Element? lastChild; - readonly attribute Element? nextSibling; - readonly attribute Element? previousSibling; -}; diff --git a/dom/webidl/Document.webidl b/dom/webidl/Document.webidl index 2f811e33a6be..28f77d6a0519 100644 --- a/dom/webidl/Document.webidl +++ b/dom/webidl/Document.webidl @@ -192,8 +192,6 @@ partial interface Document { * etc. */ [Func="IsChromeOrXBLOrUAWidget"] readonly attribute boolean mozSyntheticDocument; - [Throws, Func="IsChromeOrXBL"] - BoxObject? getBoxObjectFor(Element? element); /** * Returns the script element whose script is currently being processed. * diff --git a/dom/webidl/XULElement.webidl b/dom/webidl/XULElement.webidl index 5bbdeed40453..55ba385034a2 100644 --- a/dom/webidl/XULElement.webidl +++ b/dom/webidl/XULElement.webidl @@ -76,8 +76,6 @@ interface XULElement : Element { [Throws, ChromeOnly] readonly attribute XULControllers controllers; - [Throws] - readonly attribute BoxObject? boxObject; [SetterThrows] attribute long tabIndex; diff --git a/dom/webidl/moz.build b/dom/webidl/moz.build index fcb5775600be..9c095ef32cee 100644 --- a/dom/webidl/moz.build +++ b/dom/webidl/moz.build @@ -400,7 +400,6 @@ WEBIDL_FILES = [ 'BeforeUnloadEvent.webidl', 'BiquadFilterNode.webidl', 'Blob.webidl', - 'BoxObject.webidl', 'BroadcastChannel.webidl', 'BrowserElement.webidl', 'BrowserElementDictionaries.webidl', diff --git a/dom/xul/XULDocument.cpp b/dom/xul/XULDocument.cpp index 032642ab45d2..d3b5e040ded6 100644 --- a/dom/xul/XULDocument.cpp +++ b/dom/xul/XULDocument.cpp @@ -28,7 +28,6 @@ #include "XULDocument.h" #include "nsError.h" -#include "nsIBoxObject.h" #include "nsView.h" #include "nsViewManager.h" #include "nsIContentViewer.h" @@ -43,8 +42,6 @@ #include "nsDocElementCreatedNotificationRunner.h" #include "nsNetUtil.h" #include "nsParserCIID.h" -#include "nsPIBoxObject.h" -#include "mozilla/dom/BoxObject.h" #include "nsString.h" #include "nsPIDOMWindow.h" #include "nsPIWindowRoot.h" diff --git a/dom/xul/nsXULElement.cpp b/dom/xul/nsXULElement.cpp index fcfec2718acf..7328fccc58e6 100644 --- a/dom/xul/nsXULElement.cpp +++ b/dom/xul/nsXULElement.cpp @@ -41,8 +41,6 @@ #include "nsStyleConsts.h" #include "nsString.h" #include "nsXULControllers.h" -#include "nsIBoxObject.h" -#include "nsPIBoxObject.h" #include "XULDocument.h" #include "nsXULPopupListener.h" #include "nsContentUtils.h" @@ -81,7 +79,6 @@ #include "XULTreeElement.h" #include "mozilla/dom/XULElementBinding.h" -#include "mozilla/dom/BoxObject.h" #include "mozilla/dom/XULBroadcastManager.h" #include "mozilla/dom/MouseEventBinding.h" #include "mozilla/dom/MutationEventBinding.h" @@ -1136,11 +1133,6 @@ nsIControllers* nsXULElement::GetControllers(ErrorResult& rv) { return Controllers(); } -already_AddRefed nsXULElement::GetBoxObject(ErrorResult& rv) { - // XXX sXBL/XBL2 issue! Owner or current document? - return OwnerDoc()->GetBoxObjectFor(this, rv); -} - void nsXULElement::Click(CallerType aCallerType) { ClickWithInputSource(MouseEvent_Binding::MOZ_SOURCE_UNKNOWN, aCallerType == CallerType::System); diff --git a/dom/xul/nsXULElement.h b/dom/xul/nsXULElement.h index eb1d984c3d47..45b38a49e25b 100644 --- a/dom/xul/nsXULElement.h +++ b/dom/xul/nsXULElement.h @@ -46,7 +46,6 @@ namespace css { class StyleRule; } // namespace css namespace dom { -class BoxObject; class HTMLIFrameElement; class PrototypeDocumentContentSink; enum class CallerType : uint32_t; @@ -510,10 +509,6 @@ class nsXULElement : public nsStyledElement { SetXULBoolAttr(nsGkAtoms::allowevents, aAllowEvents); } nsIControllers* GetControllers(mozilla::ErrorResult& rv); - // Note: this can only fail if the do_CreateInstance for the boxobject - // contact fails for some reason. - already_AddRefed GetBoxObject( - mozilla::ErrorResult& rv); void Click(mozilla::dom::CallerType aCallerType); MOZ_CAN_RUN_SCRIPT_BOUNDARY void DoCommand(); // Style() inherited from nsStyledElement diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index 12e53030f245..6ee53ade871d 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -48,7 +48,6 @@ #include "nsStyleConsts.h" #ifdef MOZ_XUL # include "nsXULElement.h" -# include "mozilla/dom/BoxObject.h" #endif // MOZ_XUL #include "nsContainerFrame.h" #include "nsNameSpaceManager.h" @@ -207,7 +206,6 @@ static FrameCtorDebugFlags gFlags[] = { # include "nsMenuFrame.h" # include "nsPopupSetFrame.h" # include "nsTreeColFrame.h" -# include "nsIBoxObject.h" # include "nsXULLabelFrame.h" //------------------------------------------------------------------ diff --git a/layout/xul/BoxObject.cpp b/layout/xul/BoxObject.cpp deleted file mode 100644 index 058984a351b9..000000000000 --- a/layout/xul/BoxObject.cpp +++ /dev/null @@ -1,526 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=2 et sw=2 tw=80: */ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#include "mozilla/dom/BoxObject.h" -#include "nsCOMPtr.h" -#include "mozilla/PresShell.h" -#include "mozilla/dom/Document.h" -#include "nsPresContext.h" -#include "nsIContent.h" -#include "nsContainerFrame.h" -#include "nsIDocShell.h" -#include "nsReadableUtils.h" -#include "nsView.h" -#include "nsLayoutUtils.h" -#include "nsISupportsPrimitives.h" -#include "nsSupportsPrimitives.h" -#include "mozilla/dom/Element.h" -#include "nsComponentManagerUtils.h" -#include "mozilla/dom/BoxObjectBinding.h" - -// Implementation ///////////////////////////////////////////////////////////// - -namespace mozilla { -namespace dom { - -// Static member variable initialization - -// Implement our nsISupports methods -NS_IMPL_CYCLE_COLLECTION_CLASS(BoxObject) -NS_IMPL_CYCLE_COLLECTING_ADDREF(BoxObject) -NS_IMPL_CYCLE_COLLECTING_RELEASE(BoxObject) - -// QueryInterface implementation for BoxObject -NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(BoxObject) - NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY - NS_INTERFACE_MAP_ENTRY(nsIBoxObject) - NS_INTERFACE_MAP_ENTRY(nsPIBoxObject) - NS_INTERFACE_MAP_ENTRY(nsISupports) -NS_INTERFACE_MAP_END - -NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(BoxObject) - // XXX jmorton: why aren't we unlinking mPropertyTable? - NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER -NS_IMPL_CYCLE_COLLECTION_UNLINK_END - -NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(BoxObject) - if (tmp->mPropertyTable) { - for (auto iter = tmp->mPropertyTable->Iter(); !iter.Done(); iter.Next()) { - cb.NoteXPCOMChild(iter.UserData()); - } - } -NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END - -NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(BoxObject) - -// Constructors/Destructors -BoxObject::BoxObject() : mContent(nullptr) {} - -BoxObject::~BoxObject() {} - -NS_IMETHODIMP -BoxObject::GetElement(Element** aResult) { - RefPtr element = mContent; - element.forget(aResult); - return NS_OK; -} - -// nsPIBoxObject ////////////////////////////////////////////////////////////// - -nsresult BoxObject::Init(Element* aElement) { - mContent = aElement; - return NS_OK; -} - -void BoxObject::Clear() { - mPropertyTable = nullptr; - mContent = nullptr; -} - -void BoxObject::ClearCachedValues() {} - -nsIFrame* BoxObject::GetFrame() const { - if (!GetPresShell() || !mContent) { - return nullptr; - } - return mContent->GetPrimaryFrame(); -} - -nsIFrame* BoxObject::GetFrameWithFlushPendingNotifications() { - RefPtr presShell = GetPresShellWithFlushPendingNotifications(); - if (!presShell) { - return nullptr; - } - - // If we didn't flush layout when getting the presshell, we should at least - // flush to make sure our frame model is up to date. - // XXXbz should flush on document, no? Except people call this from - // frame code, maybe? - presShell->FlushPendingNotifications(FlushType::Frames); - - // The flush might have killed mContent. - if (!mContent) { - return nullptr; - } - - return mContent->GetPrimaryFrame(); -} - -PresShell* BoxObject::GetPresShell() const { - if (!mContent) { - return nullptr; - } - - Document* doc = mContent->GetComposedDoc(); - if (!doc) { - return nullptr; - } - - return doc->GetPresShell(); -} - -PresShell* BoxObject::GetPresShellWithFlushPendingNotifications() { - if (!mContent) { - return nullptr; - } - - RefPtr doc = mContent->GetComposedDoc(); - if (!doc) { - return nullptr; - } - - doc->FlushPendingNotifications(FlushType::Layout); - - return doc->GetPresShell(); -} - -nsresult BoxObject::GetOffsetRect(nsIntRect& aRect) { - aRect.SetRect(0, 0, 0, 0); - - if (!mContent) return NS_ERROR_NOT_INITIALIZED; - - // Get the Frame for our content - nsIFrame* frame = GetFrameWithFlushPendingNotifications(); - if (frame) { - // Get its origin - nsPoint origin = frame->GetPositionIgnoringScrolling(); - - // Find the frame parent whose content is the document element. - Element* docElement = mContent->GetComposedDoc()->GetRootElement(); - nsIFrame* parent = frame->GetParent(); - for (;;) { - // If we've hit the document element, break here - if (parent->GetContent() == docElement) { - break; - } - - nsIFrame* next = parent->GetParent(); - if (!next) { - NS_WARNING("We should have hit the document element..."); - origin += parent->GetPosition(); - break; - } - - // Add the parent's origin to our own to get to the - // right coordinate system - origin += next->GetPositionOfChildIgnoringScrolling(parent); - parent = next; - } - - // For the origin, add in the border for the frame - const nsStyleBorder* border = frame->StyleBorder(); - origin.x += border->GetComputedBorderWidth(eSideLeft); - origin.y += border->GetComputedBorderWidth(eSideTop); - - // And subtract out the border for the parent - const nsStyleBorder* parentBorder = parent->StyleBorder(); - origin.x -= parentBorder->GetComputedBorderWidth(eSideLeft); - origin.y -= parentBorder->GetComputedBorderWidth(eSideTop); - - aRect.x = nsPresContext::AppUnitsToIntCSSPixels(origin.x); - aRect.y = nsPresContext::AppUnitsToIntCSSPixels(origin.y); - - // Get the union of all rectangles in this and continuation frames. - // It doesn't really matter what we use as aRelativeTo here, since - // we only care about the size. Using 'parent' might make things - // a bit faster by speeding up the internal GetOffsetTo operations. - nsRect rcFrame = nsLayoutUtils::GetAllInFlowRectsUnion(frame, parent); - aRect.width = nsPresContext::AppUnitsToIntCSSPixels(rcFrame.width); - aRect.height = nsPresContext::AppUnitsToIntCSSPixels(rcFrame.height); - } - - return NS_OK; -} - -nsresult BoxObject::GetScreenPosition(nsIntPoint& aPoint) { - aPoint.x = aPoint.y = 0; - - if (!mContent) return NS_ERROR_NOT_INITIALIZED; - - nsIFrame* frame = GetFrameWithFlushPendingNotifications(); - if (frame) { - CSSIntRect rect = frame->GetScreenRect(); - aPoint.x = rect.x; - aPoint.y = rect.y; - } - - return NS_OK; -} - -NS_IMETHODIMP -BoxObject::GetX(int32_t* aResult) { - nsIntRect rect; - GetOffsetRect(rect); - *aResult = rect.x; - return NS_OK; -} - -NS_IMETHODIMP -BoxObject::GetY(int32_t* aResult) { - nsIntRect rect; - GetOffsetRect(rect); - *aResult = rect.y; - return NS_OK; -} - -NS_IMETHODIMP -BoxObject::GetWidth(int32_t* aResult) { - nsIntRect rect; - GetOffsetRect(rect); - *aResult = rect.width; - return NS_OK; -} - -NS_IMETHODIMP -BoxObject::GetHeight(int32_t* aResult) { - nsIntRect rect; - GetOffsetRect(rect); - *aResult = rect.height; - return NS_OK; -} - -NS_IMETHODIMP -BoxObject::GetScreenX(int32_t* _retval) { - nsIntPoint position; - nsresult rv = GetScreenPosition(position); - if (NS_FAILED(rv)) return rv; - *_retval = position.x; - return NS_OK; -} - -NS_IMETHODIMP -BoxObject::GetScreenY(int32_t* _retval) { - nsIntPoint position; - nsresult rv = GetScreenPosition(position); - if (NS_FAILED(rv)) return rv; - *_retval = position.y; - return NS_OK; -} - -NS_IMETHODIMP -BoxObject::GetPropertyAsSupports(const char16_t* aPropertyName, - nsISupports** aResult) { - NS_ENSURE_ARG(aPropertyName && *aPropertyName); - if (!mPropertyTable) { - *aResult = nullptr; - return NS_OK; - } - nsDependentString propertyName(aPropertyName); - mPropertyTable->Get(propertyName, aResult); // Addref here. - return NS_OK; -} - -NS_IMETHODIMP -BoxObject::SetPropertyAsSupports(const char16_t* aPropertyName, - nsISupports* aValue) { - NS_ENSURE_ARG(aPropertyName && *aPropertyName); - - if (!mPropertyTable) { - mPropertyTable = new nsInterfaceHashtable(4); - } - - nsDependentString propertyName(aPropertyName); - mPropertyTable->Put(propertyName, aValue); - return NS_OK; -} - -NS_IMETHODIMP -BoxObject::GetProperty(const char16_t* aPropertyName, char16_t** aResult) { - nsCOMPtr data; - nsresult rv = GetPropertyAsSupports(aPropertyName, getter_AddRefs(data)); - NS_ENSURE_SUCCESS(rv, rv); - - if (!data) { - *aResult = nullptr; - return NS_OK; - } - - nsCOMPtr supportsStr = do_QueryInterface(data); - if (!supportsStr) { - return NS_ERROR_FAILURE; - } - - return supportsStr->ToString(aResult); -} - -NS_IMETHODIMP -BoxObject::SetProperty(const char16_t* aPropertyName, - const char16_t* aPropertyValue) { - NS_ENSURE_ARG(aPropertyName && *aPropertyName); - - nsDependentString propertyName(aPropertyName); - nsDependentString propertyValue; - if (aPropertyValue) { - propertyValue.Rebind(aPropertyValue); - } else { - propertyValue.SetIsVoid(true); - } - - nsCOMPtr supportsStr( - do_CreateInstance(NS_SUPPORTS_STRING_CONTRACTID)); - NS_ENSURE_TRUE(supportsStr, NS_ERROR_OUT_OF_MEMORY); - supportsStr->SetData(propertyValue); - - return SetPropertyAsSupports(aPropertyName, supportsStr); -} - -NS_IMETHODIMP -BoxObject::RemoveProperty(const char16_t* aPropertyName) { - NS_ENSURE_ARG(aPropertyName && *aPropertyName); - - if (!mPropertyTable) return NS_OK; - - nsDependentString propertyName(aPropertyName); - mPropertyTable->Remove(propertyName); - return NS_OK; -} - -Element* BoxObject::GetParentBox() { - nsIFrame* frame = GetFrame(); - if (!frame) { - return nullptr; - } - - nsIFrame* parent = frame->GetParent(); - if (!parent) { - return nullptr; - } - - nsIContent* parentContent = parent->GetContent(); - // In theory parent could be viewport, and then parentContent is null. - if (parentContent && parentContent->IsElement()) { - return parentContent->AsElement(); - } - - return nullptr; -} - -Element* BoxObject::GetFirstChild() { - nsIFrame* frame = GetFrame(); - if (!frame) { - return nullptr; - } - - nsIFrame* firstFrame = frame->PrincipalChildList().FirstChild(); - if (!firstFrame) { - return nullptr; - } - - nsIContent* content = firstFrame->GetContent(); - if (content->IsElement()) { - return content->AsElement(); - } - - return nullptr; -} - -Element* BoxObject::GetLastChild() { - nsIFrame* frame = GetFrame(); - if (!frame) { - return nullptr; - } - return GetPreviousSibling(frame, nullptr); -} - -Element* BoxObject::GetNextSibling() { - nsIFrame* frame = GetFrame(); - if (!frame) { - return nullptr; - } - - nsIFrame* nextFrame = frame->GetNextSibling(); - if (!nextFrame) { - return nullptr; - } - - nsIContent* content = nextFrame->GetContent(); - if (content->IsElement()) { - return content->AsElement(); - } - - return nullptr; -} - -Element* BoxObject::GetPreviousSibling() { - nsIFrame* frame = GetFrame(); - if (!frame) { - return nullptr; - } - nsIFrame* parentFrame = frame->GetParent(); - if (!parentFrame) { - return nullptr; - } - return GetPreviousSibling(parentFrame, frame); -} - -Element* BoxObject::GetPreviousSibling(nsIFrame* aParentFrame, - nsIFrame* aFrame) { - nsIFrame* nextFrame = aParentFrame->PrincipalChildList().FirstChild(); - nsIFrame* prevFrame = nullptr; - while (nextFrame) { - if (nextFrame == aFrame) break; - prevFrame = nextFrame; - nextFrame = nextFrame->GetNextSibling(); - } - - if (!prevFrame) { - return nullptr; - } - - nsIContent* content = prevFrame->GetContent(); - if (!content->IsElement()) { - return nullptr; - } - - return content->AsElement(); -} - -Element* BoxObject::GetParentObject() const { return mContent; } - -JSObject* BoxObject::WrapObject(JSContext* aCx, - JS::Handle aGivenProto) { - return BoxObject_Binding::Wrap(aCx, this, aGivenProto); -} - -Element* BoxObject::GetElement() const { return mContent; } - -int32_t BoxObject::X() { - int32_t ret = 0; - GetX(&ret); - return ret; -} - -int32_t BoxObject::Y() { - int32_t ret = 0; - GetY(&ret); - return ret; -} - -int32_t BoxObject::GetScreenX(ErrorResult& aRv) { - int32_t ret = 0; - aRv = GetScreenX(&ret); - return ret; -} - -int32_t BoxObject::GetScreenY(ErrorResult& aRv) { - int32_t ret = 0; - aRv = GetScreenY(&ret); - return ret; -} - -int32_t BoxObject::Width() { - int32_t ret = 0; - GetWidth(&ret); - return ret; -} - -int32_t BoxObject::Height() { - int32_t ret = 0; - GetHeight(&ret); - return ret; -} - -already_AddRefed BoxObject::GetPropertyAsSupports( - const nsAString& propertyName) { - nsCOMPtr ret; - GetPropertyAsSupports(PromiseFlatString(propertyName).get(), - getter_AddRefs(ret)); - return ret.forget(); -} - -void BoxObject::SetPropertyAsSupports(const nsAString& propertyName, - nsISupports* value) { - SetPropertyAsSupports(PromiseFlatString(propertyName).get(), value); -} - -void BoxObject::GetProperty(const nsAString& propertyName, nsString& aRetVal, - ErrorResult& aRv) { - nsCOMPtr data(GetPropertyAsSupports(propertyName)); - if (!data) { - return; - } - - nsCOMPtr supportsStr(do_QueryInterface(data)); - if (!supportsStr) { - aRv.Throw(NS_ERROR_FAILURE); - return; - } - - supportsStr->GetData(aRetVal); -} - -void BoxObject::SetProperty(const nsAString& propertyName, - const nsAString& propertyValue) { - SetProperty(PromiseFlatString(propertyName).get(), - PromiseFlatString(propertyValue).get()); -} - -void BoxObject::RemoveProperty(const nsAString& propertyName) { - RemoveProperty(PromiseFlatString(propertyName).get()); -} - -} // namespace dom -} // namespace mozilla diff --git a/layout/xul/BoxObject.h b/layout/xul/BoxObject.h deleted file mode 100644 index 545c274ec6e3..000000000000 --- a/layout/xul/BoxObject.h +++ /dev/null @@ -1,98 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=2 et sw=2 tw=80: */ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#ifndef mozilla_dom_BoxObject_h__ -#define mozilla_dom_BoxObject_h__ - -#include "mozilla/Attributes.h" -#include "mozilla/ErrorResult.h" -#include "nsCOMPtr.h" -#include "nsIBoxObject.h" -#include "nsPIBoxObject.h" -#include "nsPoint.h" -#include "nsAutoPtr.h" -#include "nsHashKeys.h" -#include "nsInterfaceHashtable.h" -#include "nsCycleCollectionParticipant.h" -#include "nsWrapperCache.h" -#include "nsRect.h" - -class nsIFrame; - -namespace mozilla { - -class PresShell; - -namespace dom { - -class Element; - -class BoxObject : public nsPIBoxObject, public nsWrapperCache { - NS_DECL_CYCLE_COLLECTING_ISUPPORTS - NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(BoxObject) - NS_DECL_NSIBOXOBJECT - - public: - BoxObject(); - - // nsPIBoxObject - virtual nsresult Init(Element* aElement) override; - virtual void Clear() override; - virtual void ClearCachedValues() override; - - MOZ_CAN_RUN_SCRIPT_BOUNDARY nsresult GetOffsetRect(nsIntRect& aRect); - MOZ_CAN_RUN_SCRIPT_BOUNDARY nsresult GetScreenPosition(nsIntPoint& aPoint); - - // Given a parent frame and a child frame, find the frame whose - // next sibling is the given child frame and return its element - static Element* GetPreviousSibling(nsIFrame* aParentFrame, nsIFrame* aFrame); - - // WebIDL (wraps old impls) - Element* GetParentObject() const; - virtual JSObject* WrapObject(JSContext* aCx, - JS::Handle aGivenProto) override; - - Element* GetElement() const; - - int32_t X(); - int32_t Y(); - int32_t GetScreenX(ErrorResult& aRv); - int32_t GetScreenY(ErrorResult& aRv); - int32_t Width(); - int32_t Height(); - - already_AddRefed GetPropertyAsSupports( - const nsAString& propertyName); - void SetPropertyAsSupports(const nsAString& propertyName, nsISupports* value); - void GetProperty(const nsAString& propertyName, nsString& aRetVal, - ErrorResult& aRv); - void SetProperty(const nsAString& propertyName, - const nsAString& propertyValue); - void RemoveProperty(const nsAString& propertyName); - - Element* GetParentBox(); - Element* GetFirstChild(); - Element* GetLastChild(); - Element* GetNextSibling(); - Element* GetPreviousSibling(); - - protected: - virtual ~BoxObject(); - - nsIFrame* GetFrame() const; - MOZ_CAN_RUN_SCRIPT nsIFrame* GetFrameWithFlushPendingNotifications(); - PresShell* GetPresShell() const; - MOZ_CAN_RUN_SCRIPT PresShell* GetPresShellWithFlushPendingNotifications(); - - nsAutoPtr> mPropertyTable; - - Element* mContent; // [WEAK] -}; - -} // namespace dom -} // namespace mozilla - -#endif diff --git a/layout/xul/moz.build b/layout/xul/moz.build index d66c4843d512..3998afdec227 100644 --- a/layout/xul/moz.build +++ b/layout/xul/moz.build @@ -15,26 +15,14 @@ if CONFIG['ENABLE_TESTS']: MOCHITEST_CHROME_MANIFESTS += ['test/chrome.ini'] BROWSER_CHROME_MANIFESTS += ['test/browser.ini'] -XPIDL_SOURCES += [ - 'nsIBoxObject.idl', -] - -XPIDL_MODULE = 'layout_xul' - EXPORTS += [ 'nsBox.h', 'nsIScrollbarMediator.h', - 'nsPIBoxObject.h', 'nsXULPopupManager.h', 'nsXULTooltipListener.h', ] -EXPORTS.mozilla.dom += [ - 'BoxObject.h', -] - UNIFIED_SOURCES += [ - 'BoxObject.cpp', 'nsBox.cpp', 'nsBoxFrame.cpp', 'nsBoxLayout.cpp', diff --git a/layout/xul/nsIBoxObject.idl b/layout/xul/nsIBoxObject.idl deleted file mode 100644 index b3b0d97cf47b..000000000000 --- a/layout/xul/nsIBoxObject.idl +++ /dev/null @@ -1,27 +0,0 @@ -/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* 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" - -webidl Element; - -[scriptable, uuid(ce572460-b0f2-4650-a9e7-c53a99d3b6ad)] -interface nsIBoxObject : nsISupports -{ - readonly attribute Element element; - - readonly attribute long x; - readonly attribute long y; - readonly attribute long screenX; - readonly attribute long screenY; - readonly attribute long width; - readonly attribute long height; - - nsISupports getPropertyAsSupports(in wstring propertyName); - void setPropertyAsSupports(in wstring propertyName, in nsISupports value); - wstring getProperty(in wstring propertyName); - void setProperty(in wstring propertyName, in wstring propertyValue); - void removeProperty(in wstring propertyName); -}; diff --git a/layout/xul/nsPIBoxObject.h b/layout/xul/nsPIBoxObject.h deleted file mode 100644 index 01bfa5c129f3..000000000000 --- a/layout/xul/nsPIBoxObject.h +++ /dev/null @@ -1,42 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=2 et sw=2 tw=80: */ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#ifndef nsPIBoxObject_h___ -#define nsPIBoxObject_h___ - -#include "nsIBoxObject.h" - -// {2b8bb262-1b0f-4572-ba87-5d4ae4954445} -#define NS_PIBOXOBJECT_IID \ - { \ - 0x2b8bb262, 0x1b0f, 0x4572, { \ - 0xba, 0x87, 0x5d, 0x4a, 0xe4, 0x95, 0x44, 0x45 \ - } \ - } - -namespace mozilla { -namespace dom { -class Element; -} // namespace dom -} // namespace mozilla - -class nsPIBoxObject : public nsIBoxObject { - public: - NS_DECLARE_STATIC_IID_ACCESSOR(NS_PIBOXOBJECT_IID) - - virtual nsresult Init(mozilla::dom::Element* aElement) = 0; - - // Drop the weak ref to the content node as needed - virtual void Clear() = 0; - - // The values cached by the implementation of this interface should be - // cleared when this method is called. - virtual void ClearCachedValues() = 0; -}; - -NS_DEFINE_STATIC_IID_ACCESSOR(nsPIBoxObject, NS_PIBOXOBJECT_IID) - -#endif diff --git a/layout/xul/tree/nsTreeColumns.cpp b/layout/xul/tree/nsTreeColumns.cpp index b5b8ff1d724f..4236608792a6 100644 --- a/layout/xul/tree/nsTreeColumns.cpp +++ b/layout/xul/tree/nsTreeColumns.cpp @@ -6,7 +6,6 @@ #include "nsNameSpaceManager.h" #include "nsGkAtoms.h" -#include "nsIBoxObject.h" #include "nsTreeColumns.h" #include "nsTreeUtils.h" #include "mozilla/ComputedStyle.h" diff --git a/layout/xul/tree/nsTreeSelection.cpp b/layout/xul/tree/nsTreeSelection.cpp index 25382d46db42..73e848c704c7 100644 --- a/layout/xul/tree/nsTreeSelection.cpp +++ b/layout/xul/tree/nsTreeSelection.cpp @@ -8,7 +8,6 @@ #include "mozilla/dom/Element.h" #include "nsCOMPtr.h" #include "nsTreeSelection.h" -#include "nsIBoxObject.h" #include "XULTreeElement.h" #include "nsITreeView.h" #include "nsString.h" From f5bd87247f49c44f2d54ad698b9ff40512a85c5c Mon Sep 17 00:00:00 2001 From: Gabriel Luong Date: Fri, 24 May 2019 14:02:14 -0400 Subject: [PATCH 2/8] Bug 1550519 - Show a translucent parent highlight when a subgrid is highlighted. r=pbro Differential Revision: https://phabricator.services.mozilla.com/D32507 --- .../client/inspector/grids/grid-inspector.js | 8 +- .../test/browser_grids_grid-list-subgrids.js | 50 +++++- .../inspector/shared/highlighters-overlay.js | 165 +++++++++++++++--- .../server/actors/highlighters/css-grid.js | 13 +- 4 files changed, 203 insertions(+), 33 deletions(-) diff --git a/devtools/client/inspector/grids/grid-inspector.js b/devtools/client/inspector/grids/grid-inspector.js index d0a5d480063f..60360ca97223 100644 --- a/devtools/client/inspector/grids/grid-inspector.js +++ b/devtools/client/inspector/grids/grid-inspector.js @@ -346,6 +346,10 @@ class GridInspector { return; } + if (!parentGridNodeFront) { + return; + } + const parentIndex = grids.findIndex(g => g.nodeFront.actorID === parentGridNodeFront.actorID); gridData.parentNodeActorID = parentGridNodeFront.actorID; @@ -506,8 +510,10 @@ class GridInspector { // If the grid for which the color was updated currently has a highlighter, update // the color. - if (grid.highlighted) { + if (this.highlighters.gridHighlighters.has(node)) { this.highlighters.showGridHighlighter(node); + } else if (this.highlighters.parentGridHighlighters.has(node)) { + this.highlighters.showParentGridHighlighter(node); } } } diff --git a/devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js b/devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js index e420f687d3ec..6707b100a02b 100644 --- a/devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js +++ b/devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js @@ -25,28 +25,62 @@ add_task(async function() { "Got the correct number of subgrids in div.container"); is(getGridItemElements(mainSubgridListEl).length, 2, "Got the correct number of subgrids in main.subgrid"); - ok(!highlighters.gridHighlighters.size, "No CSS grid highlighter is shown."); + ok(!highlighters.gridHighlighters.size && !highlighters.parentGridHighlighters.size, + "No CSS grid highlighter is shown."); - info("Toggling ON the CSS grid highlighter from the layout panel."); - const onHighlighterShown = highlighters.once("grid-highlighter-shown"); + info("Toggling ON the CSS grid highlighter for header."); + let onHighlighterShown = highlighters.once("grid-highlighter-shown"); let onCheckboxChange = waitUntilState(store, state => state.grids[1].highlighted); - const checkbox = containerSubgridListEl.children[0].querySelector("input"); + let checkbox = containerSubgridListEl.children[0].querySelector("input"); checkbox.click(); await onHighlighterShown; await onCheckboxChange; - info("Checking the CSS grid highlighter is created."); + info("Checking the CSS grid highlighter and parent grid highlighter are created."); is(highlighters.gridHighlighters.size, 1, "CSS grid highlighter is shown."); + is(highlighters.parentGridHighlighters.size, 1, + "CSS grid highlighter for parent grid container is shown."); - info("Toggling OFF the CSS grid highlighter from the layout panel."); - const onHighlighterHidden = highlighters.once("grid-highlighter-hidden"); + info("Toggling ON the CSS grid highlighter for main."); + onHighlighterShown = highlighters.once("grid-highlighter-shown"); + onCheckboxChange = waitUntilState(store, state => + state.grids[1].highlighted && state.grids[2].highlighted); + checkbox = containerSubgridListEl.children[1].querySelector("input"); + checkbox.click(); + await onHighlighterShown; + await onCheckboxChange; + + info("Checking the number of CSS grid highlighters present."); + is(highlighters.gridHighlighters.size, 2, + "Got the correct number of CSS grid highlighter shown."); + is(highlighters.parentGridHighlighters.size, 1, + "Only 1 parent grid highlighter should be shown for the same subgrid parent."); + + info("Toggling OFF the CSS grid highlighter for main."); + let onHighlighterHidden = highlighters.once("grid-highlighter-hidden"); + onCheckboxChange = waitUntilState(store, state => + state.grids[1].highlighted && !state.grids[2].highlighted); + checkbox.click(); + await onHighlighterHidden; + await onCheckboxChange; + + info("Checking the number of CSS grid highlighters present."); + is(highlighters.gridHighlighters.size, 1, + "Got the correct number of CSS grid highlighter shown."); + is(highlighters.parentGridHighlighters.size, 1, + "Got the correct number of CSS grid parent highlighter shown."); + + info("Toggling OFF the CSS grid highlighter for header."); + onHighlighterHidden = highlighters.once("grid-highlighter-hidden"); onCheckboxChange = waitUntilState(store, state => !state.grids[1].highlighted); + checkbox = containerSubgridListEl.children[0].querySelector("input"); checkbox.click(); await onHighlighterHidden; await onCheckboxChange; info("Checking the CSS grid highlighter is not shown."); - ok(!highlighters.gridHighlighters.size, "No CSS grid highlighter is shown."); + ok(!highlighters.gridHighlighters.size && !highlighters.parentGridHighlighters.size, + "No CSS grid highlighter is shown."); }); /** diff --git a/devtools/client/inspector/shared/highlighters-overlay.js b/devtools/client/inspector/shared/highlighters-overlay.js index 283b74a124c9..7177e1426f0d 100644 --- a/devtools/client/inspector/shared/highlighters-overlay.js +++ b/devtools/client/inspector/shared/highlighters-overlay.js @@ -17,6 +17,7 @@ loader.lazyRequireGetter(this, "parseURL", "devtools/client/shared/source-utils" loader.lazyRequireGetter(this, "asyncStorage", "devtools/shared/async-storage"); const DEFAULT_HIGHLIGHTER_COLOR = "#9400FF"; +const SUBGRID_PARENT_ALPHA = 0.5; /** * Highlighters overlay is a singleton managing all highlighters in the Inspector. @@ -30,7 +31,9 @@ class HighlightersOverlay { this.inspector = inspector; this.inspectorFront = this.inspector.inspector; this.store = this.inspector.store; - this.telemetry = inspector.telemetry; + this.target = this.inspector.target; + this.telemetry = this.inspector.telemetry; + this.walker = this.inspector.walker; this.maxGridHighlighters = Services.prefs.getIntPref("devtools.gridinspector.maxHighlighters"); @@ -39,10 +42,16 @@ class HighlightersOverlay { this.highlighters = {}; // Map of grid container NodeFront to their instantiated grid highlighter actors. this.gridHighlighters = new Map(); + // Map of parent grid container NodeFront to their instantiated grid highlighter + // actors. + this.parentGridHighlighters = new Map(); // Array of reusable grid highlighters that have been instantiated and are not // associated with any NodeFront. this.extraGridHighlighterPool = []; + // Map of grid container NodeFront to their parent grid container. + this.subgridToParentMap = new Map(); + // Boolean flag to keep track of whether or not the telemetry timer for the grid // highlighter active time is active. We keep track of this to avoid re-starting a // new timer when an additional grid highlighter is turned on. @@ -94,12 +103,21 @@ class HighlightersOverlay { // Add inspector events, not specific to a given view. this.inspector.on("markupmutation", this.onMarkupMutation); - this.inspector.walker.on("display-change", this.onDisplayChange); - this.inspector.target.on("will-navigate", this.onWillNavigate); + this.target.on("will-navigate", this.onWillNavigate); + this.walker.on("display-change", this.onDisplayChange); EventEmitter.decorate(this); } + async canGetParentGridNode() { + if (this._canGetParentGridNode === undefined) { + this._canGetParentGridNode = await this.target.actorHasMethod("domwalker", + "getParentGridNode"); + } + + return this._canGetParentGridNode; + } + /** * Returns true if the grid highlighter can be toggled on/off for the given node, and * false otherwise. A grid container can be toggled on if the max grid highlighters @@ -274,8 +292,8 @@ class HighlightersOverlay { // ex: `data:` uri, and `about:` pages let hostname; try { - hostname = parseURL(this.inspector.target.url).hostname || - parseURL(this.inspector.target.url).protocol; + hostname = parseURL(this.target.url).hostname || + parseURL(this.target.url).protocol; } catch (e) { this._handleRejection(e); } @@ -353,7 +371,7 @@ class HighlightersOverlay { try { // Save flexbox highlighter state. - const { url } = this.inspector.target; + const { url } = this.target; const selector = await node.getUniqueSelector(); this.state.flexbox = { selector, options, url }; this.flexboxHighlighterShown = node; @@ -487,7 +505,7 @@ class HighlightersOverlay { * The NodeFront of the grid container element to highlight. * @param {Object} options * Object used for passing options to the grid highlighter. - * @param. {String} trigger + * @param {String} trigger * String name matching "grid", "markup" or "rule" to indicate where the * grid highlighter was toggled on from. "grid" represents the grid view. * "markup" represents the markup view. "rule" represents the rule view. @@ -507,6 +525,18 @@ class HighlightersOverlay { // any additional grid highlighters. return; } + } else if (this.parentGridHighlighters.has(node)) { + // A translucent parent grid container is being highlighted, hide the translucent + // highlight of the parent grid container. + await this.hideGridHighlighter(node); + } + + if (node.displayType === "subgrid" && await this.canGetParentGridNode()) { + // Show a translucent highlight of the parent grid container if the given node is + // a subgrid. + const parentGridNode = await this.walker.getParentGridNode(node); + this.subgridToParentMap.set(node, parentGridNode); + await this.showParentGridHighlighter(parentGridNode); } const highlighter = await this._getGridHighlighter(node); @@ -539,7 +569,7 @@ class HighlightersOverlay { try { // Save grid highlighter state. - const { url } = this.inspector.target; + const { url } = this.target; const selector = await node.getUniqueSelector(); this.state.grids.set(node, { selector, options, url }); @@ -551,6 +581,24 @@ class HighlightersOverlay { } } + /** + * Show the grid highlighter for the given parent grid container element. + * + * @param {NodeFront} node + * The NodeFront of the parent grid container element to highlight. + */ + async showParentGridHighlighter(node) { + const highlighter = await this._getGridHighlighter(node, true); + if (!highlighter) { + return; + } + + await highlighter.show(node, { + ...this.getGridHighlighterSettings(node), + globalAlpha: SUBGRID_PARENT_ALPHA, + }); + } + /** * Hide the grid highlighter for the given grid container element. * @@ -558,18 +606,29 @@ class HighlightersOverlay { * The NodeFront of the grid container element to unhighlight. */ async hideGridHighlighter(node) { - if (!this.gridHighlighters.has(node)) { + let highlighter; + + if (this.gridHighlighters.has(node)) { + highlighter = this.gridHighlighters.get(node); + this.gridHighlighters.delete(node); + } else if (this.parentGridHighlighters.has(node)) { + highlighter = this.parentGridHighlighters.get(node); + this.parentGridHighlighters.delete(node); + } else { return; } // Hide the highlighter and put it in the pool of extra grid highlighters // so that it can be reused. - const highlighter = this.gridHighlighters.get(node); await highlighter.hide(); this.extraGridHighlighterPool.push(highlighter); - this.state.grids.delete(node); - this.gridHighlighters.delete(node); + + if (this.subgridToParentMap.has(node)) { + const parentGridNode = this.subgridToParentMap.get(node); + this.subgridToParentMap.delete(node); + await this.hideParentGridHighlighter(parentGridNode); + } this._toggleRuleViewIcon(node, false, ".ruleview-grid"); @@ -584,6 +643,30 @@ class HighlightersOverlay { this.emit("grid-highlighter-hidden", node); } + /** + * Hide the parent grid highlighter for the given parent grid container element. + * + * @param {NodeFront} node + * The NodeFront of the parent grid container element to unhiglight. + */ + async hideParentGridHighlighter(node) { + // Before hiding the parent grid highlighter, check if there are any other subgrids + // highlighted with the same parent grid container. + for (const parentGridNode of this.subgridToParentMap.values()) { + if (parentGridNode === node) { + // Don't hide the parent grid highlighter if another subgrid is highlighted + // with the given parent node. + return; + } + } + + const highlighter = this.parentGridHighlighters.get(node); + await highlighter.hide(); + this.extraGridHighlighterPool.push(highlighter); + this.state.grids.delete(node); + this.parentGridHighlighters.delete(node); + } + /** * Show the box model highlighter for the given node. * @@ -719,15 +802,14 @@ class HighlightersOverlay { async restoreState(name, state, showFunction) { const { selector, options, url } = state; - if (!selector || url !== this.inspector.target.url) { + if (!selector || url !== this.target.url) { // Bail out if no selector was saved, or if we are on a different page. this.emit(`${name}-state-restored`, { restored: false }); return; } - const walker = this.inspector.walker; - const rootNode = await walker.getRootNode(); - const nodeFront = await walker.querySelector(rootNode, selector); + const rootNode = await this.walker.getRootNode(); + const nodeFront = await this.walker.querySelector(rootNode, selector); if (nodeFront) { if (options.hoverPoint) { @@ -814,10 +896,14 @@ class HighlightersOverlay { * * @param {NodeFront} node * The NodeFront of the grid container element to highlight. + * @param {Boolean} isParent + * Whether or not the given node is a parent grid container element. * @return {Promise} that resolves to the grid highlighter front. */ - async _getGridHighlighter(node) { - if (this.gridHighlighters.has(node)) { + async _getGridHighlighter(node, isParent) { + if (isParent && this.parentGridHighlighters.has(node)) { + return this.parentGridHighlighters.get(node); + } else if (this.gridHighlighters.has(node)) { return this.gridHighlighters.get(node); } @@ -840,7 +926,12 @@ class HighlightersOverlay { return null; } - this.gridHighlighters.set(node, highlighter); + if (isParent) { + this.parentGridHighlighters.set(node, highlighter); + } else { + this.gridHighlighters.set(node, highlighter); + } + return highlighter; } @@ -938,7 +1029,7 @@ class HighlightersOverlay { } try { - const isInTree = await this.inspector.walker.isInDOMTree(node); + const isInTree = await this.walker.isInDOMTree(node); if (!isInTree) { hideHighlighter(node); } @@ -1079,7 +1170,13 @@ class HighlightersOverlay { // Hide the grid highlighter if the node is no longer a grid container. if (display !== "grid" && display !== "inline-grid" && - this.gridHighlighters.has(node)) { + (this.gridHighlighters.has(node) || this.parentGridHighlighters.has(node))) { + await this.hideGridHighlighter(node); + return; + } + + // Hide the grid highlighter if the node is no longer a subgrid. + if (display !== "subgrid" && this.gridHighlighters.has(node)) { await this.hideGridHighlighter(node); return; } @@ -1166,6 +1263,10 @@ class HighlightersOverlay { await this._hideHighlighterIfDeadNode(node, this.hideGridHighlighter); } + for (const node of this.parentGridHighlighters.keys()) { + await this._hideHighlighterIfDeadNode(node, this.hideGridHighlighter); + } + await this._hideHighlighterIfDeadNode(this.flexboxHighlighterShown, this.hideFlexboxHighlighter); await this._hideHighlighterIfDeadNode(this.flexItemHighlighterShown, @@ -1186,7 +1287,13 @@ class HighlightersOverlay { this.extraGridHighlighterPool.push(highlighter); } + for (const highlighter of this.parentGridHighlighters.values()) { + this.extraGridHighlighterPool.push(highlighter); + } + this.gridHighlighters.clear(); + this.parentGridHighlighters.clear(); + this.subgridToParentMap.clear(); this.boxModelHighlighterShown = null; this.flexboxHighlighterShown = null; @@ -1218,13 +1325,19 @@ class HighlightersOverlay { highlighter.finalize(); } + for (const highlighter of this.parentGridHighlighters.values()) { + highlighter.finalize(); + } + for (const highlighter of this.extraGridHighlighterPool) { highlighter.finalize(); } this.gridHighlighters.clear(); + this.parentGridHighlighters.clear(); this.gridHighlighters = null; + this.parentGridHighlighters = null; this.extraGridHighlighterPool = null; } @@ -1248,18 +1361,26 @@ class HighlightersOverlay { */ destroy() { this.inspector.off("markupmutation", this.onMarkupMutation); - this.inspector.target.off("will-navigate", this.onWillNavigate); + this.target.off("will-navigate", this.onWillNavigate); + this.walker.off("display-change", this.onDisplayChange); this.destroyEditors(); this.destroyGridHighlighters(); this.destroyHighlighters(); + this.subgridToParentMap.clear(); + + this._canGetParentGridNode = null; this._lastHovered = null; this.inspector = null; this.inspectorFront = null; this.state = null; this.store = null; + this.subgridToParentMap = null; + this.target = null; + this.telemetry = null; + this.walker = null; this.boxModelHighlighterShown = null; this.flexboxHighlighterShown = null; diff --git a/devtools/server/actors/highlighters/css-grid.js b/devtools/server/actors/highlighters/css-grid.js index 8411f4de8672..3be3104bc5e9 100644 --- a/devtools/server/actors/highlighters/css-grid.js +++ b/devtools/server/actors/highlighters/css-grid.js @@ -136,6 +136,9 @@ const gCachedGridPattern = new Map(); * * @param {String} options.color * The color that should be used to draw the highlighter for this grid. + * @param {Number} options.globalAlpha + * The alpha (transparency) value that should be used to draw the highlighter for + * this grid. * @param {Boolean} options.showAllGridAreas * Shows all the grid area highlights for the current grid if isShown is * true. @@ -494,6 +497,10 @@ class CssGridHighlighter extends AutoRefreshHighlighter { return this.canvas.getCanvasContext("2d"); } + get globalAlpha() { + return this.options.globalAlpha || 1; + } + getElement(id) { return this.markup.getElement(this.ID_CLASS_PREFIX + id); } @@ -550,7 +557,7 @@ class CssGridHighlighter extends AutoRefreshHighlighter { } ctx.strokeStyle = this.color; - ctx.globalAlpha = GRID_GAP_ALPHA; + ctx.globalAlpha = GRID_GAP_ALPHA * this.globalAlpha; ctx.stroke(); ctx.restore(); @@ -880,6 +887,7 @@ class CssGridHighlighter extends AutoRefreshHighlighter { this.ctx.save(); this.ctx.translate(offset - canvasX, offset - canvasY); this.ctx.font = fontSize + "px " + GRID_FONT_FAMILY; + this.ctx.globalAlpha = this.globalAlpha; this.ctx.strokeStyle = this.color; this.ctx.textAlign = "center"; this.ctx.textBaseline = "middle"; @@ -1210,6 +1218,7 @@ class CssGridHighlighter extends AutoRefreshHighlighter { this.ctx.lineWidth = 2 * displayPixelRatio; this.ctx.strokeStyle = this.color; this.ctx.fillStyle = "white"; + this.ctx.globalAlpha = this.globalAlpha; // See param definitions of drawBubbleRect. const radius = 2 * displayPixelRatio; @@ -1449,7 +1458,7 @@ class CssGridHighlighter extends AutoRefreshHighlighter { } this.ctx.strokeStyle = this.color; - this.ctx.globalAlpha = GRID_LINES_PROPERTIES[lineType].alpha; + this.ctx.globalAlpha = GRID_LINES_PROPERTIES[lineType].alpha * this.globalAlpha; if (GRID_LINES_PROPERTIES[lineType].lineWidth) { this.ctx.lineWidth = GRID_LINES_PROPERTIES[lineType].lineWidth * devicePixelRatio; From e84d1ebb62e712d7e484368352c1e3d9b4161255 Mon Sep 17 00:00:00 2001 From: Jan Varga Date: Mon, 3 Jun 2019 15:32:50 +0200 Subject: [PATCH 3/8] Bug 1534431 - Part 1: Verify LS request parameters as part of LS request state machine; r=asuth LS Request parameters are now verified as part of the state machine so an actor is always created. Before this patch, we were doing verification prior to actor creation. If the verification failed, we didn't create an actor and content was endlessly waiting for a reply. --- dom/localstorage/ActorsParent.cpp | 536 +++++++++++++++++------------- 1 file changed, 299 insertions(+), 237 deletions(-) diff --git a/dom/localstorage/ActorsParent.cpp b/dom/localstorage/ActorsParent.cpp index ef9471a50cd3..518facac5aa9 100644 --- a/dom/localstorage/ActorsParent.cpp +++ b/dom/localstorage/ActorsParent.cpp @@ -2193,13 +2193,13 @@ class LSRequestBase : public DatastoreOperationBase, public PBackgroundLSRequestParent { protected: enum class State { - // Just created on the PBackground thread. Next step is Opening. + // Just created on the PBackground thread. Next step is StartingRequest. Initial, - // Waiting to open/opening on the main thread. Next step is either - // Nesting if a subclass needs to process more nested states or + // Waiting to start/starting request on the PBackground thread. Next step is + // either Nesting if a subclass needs to process more nested states or // SendingReadyMessage if a subclass doesn't need any nested processing. - Opening, + StartingRequest, // Doing nested processing. Nesting, @@ -2221,18 +2221,22 @@ class LSRequestBase : public DatastoreOperationBase, }; nsCOMPtr mMainEventTarget; + const LSRequestParams mParams; + Maybe mContentParentId; State mState; bool mWaitingForFinish; public: - explicit LSRequestBase(nsIEventTarget* aMainEventTarget); + LSRequestBase(nsIEventTarget* aMainEventTarget, + const LSRequestParams& aParams, + const Maybe& aContentParentId); void Dispatch(); protected: ~LSRequestBase() override; - virtual nsresult Open() = 0; + virtual nsresult Start() = 0; virtual nsresult NestedRun(); @@ -2245,6 +2249,10 @@ class LSRequestBase : public DatastoreOperationBase, virtual void LogNestedState() {} private: + bool VerifyRequestParams(); + + nsresult StartRequest(); + void SendReadyMessage(); nsresult SendReadyMessageInternal(); @@ -2336,8 +2344,6 @@ class PrepareDatastoreOp LoadDataOp* mLoadDataOp; nsDataHashtable mValues; nsTArray mOrderedItems; - const LSRequestCommonParams mParams; - Maybe mContentParentId; nsCString mSuffix; nsCString mGroup; nsCString mMainThreadOrigin; @@ -2391,7 +2397,7 @@ class PrepareDatastoreOp private: ~PrepareDatastoreOp() override; - nsresult Open() override; + nsresult Start() override; nsresult CheckExistingOperations(); @@ -2489,15 +2495,15 @@ class PrepareDatastoreOp::CompressibleFunction final }; class PrepareObserverOp : public LSRequestBase { - const LSRequestPrepareObserverParams mParams; nsCString mOrigin; public: PrepareObserverOp(nsIEventTarget* aMainEventTarget, - const LSRequestParams& aParams); + const LSRequestParams& aParams, + const Maybe& aContentParentId); private: - nsresult Open() override; + nsresult Start() override; void GetResponse(LSRequestResponse& aResponse) override; }; @@ -2506,11 +2512,12 @@ class LSSimpleRequestBase : public DatastoreOperationBase, public PBackgroundLSSimpleRequestParent { protected: enum class State { - // Just created on the PBackground thread. Next step is Opening. + // Just created on the PBackground thread. Next step is StartingRequest. Initial, - // Waiting to open/opening on the main thread. Next step is SendingResults. - Opening, + // Waiting to start/starting request on the PBackground thread. Next step is + // SendingResults. + StartingRequest, // Waiting to send/sending results on the PBackground thread. Next step is // Completed. @@ -2520,21 +2527,28 @@ class LSSimpleRequestBase : public DatastoreOperationBase, Completed }; + const LSSimpleRequestParams mParams; + Maybe mContentParentId; State mState; public: - LSSimpleRequestBase(); + LSSimpleRequestBase(const LSSimpleRequestParams& aParams, + const Maybe& aContentParentId); void Dispatch(); protected: ~LSSimpleRequestBase() override; - virtual nsresult Open() = 0; + virtual nsresult Start() = 0; virtual void GetResponse(LSSimpleRequestResponse& aResponse) = 0; private: + bool VerifyRequestParams(); + + nsresult StartRequest(); + void SendResults(); // Common nsIRunnable implementation that subclasses may not override. @@ -2546,14 +2560,14 @@ class LSSimpleRequestBase : public DatastoreOperationBase, }; class PreloadedOp : public LSSimpleRequestBase { - const LSSimpleRequestPreloadedParams mParams; nsCString mOrigin; public: - explicit PreloadedOp(const LSSimpleRequestParams& aParams); + PreloadedOp(const LSSimpleRequestParams& aParams, + const Maybe& aContentParentId); private: - nsresult Open() override; + nsresult Start() override; void GetResponse(LSSimpleRequestResponse& aResponse) override; }; @@ -3134,6 +3148,53 @@ void RequestAllowToCloseIf(P aPredicate) { databases.Clear(); } +bool VerifyPrincipalInfo(const Maybe& aContentParentId, + const PrincipalInfo& aPrincipalInfo, + const PrincipalInfo& aStoragePrincipalInfo, + const Maybe& aClientId) { + AssertIsOnBackgroundThread(); + + if (NS_WARN_IF(!QuotaManager::IsPrincipalInfoValid(aPrincipalInfo))) { + return false; + } + + if (gClientValidation && aClientId.isSome()) { + RefPtr svc = ClientManagerService::GetInstance(); + if (svc && NS_WARN_IF(!svc->HasWindow(aContentParentId, aPrincipalInfo, + aClientId.ref()))) { + return false; + } + } + + if (NS_WARN_IF(!StoragePrincipalHelper:: + VerifyValidStoragePrincipalInfoForPrincipalInfo( + aStoragePrincipalInfo, aPrincipalInfo))) { + return false; + } + + return true; +} + +bool VerifyOriginKey(const nsACString& aOriginKey, + const PrincipalInfo& aPrincipalInfo) { + AssertIsOnBackgroundThread(); + + nsCString originAttrSuffix; + nsCString originKey; + nsresult rv = GenerateOriginKey2(aPrincipalInfo, originAttrSuffix, originKey); + if (NS_WARN_IF(NS_FAILED(rv))) { + return false; + } + + if (NS_WARN_IF(originKey != aOriginKey)) { + LS_WARNING("originKey (%s) doesn't match passed one (%s)!", originKey.get(), + nsCString(aOriginKey).get()); + return false; + } + + return true; +} + } // namespace /******************************************************************************* @@ -3330,119 +3391,6 @@ bool DeallocPBackgroundLSObserverParent(PBackgroundLSObserverParent* aActor) { return true; } -bool VerifyPrincipalInfo(const Maybe& aContentParentId, - const PrincipalInfo& aPrincipalInfo, - const PrincipalInfo& aStoragePrincipalInfo, - const Maybe& aClientId) { - AssertIsOnBackgroundThread(); - - if (NS_WARN_IF(!QuotaManager::IsPrincipalInfoValid(aPrincipalInfo))) { - ASSERT_UNLESS_FUZZING(); - return false; - } - - if (aClientId.isSome() && gClientValidation) { - RefPtr svc = ClientManagerService::GetInstance(); - if (svc && - !svc->HasWindow(aContentParentId, aPrincipalInfo, aClientId.ref())) { - ASSERT_UNLESS_FUZZING(); - return false; - } - } - - return StoragePrincipalHelper:: - VerifyValidStoragePrincipalInfoForPrincipalInfo(aStoragePrincipalInfo, - aPrincipalInfo); -} - -bool VerifyOriginKey(const nsACString& aOriginKey, - const PrincipalInfo& aPrincipalInfo) { - AssertIsOnBackgroundThread(); - - nsCString originAttrSuffix; - nsCString originKey; - nsresult rv = GenerateOriginKey2(aPrincipalInfo, originAttrSuffix, originKey); - if (NS_WARN_IF(NS_FAILED(rv))) { - ASSERT_UNLESS_FUZZING(); - return false; - } - - if (NS_WARN_IF(originKey != aOriginKey)) { - LS_WARNING("originKey (%s) doesn't match passed one (%s)!", originKey.get(), - nsCString(aOriginKey).get()); - ASSERT_UNLESS_FUZZING(); - return false; - } - - return true; -} - -bool VerifyRequestParams(const Maybe& aContentParentId, - const LSRequestParams& aParams) { - AssertIsOnBackgroundThread(); - MOZ_ASSERT(aParams.type() != LSRequestParams::T__None); - - switch (aParams.type()) { - case LSRequestParams::TLSRequestPreloadDatastoreParams: { - const LSRequestCommonParams& params = - aParams.get_LSRequestPreloadDatastoreParams().commonParams(); - - if (NS_WARN_IF( - !VerifyPrincipalInfo(aContentParentId, params.principalInfo(), - params.storagePrincipalInfo(), Nothing()))) { - ASSERT_UNLESS_FUZZING(); - return false; - } - - if (NS_WARN_IF( - !VerifyOriginKey(params.originKey(), params.principalInfo()))) { - ASSERT_UNLESS_FUZZING(); - return false; - } - break; - } - - case LSRequestParams::TLSRequestPrepareDatastoreParams: { - const LSRequestPrepareDatastoreParams& params = - aParams.get_LSRequestPrepareDatastoreParams(); - - const LSRequestCommonParams& commonParams = params.commonParams(); - - if (NS_WARN_IF(!VerifyPrincipalInfo( - aContentParentId, commonParams.principalInfo(), - commonParams.storagePrincipalInfo(), params.clientId()))) { - ASSERT_UNLESS_FUZZING(); - return false; - } - - if (NS_WARN_IF(!VerifyOriginKey(commonParams.originKey(), - commonParams.principalInfo()))) { - ASSERT_UNLESS_FUZZING(); - return false; - } - break; - } - - case LSRequestParams::TLSRequestPrepareObserverParams: { - const LSRequestPrepareObserverParams& params = - aParams.get_LSRequestPrepareObserverParams(); - - if (NS_WARN_IF(!VerifyPrincipalInfo( - aContentParentId, params.principalInfo(), - params.storagePrincipalInfo(), params.clientId()))) { - ASSERT_UNLESS_FUZZING(); - return false; - } - break; - } - - default: - MOZ_CRASH("Should never get here!"); - } - - return true; -} - PBackgroundLSRequestParent* AllocPBackgroundLSRequestParent( PBackgroundParent* aBackgroundActor, const LSRequestParams& aParams) { AssertIsOnBackgroundThread(); @@ -3452,13 +3400,6 @@ PBackgroundLSRequestParent* AllocPBackgroundLSRequestParent( return nullptr; } -#ifdef DEBUG - // Always verify parameters in DEBUG builds! - bool trustParams = false; -#else - bool trustParams = !BackgroundParent::IsOtherProcessActor(aBackgroundActor); -#endif - Maybe contentParentId; uint64_t childID = BackgroundParent::GetChildID(aBackgroundActor); @@ -3466,12 +3407,6 @@ PBackgroundLSRequestParent* AllocPBackgroundLSRequestParent( contentParentId = Some(ContentParentId(childID)); } - if (!trustParams && - NS_WARN_IF(!VerifyRequestParams(contentParentId, aParams))) { - ASSERT_UNLESS_FUZZING(); - return nullptr; - } - // If we're in the same process as the actor, we need to get the target event // queue from the current RequestHelper. nsCOMPtr mainEventTarget; @@ -3499,7 +3434,7 @@ PBackgroundLSRequestParent* AllocPBackgroundLSRequestParent( case LSRequestParams::TLSRequestPrepareObserverParams: { RefPtr prepareObserverOp = - new PrepareObserverOp(mainEventTarget, aParams); + new PrepareObserverOp(mainEventTarget, aParams, contentParentId); actor = std::move(prepareObserverOp); @@ -3540,32 +3475,6 @@ bool DeallocPBackgroundLSRequestParent(PBackgroundLSRequestParent* aActor) { return true; } -bool VerifyRequestParams(const Maybe& aContentParentId, - const LSSimpleRequestParams& aParams) { - AssertIsOnBackgroundThread(); - MOZ_ASSERT(aParams.type() != LSSimpleRequestParams::T__None); - - switch (aParams.type()) { - case LSSimpleRequestParams::TLSSimpleRequestPreloadedParams: { - const LSSimpleRequestPreloadedParams& params = - aParams.get_LSSimpleRequestPreloadedParams(); - - if (NS_WARN_IF( - !VerifyPrincipalInfo(aContentParentId, params.principalInfo(), - params.storagePrincipalInfo(), Nothing()))) { - ASSERT_UNLESS_FUZZING(); - return false; - } - break; - } - - default: - MOZ_CRASH("Should never get here!"); - } - - return true; -} - PBackgroundLSSimpleRequestParent* AllocPBackgroundLSSimpleRequestParent( PBackgroundParent* aBackgroundActor, const LSSimpleRequestParams& aParams) { AssertIsOnBackgroundThread(); @@ -3575,32 +3484,19 @@ PBackgroundLSSimpleRequestParent* AllocPBackgroundLSSimpleRequestParent( return nullptr; } -#ifdef DEBUG - // Always verify parameters in DEBUG builds! - bool trustParams = false; -#else - bool trustParams = !BackgroundParent::IsOtherProcessActor(aBackgroundActor); -#endif + Maybe contentParentId; - if (!trustParams) { - Maybe contentParentId; - - uint64_t childID = BackgroundParent::GetChildID(aBackgroundActor); - if (childID) { - contentParentId = Some(ContentParentId(childID)); - } - - if (NS_WARN_IF(!VerifyRequestParams(contentParentId, aParams))) { - ASSERT_UNLESS_FUZZING(); - return nullptr; - } + uint64_t childID = BackgroundParent::GetChildID(aBackgroundActor); + if (childID) { + contentParentId = Some(ContentParentId(childID)); } RefPtr actor; switch (aParams.type()) { case LSSimpleRequestParams::TLSSimpleRequestPreloadedParams: { - RefPtr preloadedOp = new PreloadedOp(aParams); + RefPtr preloadedOp = + new PreloadedOp(aParams, contentParentId); actor = std::move(preloadedOp); @@ -6346,8 +6242,12 @@ mozilla::ipc::IPCResult Observer::RecvDeleteMe() { * LSRequestBase ******************************************************************************/ -LSRequestBase::LSRequestBase(nsIEventTarget* aMainEventTarget) +LSRequestBase::LSRequestBase(nsIEventTarget* aMainEventTarget, + const LSRequestParams& aParams, + const Maybe& aContentParentId) : mMainEventTarget(aMainEventTarget), + mParams(aParams), + mContentParentId(aContentParentId), mState(State::Initial), mWaitingForFinish(false) {} @@ -6359,7 +6259,7 @@ LSRequestBase::~LSRequestBase() { void LSRequestBase::Dispatch() { AssertIsOnOwningThread(); - mState = State::Opening; + mState = State::StartingRequest; MOZ_ALWAYS_SUCCEEDS(NS_DispatchToCurrentThread(this)); } @@ -6382,8 +6282,8 @@ void LSRequestBase::LogState() { state.AssignLiteral("Initial"); break; - case State::Opening: - state.AssignLiteral("Opening"); + case State::StartingRequest: + state.AssignLiteral("StartingRequest"); break; case State::Nesting: @@ -6421,6 +6321,98 @@ void LSRequestBase::LogState() { } } +bool LSRequestBase::VerifyRequestParams() { + AssertIsOnBackgroundThread(); + + MOZ_ASSERT(mParams.type() != LSRequestParams::T__None); + + switch (mParams.type()) { + case LSRequestParams::TLSRequestPreloadDatastoreParams: { + const LSRequestCommonParams& params = + mParams.get_LSRequestPreloadDatastoreParams().commonParams(); + + if (NS_WARN_IF( + !VerifyPrincipalInfo(mContentParentId, params.principalInfo(), + params.storagePrincipalInfo(), Nothing()))) { + return false; + } + + if (NS_WARN_IF( + !VerifyOriginKey(params.originKey(), params.principalInfo()))) { + return false; + } + + break; + } + + case LSRequestParams::TLSRequestPrepareDatastoreParams: { + const LSRequestPrepareDatastoreParams& params = + mParams.get_LSRequestPrepareDatastoreParams(); + + const LSRequestCommonParams& commonParams = params.commonParams(); + + if (NS_WARN_IF(!VerifyPrincipalInfo( + mContentParentId, commonParams.principalInfo(), + commonParams.storagePrincipalInfo(), params.clientId()))) { + return false; + } + + if (NS_WARN_IF(!VerifyOriginKey(commonParams.originKey(), + commonParams.principalInfo()))) { + return false; + } + + break; + } + + case LSRequestParams::TLSRequestPrepareObserverParams: { + const LSRequestPrepareObserverParams& params = + mParams.get_LSRequestPrepareObserverParams(); + + if (NS_WARN_IF(!VerifyPrincipalInfo( + mContentParentId, params.principalInfo(), + params.storagePrincipalInfo(), params.clientId()))) { + return false; + } + + break; + } + + default: + MOZ_CRASH("Should never get here!"); + } + + return true; +} + +nsresult LSRequestBase::StartRequest() { + AssertIsOnOwningThread(); + MOZ_ASSERT(mState == State::StartingRequest); + + if (NS_WARN_IF(QuotaClient::IsShuttingDownOnBackgroundThread()) || + !MayProceed()) { + return NS_ERROR_FAILURE; + } + +#ifdef DEBUG + // Always verify parameters in DEBUG builds! + bool trustParams = false; +#else + bool trustParams = !BackgroundParent::IsOtherProcessActor(Manager()); +#endif + + if (!trustParams && NS_WARN_IF(!VerifyRequestParams())) { + return NS_ERROR_FAILURE; + } + + nsresult rv = Start(); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + + return NS_OK; +} + void LSRequestBase::SendReadyMessage() { AssertIsOnOwningThread(); MOZ_ASSERT(mState == State::SendingReadyMessage); @@ -6511,8 +6503,8 @@ LSRequestBase::Run() { nsresult rv; switch (mState) { - case State::Opening: - rv = Open(); + case State::StartingRequest: + rv = StartRequest(); break; case State::Nesting: @@ -6605,14 +6597,9 @@ mozilla::ipc::IPCResult LSRequestBase::RecvFinish() { PrepareDatastoreOp::PrepareDatastoreOp( nsIEventTarget* aMainEventTarget, const LSRequestParams& aParams, const Maybe& aContentParentId) - : LSRequestBase(aMainEventTarget), + : LSRequestBase(aMainEventTarget, aParams, aContentParentId), mMainEventTarget(aMainEventTarget), mLoadDataOp(nullptr), - mParams( - aParams.type() == LSRequestParams::TLSRequestPreloadDatastoreParams - ? aParams.get_LSRequestPreloadDatastoreParams().commonParams() - : aParams.get_LSRequestPrepareDatastoreParams().commonParams()), - mContentParentId(aContentParentId), mPrivateBrowsingId(0), mUsage(0), mSizeOfKeys(0), @@ -6641,17 +6628,20 @@ PrepareDatastoreOp::~PrepareDatastoreOp() { MOZ_ASSERT(!mLoadDataOp); } -nsresult PrepareDatastoreOp::Open() { +nsresult PrepareDatastoreOp::Start() { AssertIsOnOwningThread(); - MOZ_ASSERT(mState == State::Opening); + MOZ_ASSERT(mState == State::StartingRequest); MOZ_ASSERT(mNestedState == NestedState::BeforeNesting); + MOZ_ASSERT(!QuotaClient::IsShuttingDownOnBackgroundThread()); + MOZ_ASSERT(MayProceed()); - if (NS_WARN_IF(QuotaClient::IsShuttingDownOnBackgroundThread()) || - !MayProceedOnNonOwningThread()) { - return NS_ERROR_FAILURE; - } + const LSRequestCommonParams& commonParams = + mForPreload + ? mParams.get_LSRequestPreloadDatastoreParams().commonParams() + : mParams.get_LSRequestPrepareDatastoreParams().commonParams(); - const PrincipalInfo& storagePrincipalInfo = mParams.storagePrincipalInfo(); + const PrincipalInfo& storagePrincipalInfo = + commonParams.storagePrincipalInfo(); if (storagePrincipalInfo.type() == PrincipalInfo::TSystemPrincipalInfo) { QuotaManager::GetInfoForChrome(&mSuffix, &mGroup, &mOrigin); @@ -6682,7 +6672,13 @@ nsresult PrepareDatastoreOp::CheckExistingOperations() { return NS_ERROR_FAILURE; } - const PrincipalInfo& storagePrincipalInfo = mParams.storagePrincipalInfo(); + const LSRequestCommonParams& commonParams = + mForPreload + ? mParams.get_LSRequestPreloadDatastoreParams().commonParams() + : mParams.get_LSRequestPrepareDatastoreParams().commonParams(); + + const PrincipalInfo& storagePrincipalInfo = + commonParams.storagePrincipalInfo(); nsCString originAttrSuffix; uint32_t privateBrowsingId; @@ -6702,7 +6698,7 @@ nsresult PrepareDatastoreOp::CheckExistingOperations() { } mArchivedOriginScope = ArchivedOriginScope::CreateFromOrigin( - originAttrSuffix, mParams.originKey()); + originAttrSuffix, commonParams.originKey()); MOZ_ASSERT(mArchivedOriginScope); // Normally it's safe to access member variables without a mutex because even @@ -7467,6 +7463,8 @@ void PrepareDatastoreOp::GetResponse(LSRequestResponse& aResponse) { AssertIsOnOwningThread(); MOZ_ASSERT(mState == State::SendingResults); MOZ_ASSERT(NS_SUCCEEDED(ResultCode())); + MOZ_ASSERT(!QuotaClient::IsShuttingDownOnBackgroundThread()); + MOZ_ASSERT(MayProceed()); // A datastore is not created when we are just trying to preload data and // there's no database file. @@ -7942,24 +7940,24 @@ PrepareDatastoreOp::CompressibleFunction::OnFunctionCall( * PrepareObserverOp ******************************************************************************/ -PrepareObserverOp::PrepareObserverOp(nsIEventTarget* aMainEventTarget, - const LSRequestParams& aParams) - : LSRequestBase(aMainEventTarget), - mParams(aParams.get_LSRequestPrepareObserverParams()) { +PrepareObserverOp::PrepareObserverOp( + nsIEventTarget* aMainEventTarget, const LSRequestParams& aParams, + const Maybe& aContentParentId) + : LSRequestBase(aMainEventTarget, aParams, aContentParentId) { MOZ_ASSERT(aParams.type() == LSRequestParams::TLSRequestPrepareObserverParams); } -nsresult PrepareObserverOp::Open() { +nsresult PrepareObserverOp::Start() { AssertIsOnOwningThread(); - MOZ_ASSERT(mState == State::Opening); + MOZ_ASSERT(mState == State::StartingRequest); + MOZ_ASSERT(!QuotaClient::IsShuttingDownOnBackgroundThread()); + MOZ_ASSERT(MayProceed()); - if (NS_WARN_IF(QuotaClient::IsShuttingDownOnBackgroundThread()) || - !MayProceedOnNonOwningThread()) { - return NS_ERROR_FAILURE; - } + const LSRequestPrepareObserverParams params = + mParams.get_LSRequestPrepareObserverParams(); - const PrincipalInfo& storagePrincipalInfo = mParams.storagePrincipalInfo(); + const PrincipalInfo& storagePrincipalInfo = params.storagePrincipalInfo(); if (storagePrincipalInfo.type() == PrincipalInfo::TSystemPrincipalInfo) { QuotaManager::GetInfoForChrome(nullptr, nullptr, &mOrigin); @@ -7981,6 +7979,8 @@ void PrepareObserverOp::GetResponse(LSRequestResponse& aResponse) { AssertIsOnOwningThread(); MOZ_ASSERT(mState == State::SendingResults); MOZ_ASSERT(NS_SUCCEEDED(ResultCode())); + MOZ_ASSERT(!QuotaClient::IsShuttingDownOnBackgroundThread()); + MOZ_ASSERT(MayProceed()); uint64_t observerId = ++gLastObserverId; @@ -8002,7 +8002,12 @@ void PrepareObserverOp::GetResponse(LSRequestResponse& aResponse) { + ******************************************************************************/ -LSSimpleRequestBase::LSSimpleRequestBase() : mState(State::Initial) {} +LSSimpleRequestBase::LSSimpleRequestBase( + const LSSimpleRequestParams& aParams, + const Maybe& aContentParentId) + : mParams(aParams), + mContentParentId(aContentParentId), + mState(State::Initial) {} LSSimpleRequestBase::~LSSimpleRequestBase() { MOZ_ASSERT_IF(MayProceedOnNonOwningThread(), @@ -8012,11 +8017,65 @@ LSSimpleRequestBase::~LSSimpleRequestBase() { void LSSimpleRequestBase::Dispatch() { AssertIsOnOwningThread(); - mState = State::Opening; + mState = State::StartingRequest; MOZ_ALWAYS_SUCCEEDS(NS_DispatchToCurrentThread(this)); } +bool LSSimpleRequestBase::VerifyRequestParams() { + AssertIsOnBackgroundThread(); + + MOZ_ASSERT(mParams.type() != LSSimpleRequestParams::T__None); + + switch (mParams.type()) { + case LSSimpleRequestParams::TLSSimpleRequestPreloadedParams: { + const LSSimpleRequestPreloadedParams& params = + mParams.get_LSSimpleRequestPreloadedParams(); + + if (NS_WARN_IF( + !VerifyPrincipalInfo(mContentParentId, params.principalInfo(), + params.storagePrincipalInfo(), Nothing()))) { + return false; + } + + break; + } + + default: + MOZ_CRASH("Should never get here!"); + } + + return true; +} + +nsresult LSSimpleRequestBase::StartRequest() { + AssertIsOnOwningThread(); + MOZ_ASSERT(mState == State::StartingRequest); + + if (NS_WARN_IF(QuotaClient::IsShuttingDownOnBackgroundThread()) || + !MayProceed()) { + return NS_ERROR_FAILURE; + } + +#ifdef DEBUG + // Always verify parameters in DEBUG builds! + bool trustParams = false; +#else + bool trustParams = !BackgroundParent::IsOtherProcessActor(Manager()); +#endif + + if (!trustParams && NS_WARN_IF(!VerifyRequestParams())) { + return NS_ERROR_FAILURE; + } + + nsresult rv = Start(); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + + return NS_OK; +} + void LSSimpleRequestBase::SendResults() { AssertIsOnOwningThread(); MOZ_ASSERT(mState == State::SendingResults); @@ -8046,8 +8105,8 @@ LSSimpleRequestBase::Run() { nsresult rv; switch (mState) { - case State::Opening: - rv = Open(); + case State::StartingRequest: + rv = StartRequest(); break; case State::SendingResults: @@ -8086,22 +8145,23 @@ void LSSimpleRequestBase::ActorDestroy(ActorDestroyReason aWhy) { * PreloadedOp ******************************************************************************/ -PreloadedOp::PreloadedOp(const LSSimpleRequestParams& aParams) - : mParams(aParams.get_LSSimpleRequestPreloadedParams()) { +PreloadedOp::PreloadedOp(const LSSimpleRequestParams& aParams, + const Maybe& aContentParentId) + : LSSimpleRequestBase(aParams, aContentParentId) { MOZ_ASSERT(aParams.type() == LSSimpleRequestParams::TLSSimpleRequestPreloadedParams); } -nsresult PreloadedOp::Open() { +nsresult PreloadedOp::Start() { AssertIsOnOwningThread(); - MOZ_ASSERT(mState == State::Opening); + MOZ_ASSERT(mState == State::StartingRequest); + MOZ_ASSERT(!QuotaClient::IsShuttingDownOnBackgroundThread()); + MOZ_ASSERT(MayProceed()); - if (NS_WARN_IF(QuotaClient::IsShuttingDownOnBackgroundThread()) || - !MayProceedOnNonOwningThread()) { - return NS_ERROR_FAILURE; - } + const LSSimpleRequestPreloadedParams& params = + mParams.get_LSSimpleRequestPreloadedParams(); - const PrincipalInfo& storagePrincipalInfo = mParams.storagePrincipalInfo(); + const PrincipalInfo& storagePrincipalInfo = params.storagePrincipalInfo(); if (storagePrincipalInfo.type() == PrincipalInfo::TSystemPrincipalInfo) { QuotaManager::GetInfoForChrome(nullptr, nullptr, &mOrigin); @@ -8123,6 +8183,8 @@ void PreloadedOp::GetResponse(LSSimpleRequestResponse& aResponse) { AssertIsOnOwningThread(); MOZ_ASSERT(mState == State::SendingResults); MOZ_ASSERT(NS_SUCCEEDED(ResultCode())); + MOZ_ASSERT(!QuotaClient::IsShuttingDownOnBackgroundThread()); + MOZ_ASSERT(MayProceed()); bool preloaded; RefPtr datastore; From fe813f031dcafd156579e2e66de02f0d7662df65 Mon Sep 17 00:00:00 2001 From: Jan Varga Date: Mon, 3 Jun 2019 15:32:59 +0200 Subject: [PATCH 4/8] Bug 1534431 - Part 2: Disallow LS requests with undefined client id; r=asuth Differential Revision: https://phabricator.services.mozilla.com/D33595 --- dom/localstorage/ActorsParent.cpp | 64 ++++++++++++------- dom/localstorage/LSObject.cpp | 2 + dom/localstorage/test/unit/head.js | 7 ++ .../test/unit/test_clientValidation.js | 29 +++++++++ dom/localstorage/test/unit/xpcshell.ini | 1 + .../unit/test_localStorageArchive4upgrade.js | 7 ++ 6 files changed, 87 insertions(+), 23 deletions(-) create mode 100644 dom/localstorage/test/unit/test_clientValidation.js diff --git a/dom/localstorage/ActorsParent.cpp b/dom/localstorage/ActorsParent.cpp index 518facac5aa9..84aaea2ecdda 100644 --- a/dom/localstorage/ActorsParent.cpp +++ b/dom/localstorage/ActorsParent.cpp @@ -3148,17 +3148,33 @@ void RequestAllowToCloseIf(P aPredicate) { databases.Clear(); } -bool VerifyPrincipalInfo(const Maybe& aContentParentId, - const PrincipalInfo& aPrincipalInfo, - const PrincipalInfo& aStoragePrincipalInfo, - const Maybe& aClientId) { +bool VerifyPrincipalInfo(const PrincipalInfo& aPrincipalInfo, + const PrincipalInfo& aStoragePrincipalInfo) { AssertIsOnBackgroundThread(); if (NS_WARN_IF(!QuotaManager::IsPrincipalInfoValid(aPrincipalInfo))) { return false; } - if (gClientValidation && aClientId.isSome()) { + if (NS_WARN_IF(!StoragePrincipalHelper:: + VerifyValidStoragePrincipalInfoForPrincipalInfo( + aStoragePrincipalInfo, aPrincipalInfo))) { + return false; + } + + return true; +} + +bool VerifyClientId(const Maybe& aContentParentId, + const PrincipalInfo& aPrincipalInfo, + const Maybe& aClientId) { + AssertIsOnBackgroundThread(); + + if (gClientValidation) { + if (NS_WARN_IF(aClientId.isNothing())) { + return false; + } + RefPtr svc = ClientManagerService::GetInstance(); if (svc && NS_WARN_IF(!svc->HasWindow(aContentParentId, aPrincipalInfo, aClientId.ref()))) { @@ -3166,12 +3182,6 @@ bool VerifyPrincipalInfo(const Maybe& aContentParentId, } } - if (NS_WARN_IF(!StoragePrincipalHelper:: - VerifyValidStoragePrincipalInfoForPrincipalInfo( - aStoragePrincipalInfo, aPrincipalInfo))) { - return false; - } - return true; } @@ -6331,9 +6341,8 @@ bool LSRequestBase::VerifyRequestParams() { const LSRequestCommonParams& params = mParams.get_LSRequestPreloadDatastoreParams().commonParams(); - if (NS_WARN_IF( - !VerifyPrincipalInfo(mContentParentId, params.principalInfo(), - params.storagePrincipalInfo(), Nothing()))) { + if (NS_WARN_IF(!VerifyPrincipalInfo(params.principalInfo(), + params.storagePrincipalInfo()))) { return false; } @@ -6351,9 +6360,15 @@ bool LSRequestBase::VerifyRequestParams() { const LSRequestCommonParams& commonParams = params.commonParams(); - if (NS_WARN_IF(!VerifyPrincipalInfo( - mContentParentId, commonParams.principalInfo(), - commonParams.storagePrincipalInfo(), params.clientId()))) { + if (NS_WARN_IF( + !VerifyPrincipalInfo(commonParams.principalInfo(), + commonParams.storagePrincipalInfo()))) { + return false; + } + + if (NS_WARN_IF(!VerifyClientId(mContentParentId, + commonParams.principalInfo(), + params.clientId()))) { return false; } @@ -6369,9 +6384,13 @@ bool LSRequestBase::VerifyRequestParams() { const LSRequestPrepareObserverParams& params = mParams.get_LSRequestPrepareObserverParams(); - if (NS_WARN_IF(!VerifyPrincipalInfo( - mContentParentId, params.principalInfo(), - params.storagePrincipalInfo(), params.clientId()))) { + if (NS_WARN_IF(!VerifyPrincipalInfo(params.principalInfo(), + params.storagePrincipalInfo()))) { + return false; + } + + if (NS_WARN_IF(!VerifyClientId(mContentParentId, params.principalInfo(), + params.clientId()))) { return false; } @@ -8032,9 +8051,8 @@ bool LSSimpleRequestBase::VerifyRequestParams() { const LSSimpleRequestPreloadedParams& params = mParams.get_LSSimpleRequestPreloadedParams(); - if (NS_WARN_IF( - !VerifyPrincipalInfo(mContentParentId, params.principalInfo(), - params.storagePrincipalInfo(), Nothing()))) { + if (NS_WARN_IF(!VerifyPrincipalInfo(params.principalInfo(), + params.storagePrincipalInfo()))) { return false; } diff --git a/dom/localstorage/LSObject.cpp b/dom/localstorage/LSObject.cpp index 351b0084b9eb..f9ddf59a4d97 100644 --- a/dom/localstorage/LSObject.cpp +++ b/dom/localstorage/LSObject.cpp @@ -405,6 +405,8 @@ nsresult LSObject::CreateForPrincipal(nsPIDOMWindowInner* aWindow, } clientId = Some(clientInfo.ref().Id()); + } else if (Preferences::GetBool("dom.storage.client_validation")) { + return NS_ERROR_FAILURE; } RefPtr object = diff --git a/dom/localstorage/test/unit/head.js b/dom/localstorage/test/unit/head.js index f94475d26fab..a1dcfc65d7cb 100644 --- a/dom/localstorage/test/unit/head.js +++ b/dom/localstorage/test/unit/head.js @@ -53,11 +53,18 @@ function returnToEventLoop() { function enableTesting() { Services.prefs.setBoolPref("dom.storage.testing", true); + + // xpcshell globals don't have associated clients in the Clients API sense, so + // we need to disable client validation so that the unit tests are allowed to + // use LocalStorage. + Services.prefs.setBoolPref("dom.storage.client_validation", false); + Services.prefs.setBoolPref("dom.quotaManager.testing", true); } function resetTesting() { Services.prefs.clearUserPref("dom.quotaManager.testing"); + Services.prefs.clearUserPref("dom.storage.client_validation"); Services.prefs.clearUserPref("dom.storage.testing"); } diff --git a/dom/localstorage/test/unit/test_clientValidation.js b/dom/localstorage/test/unit/test_clientValidation.js new file mode 100644 index 000000000000..1725315f05fd --- /dev/null +++ b/dom/localstorage/test/unit/test_clientValidation.js @@ -0,0 +1,29 @@ +/** + * Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ + */ + +/** + * Because this is an xpcshell global, it does not have an associated client id. + * We turn on client validation for LocalStorage and ensure that we don't have + * access to LocalStorage. + */ +async function testSteps() { + const principal = getPrincipal("http://example.com"); + + info("Setting prefs"); + + Services.prefs.setBoolPref("dom.storage.next_gen", true); + Services.prefs.setBoolPref("dom.storage.client_validation", true); + + info("Getting storage"); + + try { + getLocalStorage(principal); + ok(false, "Should have thrown"); + } catch (ex) { + ok(true, "Did throw"); + is(ex.name, "NS_ERROR_FAILURE", "Threw right Exception"); + is(ex.result, Cr.NS_ERROR_FAILURE, "Threw with right result"); + } +} diff --git a/dom/localstorage/test/unit/xpcshell.ini b/dom/localstorage/test/unit/xpcshell.ini index 82811ae80fd6..5c0dd0b68394 100644 --- a/dom/localstorage/test/unit/xpcshell.ini +++ b/dom/localstorage/test/unit/xpcshell.ini @@ -14,6 +14,7 @@ support-files = stringLength_profile.zip [test_archive.js] +[test_clientValidation.js] [test_corruptedDatabase.js] [test_databaseShadowing1.js] run-sequentially = test_databaseShadowing2.js depends on a file produced by this test diff --git a/dom/quota/test/unit/test_localStorageArchive4upgrade.js b/dom/quota/test/unit/test_localStorageArchive4upgrade.js index f7547e91088c..16581d30573d 100644 --- a/dom/quota/test/unit/test_localStorageArchive4upgrade.js +++ b/dom/quota/test/unit/test_localStorageArchive4upgrade.js @@ -32,6 +32,13 @@ async function testSteps() { return Services.domStorageManager.createStorage(null, principal, principal, ""); } + info("Setting pref"); + + // xpcshell globals don't have associated clients in the Clients API sense, so + // we need to disable client validation so that this unit test is allowed to + // use LocalStorage. + Services.prefs.setBoolPref("dom.storage.client_validation", false); + info("Clearing"); let request = clear(); From 49c534e5b272bc12dbb94abab1b089e8b5a6842b Mon Sep 17 00:00:00 2001 From: Narcis Beleuzu Date: Tue, 4 Jun 2019 22:43:03 +0300 Subject: [PATCH 5/8] Backed out changeset 49fbf60f938d (bug 1550519) for dt failures on grid-inspector.js . CLOSED TREE --HG-- extra : rebase_source : 3da4c4f1341edc7cc5137e7a63f4c869862d894f --- .../client/inspector/grids/grid-inspector.js | 8 +- .../test/browser_grids_grid-list-subgrids.js | 50 +----- .../inspector/shared/highlighters-overlay.js | 165 +++--------------- .../server/actors/highlighters/css-grid.js | 13 +- 4 files changed, 33 insertions(+), 203 deletions(-) diff --git a/devtools/client/inspector/grids/grid-inspector.js b/devtools/client/inspector/grids/grid-inspector.js index 60360ca97223..d0a5d480063f 100644 --- a/devtools/client/inspector/grids/grid-inspector.js +++ b/devtools/client/inspector/grids/grid-inspector.js @@ -346,10 +346,6 @@ class GridInspector { return; } - if (!parentGridNodeFront) { - return; - } - const parentIndex = grids.findIndex(g => g.nodeFront.actorID === parentGridNodeFront.actorID); gridData.parentNodeActorID = parentGridNodeFront.actorID; @@ -510,10 +506,8 @@ class GridInspector { // If the grid for which the color was updated currently has a highlighter, update // the color. - if (this.highlighters.gridHighlighters.has(node)) { + if (grid.highlighted) { this.highlighters.showGridHighlighter(node); - } else if (this.highlighters.parentGridHighlighters.has(node)) { - this.highlighters.showParentGridHighlighter(node); } } } diff --git a/devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js b/devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js index 6707b100a02b..e420f687d3ec 100644 --- a/devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js +++ b/devtools/client/inspector/grids/test/browser_grids_grid-list-subgrids.js @@ -25,62 +25,28 @@ add_task(async function() { "Got the correct number of subgrids in div.container"); is(getGridItemElements(mainSubgridListEl).length, 2, "Got the correct number of subgrids in main.subgrid"); - ok(!highlighters.gridHighlighters.size && !highlighters.parentGridHighlighters.size, - "No CSS grid highlighter is shown."); + ok(!highlighters.gridHighlighters.size, "No CSS grid highlighter is shown."); - info("Toggling ON the CSS grid highlighter for header."); - let onHighlighterShown = highlighters.once("grid-highlighter-shown"); + info("Toggling ON the CSS grid highlighter from the layout panel."); + const onHighlighterShown = highlighters.once("grid-highlighter-shown"); let onCheckboxChange = waitUntilState(store, state => state.grids[1].highlighted); - let checkbox = containerSubgridListEl.children[0].querySelector("input"); + const checkbox = containerSubgridListEl.children[0].querySelector("input"); checkbox.click(); await onHighlighterShown; await onCheckboxChange; - info("Checking the CSS grid highlighter and parent grid highlighter are created."); + info("Checking the CSS grid highlighter is created."); is(highlighters.gridHighlighters.size, 1, "CSS grid highlighter is shown."); - is(highlighters.parentGridHighlighters.size, 1, - "CSS grid highlighter for parent grid container is shown."); - info("Toggling ON the CSS grid highlighter for main."); - onHighlighterShown = highlighters.once("grid-highlighter-shown"); - onCheckboxChange = waitUntilState(store, state => - state.grids[1].highlighted && state.grids[2].highlighted); - checkbox = containerSubgridListEl.children[1].querySelector("input"); - checkbox.click(); - await onHighlighterShown; - await onCheckboxChange; - - info("Checking the number of CSS grid highlighters present."); - is(highlighters.gridHighlighters.size, 2, - "Got the correct number of CSS grid highlighter shown."); - is(highlighters.parentGridHighlighters.size, 1, - "Only 1 parent grid highlighter should be shown for the same subgrid parent."); - - info("Toggling OFF the CSS grid highlighter for main."); - let onHighlighterHidden = highlighters.once("grid-highlighter-hidden"); - onCheckboxChange = waitUntilState(store, state => - state.grids[1].highlighted && !state.grids[2].highlighted); - checkbox.click(); - await onHighlighterHidden; - await onCheckboxChange; - - info("Checking the number of CSS grid highlighters present."); - is(highlighters.gridHighlighters.size, 1, - "Got the correct number of CSS grid highlighter shown."); - is(highlighters.parentGridHighlighters.size, 1, - "Got the correct number of CSS grid parent highlighter shown."); - - info("Toggling OFF the CSS grid highlighter for header."); - onHighlighterHidden = highlighters.once("grid-highlighter-hidden"); + info("Toggling OFF the CSS grid highlighter from the layout panel."); + const onHighlighterHidden = highlighters.once("grid-highlighter-hidden"); onCheckboxChange = waitUntilState(store, state => !state.grids[1].highlighted); - checkbox = containerSubgridListEl.children[0].querySelector("input"); checkbox.click(); await onHighlighterHidden; await onCheckboxChange; info("Checking the CSS grid highlighter is not shown."); - ok(!highlighters.gridHighlighters.size && !highlighters.parentGridHighlighters.size, - "No CSS grid highlighter is shown."); + ok(!highlighters.gridHighlighters.size, "No CSS grid highlighter is shown."); }); /** diff --git a/devtools/client/inspector/shared/highlighters-overlay.js b/devtools/client/inspector/shared/highlighters-overlay.js index 7177e1426f0d..283b74a124c9 100644 --- a/devtools/client/inspector/shared/highlighters-overlay.js +++ b/devtools/client/inspector/shared/highlighters-overlay.js @@ -17,7 +17,6 @@ loader.lazyRequireGetter(this, "parseURL", "devtools/client/shared/source-utils" loader.lazyRequireGetter(this, "asyncStorage", "devtools/shared/async-storage"); const DEFAULT_HIGHLIGHTER_COLOR = "#9400FF"; -const SUBGRID_PARENT_ALPHA = 0.5; /** * Highlighters overlay is a singleton managing all highlighters in the Inspector. @@ -31,9 +30,7 @@ class HighlightersOverlay { this.inspector = inspector; this.inspectorFront = this.inspector.inspector; this.store = this.inspector.store; - this.target = this.inspector.target; - this.telemetry = this.inspector.telemetry; - this.walker = this.inspector.walker; + this.telemetry = inspector.telemetry; this.maxGridHighlighters = Services.prefs.getIntPref("devtools.gridinspector.maxHighlighters"); @@ -42,16 +39,10 @@ class HighlightersOverlay { this.highlighters = {}; // Map of grid container NodeFront to their instantiated grid highlighter actors. this.gridHighlighters = new Map(); - // Map of parent grid container NodeFront to their instantiated grid highlighter - // actors. - this.parentGridHighlighters = new Map(); // Array of reusable grid highlighters that have been instantiated and are not // associated with any NodeFront. this.extraGridHighlighterPool = []; - // Map of grid container NodeFront to their parent grid container. - this.subgridToParentMap = new Map(); - // Boolean flag to keep track of whether or not the telemetry timer for the grid // highlighter active time is active. We keep track of this to avoid re-starting a // new timer when an additional grid highlighter is turned on. @@ -103,21 +94,12 @@ class HighlightersOverlay { // Add inspector events, not specific to a given view. this.inspector.on("markupmutation", this.onMarkupMutation); - this.target.on("will-navigate", this.onWillNavigate); - this.walker.on("display-change", this.onDisplayChange); + this.inspector.walker.on("display-change", this.onDisplayChange); + this.inspector.target.on("will-navigate", this.onWillNavigate); EventEmitter.decorate(this); } - async canGetParentGridNode() { - if (this._canGetParentGridNode === undefined) { - this._canGetParentGridNode = await this.target.actorHasMethod("domwalker", - "getParentGridNode"); - } - - return this._canGetParentGridNode; - } - /** * Returns true if the grid highlighter can be toggled on/off for the given node, and * false otherwise. A grid container can be toggled on if the max grid highlighters @@ -292,8 +274,8 @@ class HighlightersOverlay { // ex: `data:` uri, and `about:` pages let hostname; try { - hostname = parseURL(this.target.url).hostname || - parseURL(this.target.url).protocol; + hostname = parseURL(this.inspector.target.url).hostname || + parseURL(this.inspector.target.url).protocol; } catch (e) { this._handleRejection(e); } @@ -371,7 +353,7 @@ class HighlightersOverlay { try { // Save flexbox highlighter state. - const { url } = this.target; + const { url } = this.inspector.target; const selector = await node.getUniqueSelector(); this.state.flexbox = { selector, options, url }; this.flexboxHighlighterShown = node; @@ -505,7 +487,7 @@ class HighlightersOverlay { * The NodeFront of the grid container element to highlight. * @param {Object} options * Object used for passing options to the grid highlighter. - * @param {String} trigger + * @param. {String} trigger * String name matching "grid", "markup" or "rule" to indicate where the * grid highlighter was toggled on from. "grid" represents the grid view. * "markup" represents the markup view. "rule" represents the rule view. @@ -525,18 +507,6 @@ class HighlightersOverlay { // any additional grid highlighters. return; } - } else if (this.parentGridHighlighters.has(node)) { - // A translucent parent grid container is being highlighted, hide the translucent - // highlight of the parent grid container. - await this.hideGridHighlighter(node); - } - - if (node.displayType === "subgrid" && await this.canGetParentGridNode()) { - // Show a translucent highlight of the parent grid container if the given node is - // a subgrid. - const parentGridNode = await this.walker.getParentGridNode(node); - this.subgridToParentMap.set(node, parentGridNode); - await this.showParentGridHighlighter(parentGridNode); } const highlighter = await this._getGridHighlighter(node); @@ -569,7 +539,7 @@ class HighlightersOverlay { try { // Save grid highlighter state. - const { url } = this.target; + const { url } = this.inspector.target; const selector = await node.getUniqueSelector(); this.state.grids.set(node, { selector, options, url }); @@ -581,24 +551,6 @@ class HighlightersOverlay { } } - /** - * Show the grid highlighter for the given parent grid container element. - * - * @param {NodeFront} node - * The NodeFront of the parent grid container element to highlight. - */ - async showParentGridHighlighter(node) { - const highlighter = await this._getGridHighlighter(node, true); - if (!highlighter) { - return; - } - - await highlighter.show(node, { - ...this.getGridHighlighterSettings(node), - globalAlpha: SUBGRID_PARENT_ALPHA, - }); - } - /** * Hide the grid highlighter for the given grid container element. * @@ -606,29 +558,18 @@ class HighlightersOverlay { * The NodeFront of the grid container element to unhighlight. */ async hideGridHighlighter(node) { - let highlighter; - - if (this.gridHighlighters.has(node)) { - highlighter = this.gridHighlighters.get(node); - this.gridHighlighters.delete(node); - } else if (this.parentGridHighlighters.has(node)) { - highlighter = this.parentGridHighlighters.get(node); - this.parentGridHighlighters.delete(node); - } else { + if (!this.gridHighlighters.has(node)) { return; } // Hide the highlighter and put it in the pool of extra grid highlighters // so that it can be reused. + const highlighter = this.gridHighlighters.get(node); await highlighter.hide(); this.extraGridHighlighterPool.push(highlighter); - this.state.grids.delete(node); - if (this.subgridToParentMap.has(node)) { - const parentGridNode = this.subgridToParentMap.get(node); - this.subgridToParentMap.delete(node); - await this.hideParentGridHighlighter(parentGridNode); - } + this.state.grids.delete(node); + this.gridHighlighters.delete(node); this._toggleRuleViewIcon(node, false, ".ruleview-grid"); @@ -643,30 +584,6 @@ class HighlightersOverlay { this.emit("grid-highlighter-hidden", node); } - /** - * Hide the parent grid highlighter for the given parent grid container element. - * - * @param {NodeFront} node - * The NodeFront of the parent grid container element to unhiglight. - */ - async hideParentGridHighlighter(node) { - // Before hiding the parent grid highlighter, check if there are any other subgrids - // highlighted with the same parent grid container. - for (const parentGridNode of this.subgridToParentMap.values()) { - if (parentGridNode === node) { - // Don't hide the parent grid highlighter if another subgrid is highlighted - // with the given parent node. - return; - } - } - - const highlighter = this.parentGridHighlighters.get(node); - await highlighter.hide(); - this.extraGridHighlighterPool.push(highlighter); - this.state.grids.delete(node); - this.parentGridHighlighters.delete(node); - } - /** * Show the box model highlighter for the given node. * @@ -802,14 +719,15 @@ class HighlightersOverlay { async restoreState(name, state, showFunction) { const { selector, options, url } = state; - if (!selector || url !== this.target.url) { + if (!selector || url !== this.inspector.target.url) { // Bail out if no selector was saved, or if we are on a different page. this.emit(`${name}-state-restored`, { restored: false }); return; } - const rootNode = await this.walker.getRootNode(); - const nodeFront = await this.walker.querySelector(rootNode, selector); + const walker = this.inspector.walker; + const rootNode = await walker.getRootNode(); + const nodeFront = await walker.querySelector(rootNode, selector); if (nodeFront) { if (options.hoverPoint) { @@ -896,14 +814,10 @@ class HighlightersOverlay { * * @param {NodeFront} node * The NodeFront of the grid container element to highlight. - * @param {Boolean} isParent - * Whether or not the given node is a parent grid container element. * @return {Promise} that resolves to the grid highlighter front. */ - async _getGridHighlighter(node, isParent) { - if (isParent && this.parentGridHighlighters.has(node)) { - return this.parentGridHighlighters.get(node); - } else if (this.gridHighlighters.has(node)) { + async _getGridHighlighter(node) { + if (this.gridHighlighters.has(node)) { return this.gridHighlighters.get(node); } @@ -926,12 +840,7 @@ class HighlightersOverlay { return null; } - if (isParent) { - this.parentGridHighlighters.set(node, highlighter); - } else { - this.gridHighlighters.set(node, highlighter); - } - + this.gridHighlighters.set(node, highlighter); return highlighter; } @@ -1029,7 +938,7 @@ class HighlightersOverlay { } try { - const isInTree = await this.walker.isInDOMTree(node); + const isInTree = await this.inspector.walker.isInDOMTree(node); if (!isInTree) { hideHighlighter(node); } @@ -1170,13 +1079,7 @@ class HighlightersOverlay { // Hide the grid highlighter if the node is no longer a grid container. if (display !== "grid" && display !== "inline-grid" && - (this.gridHighlighters.has(node) || this.parentGridHighlighters.has(node))) { - await this.hideGridHighlighter(node); - return; - } - - // Hide the grid highlighter if the node is no longer a subgrid. - if (display !== "subgrid" && this.gridHighlighters.has(node)) { + this.gridHighlighters.has(node)) { await this.hideGridHighlighter(node); return; } @@ -1263,10 +1166,6 @@ class HighlightersOverlay { await this._hideHighlighterIfDeadNode(node, this.hideGridHighlighter); } - for (const node of this.parentGridHighlighters.keys()) { - await this._hideHighlighterIfDeadNode(node, this.hideGridHighlighter); - } - await this._hideHighlighterIfDeadNode(this.flexboxHighlighterShown, this.hideFlexboxHighlighter); await this._hideHighlighterIfDeadNode(this.flexItemHighlighterShown, @@ -1287,13 +1186,7 @@ class HighlightersOverlay { this.extraGridHighlighterPool.push(highlighter); } - for (const highlighter of this.parentGridHighlighters.values()) { - this.extraGridHighlighterPool.push(highlighter); - } - this.gridHighlighters.clear(); - this.parentGridHighlighters.clear(); - this.subgridToParentMap.clear(); this.boxModelHighlighterShown = null; this.flexboxHighlighterShown = null; @@ -1325,19 +1218,13 @@ class HighlightersOverlay { highlighter.finalize(); } - for (const highlighter of this.parentGridHighlighters.values()) { - highlighter.finalize(); - } - for (const highlighter of this.extraGridHighlighterPool) { highlighter.finalize(); } this.gridHighlighters.clear(); - this.parentGridHighlighters.clear(); this.gridHighlighters = null; - this.parentGridHighlighters = null; this.extraGridHighlighterPool = null; } @@ -1361,26 +1248,18 @@ class HighlightersOverlay { */ destroy() { this.inspector.off("markupmutation", this.onMarkupMutation); - this.target.off("will-navigate", this.onWillNavigate); - this.walker.off("display-change", this.onDisplayChange); + this.inspector.target.off("will-navigate", this.onWillNavigate); this.destroyEditors(); this.destroyGridHighlighters(); this.destroyHighlighters(); - this.subgridToParentMap.clear(); - - this._canGetParentGridNode = null; this._lastHovered = null; this.inspector = null; this.inspectorFront = null; this.state = null; this.store = null; - this.subgridToParentMap = null; - this.target = null; - this.telemetry = null; - this.walker = null; this.boxModelHighlighterShown = null; this.flexboxHighlighterShown = null; diff --git a/devtools/server/actors/highlighters/css-grid.js b/devtools/server/actors/highlighters/css-grid.js index 3be3104bc5e9..8411f4de8672 100644 --- a/devtools/server/actors/highlighters/css-grid.js +++ b/devtools/server/actors/highlighters/css-grid.js @@ -136,9 +136,6 @@ const gCachedGridPattern = new Map(); * * @param {String} options.color * The color that should be used to draw the highlighter for this grid. - * @param {Number} options.globalAlpha - * The alpha (transparency) value that should be used to draw the highlighter for - * this grid. * @param {Boolean} options.showAllGridAreas * Shows all the grid area highlights for the current grid if isShown is * true. @@ -497,10 +494,6 @@ class CssGridHighlighter extends AutoRefreshHighlighter { return this.canvas.getCanvasContext("2d"); } - get globalAlpha() { - return this.options.globalAlpha || 1; - } - getElement(id) { return this.markup.getElement(this.ID_CLASS_PREFIX + id); } @@ -557,7 +550,7 @@ class CssGridHighlighter extends AutoRefreshHighlighter { } ctx.strokeStyle = this.color; - ctx.globalAlpha = GRID_GAP_ALPHA * this.globalAlpha; + ctx.globalAlpha = GRID_GAP_ALPHA; ctx.stroke(); ctx.restore(); @@ -887,7 +880,6 @@ class CssGridHighlighter extends AutoRefreshHighlighter { this.ctx.save(); this.ctx.translate(offset - canvasX, offset - canvasY); this.ctx.font = fontSize + "px " + GRID_FONT_FAMILY; - this.ctx.globalAlpha = this.globalAlpha; this.ctx.strokeStyle = this.color; this.ctx.textAlign = "center"; this.ctx.textBaseline = "middle"; @@ -1218,7 +1210,6 @@ class CssGridHighlighter extends AutoRefreshHighlighter { this.ctx.lineWidth = 2 * displayPixelRatio; this.ctx.strokeStyle = this.color; this.ctx.fillStyle = "white"; - this.ctx.globalAlpha = this.globalAlpha; // See param definitions of drawBubbleRect. const radius = 2 * displayPixelRatio; @@ -1458,7 +1449,7 @@ class CssGridHighlighter extends AutoRefreshHighlighter { } this.ctx.strokeStyle = this.color; - this.ctx.globalAlpha = GRID_LINES_PROPERTIES[lineType].alpha * this.globalAlpha; + this.ctx.globalAlpha = GRID_LINES_PROPERTIES[lineType].alpha; if (GRID_LINES_PROPERTIES[lineType].lineWidth) { this.ctx.lineWidth = GRID_LINES_PROPERTIES[lineType].lineWidth * devicePixelRatio; From 37d4796343f3a5c5ea408974b9d656e73b98a705 Mon Sep 17 00:00:00 2001 From: Gabriel Luong Date: Tue, 4 Jun 2019 13:57:47 -0400 Subject: [PATCH 6/8] Bug 1556729 - Only call setState for the TextNode when the value has changed. r=jdescottes Differential Revision: https://phabricator.services.mozilla.com/D33672 --- devtools/client/inspector/markup/views/text-editor.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/devtools/client/inspector/markup/views/text-editor.js b/devtools/client/inspector/markup/views/text-editor.js index 222ff5c00073..ff65098e4725 100644 --- a/devtools/client/inspector/markup/views/text-editor.js +++ b/devtools/client/inspector/markup/views/text-editor.js @@ -93,7 +93,10 @@ TextEditor.prototype = { update: async function() { try { const value = await getLongString(this.node.getNodeValue()); - this.textNode.setState({ value }); + + if (this.textNode.state.value !== value) { + this.textNode.setState({ value }); + } } catch (e) { console.error(e); } From 12c9031816c65b8a71bdec1c42c8a5293e47d577 Mon Sep 17 00:00:00 2001 From: premk Date: Tue, 4 Jun 2019 22:17:54 +0400 Subject: [PATCH 7/8] Bug 1548646 - correct misspelled preference name security.identitypopup.recordEventElemetry r=jaws --- browser/app/profile/firefox.js | 2 +- browser/components/BrowserGlue.jsm | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index e52e4def2d25..715223bc3829 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1375,7 +1375,7 @@ pref("pdfjs.previousHandler.alwaysAskBeforeHandling", false); // Is the sidebar positioned ahead of the content browser pref("sidebar.position_start", true); -pref("security.identitypopup.recordEventElemetry", true); +pref("security.identitypopup.recordEventTelemetry", true); // Block insecure active content on https pages pref("security.mixed_content.block_active_content", true); diff --git a/browser/components/BrowserGlue.jsm b/browser/components/BrowserGlue.jsm index 20373e7b360b..504005315be6 100644 --- a/browser/components/BrowserGlue.jsm +++ b/browser/components/BrowserGlue.jsm @@ -1451,7 +1451,10 @@ BrowserGlue.prototype = { }, _recordContentBlockingTelemetry() { - let recordIdentityPopupEvents = Services.prefs.getBoolPref("security.identitypopup.recordEventElemetry"); + let recordIdentityPopupEvents = Services.prefs.prefHasUserValue("security.identitypopup.recordEventElemetry") ? + Services.prefs.getBoolPref("security.identitypopup.recordEventElemetry") : + Services.prefs.getBoolPref("security.identitypopup.recordEventTelemetry"); + Services.telemetry.setEventRecordingEnabled("security.ui.identitypopup", recordIdentityPopupEvents); let tpEnabled = Services.prefs.getBoolPref("privacy.trackingprotection.enabled"); From d51924423e947992c64a59278b4e2510b71a9e6e Mon Sep 17 00:00:00 2001 From: Daniel Holbert Date: Tue, 4 Jun 2019 18:08:41 -0400 Subject: [PATCH 8/8] Bug 1473859: Change expectation for reftest unit-vh-vw-overflow-auto.html. rs=emilio The "-auto" and "-scroll" variants of this test used to be expected to mismatch ("!="), but our behavior changed with stylo such that they started matching, so it was marked as "fails !=". But now we've decided that the post-stylo matching behvavior is actually correct, so this patch is changing the manifest line to "==" so that the expectation is that they match. --- layout/reftests/css-valuesandunits/reftest.list | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/layout/reftests/css-valuesandunits/reftest.list b/layout/reftests/css-valuesandunits/reftest.list index e0e01d47bc3c..4e42d6f17778 100644 --- a/layout/reftests/css-valuesandunits/reftest.list +++ b/layout/reftests/css-valuesandunits/reftest.list @@ -14,6 +14,6 @@ fuzzy(0-1,0-4) == unit-vh-vw-overflow-auto.html unit-vh-vw-overflow-auto-ref.htm fails-if(!Android) == unit-vh-vw-overflow-scroll.html unit-vh-vw-overflow-scroll-ref.html fails-if(!Android) == unit-vh-vw-overflow-scroll-x.html unit-vh-vw-overflow-scroll-x-ref.html fails-if(!Android) == unit-vh-vw-overflow-scroll-y.html unit-vh-vw-overflow-scroll-y-ref.html -skip-if(Android) fuzzy-if(gtkWidget,0-1,0-2) fails != unit-vh-vw-overflow-auto.html unit-vh-vw-overflow-scroll.html +skip-if(Android) fuzzy-if(gtkWidget,0-1,0-2) == unit-vh-vw-overflow-auto.html unit-vh-vw-overflow-scroll.html == ch-width-1.html ch-width-1-ref.html