Bug 1481286 - Move command dispatcher from XULDocument to Document. r=bz
authorBrendan Dahl <bdahl@mozilla.com>
Tue, 07 Aug 2018 09:29:14 -0700
changeset 430697 271ad78ecd803a2baf59be86975aad3084fa1bd9
parent 430696 fc0dcbca21b57768a0bcd3aee60ed496318e9574
child 430698 4297c96bd1d9a0b170c85e79cb5feb62076b3cac
push id34410
push usertoros@mozilla.com
push dateThu, 09 Aug 2018 10:02:47 +0000
treeherdermozilla-central@f650c0df72f9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1481286
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1481286 - Move command dispatcher from XULDocument to Document. r=bz Allows non-XUL chrome privilege documents to also use the command dispatcher. The command dispatcher is created lazily since it will not always be used. Update test to reflect removal of the XUL attribute "commandDispatcher" from content privilege XUL. MozReview-Commit-ID: HUXMG9kx4ft
devtools/shared/webconsole/test/test_bug819670_getter_throws.html
dom/base/nsDocument.cpp
dom/base/nsGlobalWindowOuter.cpp
dom/base/nsIDocument.h
dom/webidl/Document.webidl
dom/webidl/XULDocument.webidl
dom/xul/XULDocument.cpp
dom/xul/XULDocument.h
dom/xul/moz.build
layout/xul/nsXULPopupManager.cpp
toolkit/content/widgets/general.js
--- a/devtools/shared/webconsole/test/test_bug819670_getter_throws.html
+++ b/devtools/shared/webconsole/test/test_bug819670_getter_throws.html
@@ -46,17 +46,16 @@ function onEvaluate(aState, aResponse)
 }
 
 function onInspect(aState, aResponse)
 {
   ok(!aResponse.error, "no response error");
 
   let expectedProps = {
     "addBroadcastListenerFor": { value: { type: "object" } },
-    "commandDispatcher": { get: { type: "object" } },
   };
 
   let props = aResponse.ownProperties;
   ok(props, "response properties available");
 
   if (props) {
     ok(Object.keys(props).length > Object.keys(expectedProps).length,
        "number of enumerable properties");
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -254,16 +254,17 @@
 #include "mozilla/dom/SVGDocument.h"
 #include "mozilla/dom/SVGSVGElement.h"
 #include "mozilla/dom/DocGroup.h"
 #include "mozilla/dom/TabGroup.h"
 #ifdef MOZ_XUL
 #include "mozilla/dom/MenuBoxObject.h"
 #include "mozilla/dom/TreeBoxObject.h"
 #include "nsIXULWindow.h"
+#include "nsXULCommandDispatcher.h"
 #include "nsXULPopupManager.h"
 #include "nsIDocShellTreeOwner.h"
 #endif
 #include "nsIPresShellInlines.h"
 
 #include "mozilla/DocLoadingTimelineMarker.h"
 
 #include "nsISpeculativeConnect.h"
@@ -1919,16 +1920,17 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mImages);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mEmbeds);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLinks);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mForms);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mScripts);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mApplets);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mAnchors);
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mAnonymousContents)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mCommandDispatcher)
 
   // Traverse all our nsCOMArrays.
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mStyleSheets)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPreloadingImages)
 
   for (uint32_t i = 0; i < tmp->mFrameRequestCallbacks.Length(); ++i) {
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mFrameRequestCallbacks[i]");
     cb.NoteXPCOMChild(tmp->mFrameRequestCallbacks[i].mCallback);
@@ -2009,16 +2011,17 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ns
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mLinks);
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mForms);
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mScripts);
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mApplets);
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mAnchors);
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mOrientationPendingPromise)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mFontFaceSet)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mReadyForIdle);
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mCommandDispatcher)
 
   tmp->mParentDocument = nullptr;
 
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mPreloadingImages)
 
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mIntersectionObservers)
 
   tmp->ClearAllBoxObjects();
@@ -10144,16 +10147,30 @@ nsIDocument::MaybeResolveReadyForIdle()
 {
   IgnoredErrorResult rv;
   Promise* readyPromise = GetDocumentReadyForIdle(rv);
   if (readyPromise) {
     readyPromise->MaybeResolve(this);
   }
 }
 
+nsIDOMXULCommandDispatcher*
+nsIDocument::GetCommandDispatcher()
+{
+  // Only chrome documents are allowed to use command dispatcher.
+  if (!nsContentUtils::IsChromeDoc(this)) {
+    return nullptr;
+  }
+  if (!mCommandDispatcher) {
+    // Create our command dispatcher and hook it up.
+    mCommandDispatcher = new nsXULCommandDispatcher(this);
+  }
+  return mCommandDispatcher;
+}
+
 static JSObject*
 GetScopeObjectOfNode(nsINode* node)
 {
     MOZ_ASSERT(node, "Must not be called with null.");
 
     // Window root occasionally keeps alive a node of a document whose
     // window is already dead. If in this brief period someone calls
     // GetPopupNode and we return that node, we can end up creating a
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -6262,26 +6262,26 @@ nsGlobalWindowOuter::UpdateCommands(cons
   }
 
   nsPIDOMWindowOuter *rootWindow = GetPrivateRoot();
   if (!rootWindow) {
     return;
   }
 
   nsIDocument* doc = rootWindow->GetExtantDoc();
-  // See if we contain a XUL document.
-  if (!doc || !doc->IsXULDocument()) {
+
+  if (!doc) {
     return;
   }
   // selectionchange action is only used for mozbrowser, not for XUL. So we bypass
   // XUL command dispatch if anAction is "selectionchange".
   if (!anAction.EqualsLiteral("selectionchange")) {
     // Retrieve the command dispatcher and call updateCommands on it.
     nsIDOMXULCommandDispatcher* xulCommandDispatcher =
-      doc->AsXULDocument()->GetCommandDispatcher();
+      doc->GetCommandDispatcher();
     if (xulCommandDispatcher) {
       nsContentUtils::AddScriptRunner(new CommandDispatcher(xulCommandDispatcher,
                                                             anAction));
     }
   }
 }
 
 Selection*
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -10,16 +10,17 @@
 #include "nsAutoPtr.h"                   // for member
 #include "nsCOMArray.h"                  // for member
 #include "nsCompatibility.h"             // for member
 #include "nsCOMPtr.h"                    // for member
 #include "nsGkAtoms.h"                   // for static class members
 #include "nsIApplicationCache.h"
 #include "nsIApplicationCacheContainer.h"
 #include "nsIContentViewer.h"
+#include "nsIDOMXULCommandDispatcher.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsILoadContext.h"
 #include "nsILoadGroup.h"                // for member (in nsCOMPtr)
 #include "nsINode.h"                     // for base class
 #include "nsIParser.h"
 #include "nsIPresShell.h"
 #include "nsIChannelEventSink.h"
 #include "nsIProgressEventSink.h"
@@ -3320,16 +3321,17 @@ public:
   already_AddRefed<mozilla::dom::Promise> BlockParsing(mozilla::dom::Promise& aPromise,
                                                        const mozilla::dom::BlockParsingOptions& aOptions,
                                                        mozilla::ErrorResult& aRv);
 
   already_AddRefed<nsIURI> GetMozDocumentURIIfNotForErrorPages();
 
   mozilla::dom::Promise* GetDocumentReadyForIdle(mozilla::ErrorResult& aRv);
 
+  nsIDOMXULCommandDispatcher* GetCommandDispatcher();
   already_AddRefed<nsINode> GetPopupNode();
   void SetPopupNode(nsINode* aNode);
   nsINode* GetPopupRangeParent(ErrorResult& aRv);
   int32_t GetPopupRangeOffset(ErrorResult& aRv);
   already_AddRefed<nsINode> GetTooltipNode();
   void SetTooltipNode(nsINode* aNode) { /* do nothing */ }
 
   // ParentNode
@@ -4493,16 +4495,18 @@ protected:
 
   // Used in conjunction with the create-an-element-for-the-token algorithm to
   // prevent custom element constructors from being able to use document.open(),
   // document.close(), and document.write() when they are invoked by the parser.
   uint32_t mThrowOnDynamicMarkupInsertionCounter;
 
   // Count of unload/beforeunload/pagehide operations in progress.
   uint32_t mIgnoreOpensDuringUnloadCounter;
+
+  nsCOMPtr<nsIDOMXULCommandDispatcher> mCommandDispatcher; // [OWNER] of the focus tracker
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsIDocument, NS_IDOCUMENT_IID)
 
 /**
  * mozAutoSubtreeModified batches DOM mutations so that a DOMSubtreeModified
  * event is dispatched, if necessary, when the outermost mozAutoSubtreeModified
  * object is deleted.
--- a/dom/webidl/Document.webidl
+++ b/dom/webidl/Document.webidl
@@ -402,16 +402,21 @@ partial interface Document {
   [ChromeOnly] readonly attribute URI? mozDocumentURIIfNotForErrorPages;
 
   // A promise that is resolved, with this document itself, when we have both
   // fired DOMContentLoaded and are ready to start layout.  This is used for the
   // "document_idle" webextension script injection point.
   [ChromeOnly, Throws]
   readonly attribute Promise<Document> documentReadyForIdle;
 
+  // Lazily created command dispatcher, returns null if the document is not
+  // chrome privileged.
+  [ChromeOnly]
+  readonly attribute XULCommandDispatcher? commandDispatcher;
+
   [ChromeOnly]
   attribute Node? popupNode;
 
   /**
    * These attributes correspond to rangeParent and rangeOffset. They will help
    * you find where in the DOM the popup is happening. Can be accessed only
    * during a popup event. Accessing any other time will be an error.
    */
--- a/dom/webidl/XULDocument.webidl
+++ b/dom/webidl/XULDocument.webidl
@@ -5,16 +5,14 @@
  */
 
 interface XULCommandDispatcher;
 interface MozObserver;
 
 [Func="IsChromeOrXBL"]
 interface XULDocument : Document {
 
-  readonly attribute XULCommandDispatcher? commandDispatcher;
-
   [Throws]
   void addBroadcastListenerFor(Element broadcaster, Element observer,
                                DOMString attr);
   void removeBroadcastListenerFor(Element broadcaster, Element observer,
                                   DOMString attr);
 };
--- a/dom/xul/XULDocument.cpp
+++ b/dom/xul/XULDocument.cpp
@@ -42,17 +42,16 @@
 #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"
-#include "nsXULCommandDispatcher.h"
 #include "nsXULElement.h"
 #include "nsXULPrototypeCache.h"
 #include "mozilla/Logging.h"
 #include "nsIFrame.h"
 #include "nsXBLService.h"
 #include "nsCExternalHandlerService.h"
 #include "nsMimeTypes.h"
 #include "nsIObjectInputStream.h"
@@ -230,24 +229,22 @@ NS_IMPL_CYCLE_COLLECTION_CLASS(XULDocume
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(XULDocument, XMLDocument)
     NS_ASSERTION(!nsCCUncollectableMarker::InGeneration(cb, tmp->GetMarkedCCGeneration()),
                  "Shouldn't traverse XULDocument!");
     // XXX tmp->mForwardReferences?
     // XXX tmp->mContextStack?
 
     NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mCurrentPrototype)
-    NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mCommandDispatcher)
     NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPrototypes)
     NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLocalStore)
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(XULDocument, XMLDocument)
-    NS_IMPL_CYCLE_COLLECTION_UNLINK(mCommandDispatcher)
     NS_IMPL_CYCLE_COLLECTION_UNLINK(mLocalStore)
     //XXX We should probably unlink all the objects we traverse.
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED(XULDocument,
                                              XMLDocument,
                                              nsIStreamLoaderObserver,
                                              nsICSSLoaderObserver,
@@ -1150,19 +1147,16 @@ XULDocument::Clone(mozilla::dom::NodeInf
 //
 
 nsresult
 XULDocument::Init()
 {
     nsresult rv = XMLDocument::Init();
     NS_ENSURE_SUCCESS(rv, rv);
 
-    // Create our command dispatcher and hook it up.
-    mCommandDispatcher = new nsXULCommandDispatcher(this);
-
     if (gRefCnt++ == 0) {
         // ensure that the XUL prototype cache is instantiated successfully,
         // so that we can use nsXULPrototypeCache::GetInstance() without
         // null-checks in the rest of the class.
         nsXULPrototypeCache* cache = nsXULPrototypeCache::GetInstance();
         if (!cache) {
           NS_ERROR("Could not instantiate nsXULPrototypeCache");
           return NS_ERROR_FAILURE;
--- a/dom/xul/XULDocument.h
+++ b/dom/xul/XULDocument.h
@@ -10,17 +10,16 @@
 #include "nsCOMPtr.h"
 #include "nsXULPrototypeDocument.h"
 #include "nsTArray.h"
 
 #include "mozilla/dom/XMLDocument.h"
 #include "mozilla/StyleSheet.h"
 #include "nsForwardReference.h"
 #include "nsIContent.h"
-#include "nsIDOMXULCommandDispatcher.h"
 #include "nsCOMArray.h"
 #include "nsIURI.h"
 #include "nsIStreamListener.h"
 #include "nsIStreamLoader.h"
 #include "nsICSSLoaderObserver.h"
 #include "nsIXULStore.h"
 
 #include "mozilla/Attributes.h"
@@ -132,20 +131,16 @@ public:
 
     NS_IMETHOD OnScriptCompileComplete(JSScript* aScript, nsresult aStatus) override;
 
     NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(XULDocument, XMLDocument)
 
     void TraceProtos(JSTracer* aTrc);
 
     // WebIDL API
-    nsIDOMXULCommandDispatcher* GetCommandDispatcher() const
-    {
-        return mCommandDispatcher;
-    }
     void AddBroadcastListenerFor(Element& aBroadcaster, Element& aListener,
                                  const nsAString& aAttr, ErrorResult& aRv);
     void RemoveBroadcastListenerFor(Element& aBroadcaster, Element& aListener,
                                     const nsAString& aAttr);
 
 protected:
     virtual ~XULDocument();
 
@@ -216,18 +211,16 @@ protected:
      * Since ResumeWalk is interruptible, it's possible that last
      * stylesheet finishes loading while the PD walk is still in
      * progress (waiting for an overlay to finish loading).
      * mStillWalking prevents DoneLoading (and StartLayout) from being
      * called in this situation.
      */
     bool                       mStillWalking;
 
-    nsCOMPtr<nsIDOMXULCommandDispatcher>     mCommandDispatcher; // [OWNER] of the focus tracker
-
     uint32_t mPendingSheets;
 
     /**
      * document lightweight theme for use with :-moz-lwtheme, :-moz-lwtheme-brighttext
      * and :-moz-lwtheme-darktext
      */
     DocumentTheme                         mDocLWTheme;
 
--- a/dom/xul/moz.build
+++ b/dom/xul/moz.build
@@ -11,16 +11,17 @@ if CONFIG['MOZ_BUILD_APP'] == 'browser':
     DEFINES['MOZ_BREAK_XUL_OVERLAYS'] = True
 
 MOCHITEST_MANIFESTS += ['test/mochitest.ini']
 
 MOCHITEST_CHROME_MANIFESTS += ['test/chrome.ini']
 
 if CONFIG['MOZ_XUL']:
     EXPORTS += [
+        'nsXULCommandDispatcher.h',
         'nsXULElement.h',
     ]
 
     EXPORTS.mozilla.dom += [
         'XULFrameElement.h',
         'XULPopupElement.h',
         'XULScrollElement.h',
     ]
--- a/layout/xul/nsXULPopupManager.cpp
+++ b/layout/xul/nsXULPopupManager.cpp
@@ -1948,22 +1948,20 @@ nsXULPopupManager::UpdateMenuItems(nsICo
   // command attribute. If so, then several attributes must potentially be updated.
 
   nsCOMPtr<nsIDocument> document = aPopup->GetUncomposedDoc();
   if (!document) {
     return;
   }
 
   // When a menu is opened, make sure that command updating is unlocked first.
-  if (document->IsXULDocument()) {
-    nsCOMPtr<nsIDOMXULCommandDispatcher> xulCommandDispatcher =
-      document->AsXULDocument()->GetCommandDispatcher();
-    if (xulCommandDispatcher) {
-      xulCommandDispatcher->Unlock();
-    }
+  nsCOMPtr<nsIDOMXULCommandDispatcher> commandDispatcher =
+    document->GetCommandDispatcher();
+  if (commandDispatcher) {
+    commandDispatcher->Unlock();
   }
 
   for (nsCOMPtr<nsIContent> grandChild = aPopup->GetFirstChild();
        grandChild;
        grandChild = grandChild->GetNextSibling()) {
     if (grandChild->IsXULElement(nsGkAtoms::menugroup)) {
       if (grandChild->GetChildCount() == 0) {
         continue;
--- a/toolkit/content/widgets/general.js
+++ b/toolkit/content/widgets/general.js
@@ -45,31 +45,25 @@ class MozDropmarker extends MozXULElemen
     }
   }
 }
 
 customElements.define("dropmarker", MozDropmarker);
 
 class MozCommandSet extends MozXULElement {
   connectedCallback() {
-    if (this.getAttribute("commandupdater")) {
-      let events = this.getAttribute("events");
-      if (events === "") {
-        events = "*";
-      }
-      let targets = this.getAttribute("targets");
-      if (targets === "") {
-        targets = "*";
-      }
+    if (this.getAttribute("commandupdater") === "true") {
+      const events = this.getAttribute("events") || "*";
+      const targets = this.getAttribute("targets") || "*";
       document.commandDispatcher.addCommandUpdater(this, events, targets);
     }
   }
 
   disconnectedCallback() {
-    if (this.getAttribute("commandupdater")) {
+    if (this.getAttribute("commandupdater") === "true") {
       document.commandDispatcher.removeCommandUpdater(this);
     }
   }
 }
 
 customElements.define("commandset", MozCommandSet);
 
 }