Bug 1329093 - Part 4: stylo: Delay SVG mapped attr resolution till later; r=bz
authorManish Goregaokar <manishearth@gmail.com>
Wed, 22 Feb 2017 17:19:04 -0800
changeset 346876 33e22504454407f07e8b1409012b3618043dbb6f
parent 346875 0d2f87e66db24046726d4b652cd83d18f9e00b5b
child 346877 610c74da63408e2755dd3fde13d4ae8ae61a3f6e
push id31480
push usercbook@mozilla.com
push dateFri, 10 Mar 2017 10:37:06 +0000
treeherdermozilla-central@e18d3dd20e8d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1329093
milestone55.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 1329093 - Part 4: stylo: Delay SVG mapped attr resolution till later; r=bz MozReview-Commit-ID: 2GvHPg1egjS
dom/base/Element.h
dom/base/nsDocument.cpp
dom/base/nsDocument.h
dom/base/nsIDocument.h
dom/base/nsNodeUtils.cpp
dom/html/HTMLImageElement.cpp
dom/html/HTMLImageElement.h
dom/html/nsGenericHTMLElement.cpp
dom/html/nsGenericHTMLElement.h
dom/svg/crashtests/1329093-1.html
dom/svg/crashtests/1329093-2.html
dom/svg/crashtests/crashtests.list
dom/svg/nsSVGElement.cpp
dom/svg/nsSVGElement.h
layout/style/ServoStyleSet.cpp
--- a/dom/base/Element.h
+++ b/dom/base/Element.h
@@ -1121,17 +1121,23 @@ public:
    * namespace ID must not be kNameSpaceID_Unknown and the name must not be
    * null.  Note that this can only return info on attributes that actually
    * live on this element (and is only virtual to handle XUL prototypes).  That
    * is, this should only be called from methods that only care about attrs
    * that effectively live in mAttrsAndChildren.
    */
   virtual BorrowedAttrInfo GetAttrInfo(int32_t aNamespaceID, nsIAtom* aName) const;
 
-  virtual void NodeInfoChanged()
+  /**
+   * Called when we have been adopted, and the information of the
+   * node has been changed.
+   *
+   * The new document can be reached via OwnerDoc().
+   */
+  virtual void NodeInfoChanged(nsIDocument* aOldDoc)
   {
   }
 
   /**
    * Parse a string into an nsAttrValue for a CORS attribute.  This
    * never fails.  The resulting value is an enumerated value whose
    * GetEnumValue() returns one of the above constants.
    */
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -6010,16 +6010,38 @@ nsDocument::IsWebComponentsEnabled(nsPID
 
     return perm == nsIPermissionManager::ALLOW_ACTION;
   }
 
   return false;
 }
 
 void
+nsDocument::ScheduleSVGForPresAttrEvaluation(nsSVGElement* aSVG)
+{
+  mLazySVGPresElements.PutEntry(aSVG);
+}
+
+void
+nsDocument::UnscheduleSVGForPresAttrEvaluation(nsSVGElement* aSVG)
+{
+  mLazySVGPresElements.RemoveEntry(aSVG);
+}
+
+void
+nsDocument::ResolveScheduledSVGPresAttrs()
+{
+  for (auto iter = mLazySVGPresElements.Iter(); !iter.Done(); iter.Next()) {
+    nsSVGElement* svg = iter.Get()->GetKey();
+    svg->UpdateContentDeclarationBlock(StyleBackendType::Servo);
+  }
+  mLazySVGPresElements.Clear();
+}
+
+void
 nsDocument::RegisterElement(JSContext* aCx, const nsAString& aType,
                             const ElementRegistrationOptions& aOptions,
                             JS::MutableHandle<JSObject*> aRetval,
                             ErrorResult& rv)
 {
   RefPtr<CustomElementRegistry> registry(GetCustomElementRegistry());
   if (!registry) {
     rv.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
--- a/dom/base/nsDocument.h
+++ b/dom/base/nsDocument.h
@@ -793,16 +793,19 @@ public:
   virtual void RemoveIntersectionObserver(
     mozilla::dom::DOMIntersectionObserver* aObserver) override;
   virtual void UpdateIntersectionObservations() override;
   virtual void ScheduleIntersectionObserverNotification() override;
   virtual void NotifyIntersectionObservers() override;
 
   virtual void NotifyLayerManagerRecreated() override;
 
+  virtual void ScheduleSVGForPresAttrEvaluation(nsSVGElement* aSVG) override;
+  virtual void UnscheduleSVGForPresAttrEvaluation(nsSVGElement* aSVG) override;
+  virtual void ResolveScheduledSVGPresAttrs() override;
 
 private:
   void AddOnDemandBuiltInUASheet(mozilla::StyleSheet* aSheet);
   nsRadioGroupStruct* GetRadioGroupInternal(const nsAString& aName) const;
   void SendToConsole(nsCOMArray<nsISecurityConsoleMessage>& aMessages);
 
 public:
   // nsIDOMNode
@@ -1637,16 +1640,21 @@ private:
   nsCOMPtr<nsIDocument> mMasterDocument;
   RefPtr<mozilla::dom::ImportManager> mImportManager;
   nsTArray<nsCOMPtr<nsINode> > mSubImportLinks;
 
   // Set to true when the document is possibly controlled by the ServiceWorker.
   // Used to prevent multiple requests to ServiceWorkerManager.
   bool mMaybeServiceWorkerControlled;
 
+  // We lazily calculate declaration blocks for SVG elements
+  // with mapped attributes in Servo mode. This list contains all elements which
+  // need lazy resolution
+  nsTHashtable<nsPtrHashKey<nsSVGElement>> mLazySVGPresElements;
+
 #ifdef DEBUG
 public:
   bool mWillReparent;
 #endif
 };
 
 class nsDocumentOnStack
 {
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -88,16 +88,17 @@ class nsIStreamListener;
 class nsIStructuredCloneContainer;
 class nsIURI;
 class nsIVariant;
 class nsViewManager;
 class nsPresContext;
 class nsRange;
 class nsScriptLoader;
 class nsSMILAnimationController;
+class nsSVGElement;
 class nsTextNode;
 class nsWindowSizes;
 class nsDOMCaretPosition;
 class nsViewportInfo;
 class nsIGlobalObject;
 struct nsCSSSelectorList;
 
 namespace mozilla {
@@ -1004,16 +1005,30 @@ public:
       return parentTimeStamp;
     }
 
     return mPageUnloadingEventTimeStamp;
   }
 
   virtual void NotifyLayerManagerRecreated() = 0;
 
+  /**
+   * Add an SVG element to the list of elements that need
+   * their mapped attributes resolved to a Servo declaration block.
+   *
+   * These are weak pointers, please manually unschedule them when an element
+   * is removed.
+   */
+  virtual void ScheduleSVGForPresAttrEvaluation(nsSVGElement* aSVG) = 0;
+  // Unschedule an element scheduled by ScheduleFrameRequestCallback (e.g. for when it is destroyed)
+  virtual void UnscheduleSVGForPresAttrEvaluation(nsSVGElement* aSVG) = 0;
+
+  // Resolve all SVG pres attrs scheduled in ScheduleSVGForPresAttrEvaluation
+  virtual void ResolveScheduledSVGPresAttrs() = 0;
+
 protected:
   virtual Element *GetRootElementInternal() const = 0;
 
   void SetPageUnloadingEventTimeStamp()
   {
     MOZ_ASSERT(!mPageUnloadingEventTimeStamp);
     mPageUnloadingEventTimeStamp = mozilla::TimeStamp::NowLoRes();
   }
--- a/dom/base/nsNodeUtils.cpp
+++ b/dom/base/nsNodeUtils.cpp
@@ -513,17 +513,17 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNod
     if (aNode->IsElement()) {
       Element* element = aNode->AsElement();
       oldDoc->ClearBoxObjectFor(element);
       wasRegistered = oldDoc->UnregisterActivityObserver(element);
     }
 
     aNode->mNodeInfo.swap(newNodeInfo);
     if (elem) {
-      elem->NodeInfoChanged();
+      elem->NodeInfoChanged(oldDoc);
     }
 
     nsIDocument* newDoc = aNode->OwnerDoc();
     if (newDoc) {
       // XXX what if oldDoc is null, we don't know if this should be
       // registered or not! Can that really happen?
       if (wasRegistered) {
         newDoc->RegisterActivityObserver(aNode->AsElement());
--- a/dom/html/HTMLImageElement.cpp
+++ b/dom/html/HTMLImageElement.cpp
@@ -705,17 +705,17 @@ HTMLImageElement::MaybeLoadImage()
 EventStates
 HTMLImageElement::IntrinsicState() const
 {
   return nsGenericHTMLElement::IntrinsicState() |
     nsImageLoadingContent::ImageState();
 }
 
 void
-HTMLImageElement::NodeInfoChanged()
+HTMLImageElement::NodeInfoChanged(nsIDocument* aOldDoc)
 {
   // Resetting the last selected source if adoption steps are run.
   mLastSelectedSource = nullptr;
 }
 
 // static
 already_AddRefed<HTMLImageElement>
 HTMLImageElement::Image(const GlobalObject& aGlobal,
--- a/dom/html/HTMLImageElement.h
+++ b/dom/html/HTMLImageElement.h
@@ -89,17 +89,17 @@ public:
   virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent,
                               nsIContent* aBindingParent,
                               bool aCompileEventHandlers) override;
   virtual void UnbindFromTree(bool aDeep, bool aNullParent) override;
 
   virtual EventStates IntrinsicState() const override;
   virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult) const override;
 
-  virtual void NodeInfoChanged() override;
+  virtual void NodeInfoChanged(nsIDocument* aOldDoc) override;
 
   nsresult CopyInnerTo(Element* aDest);
 
   void MaybeLoadImage();
 
   bool IsMap()
   {
     return GetBoolAttr(nsGkAtoms::ismap);
--- a/dom/html/nsGenericHTMLElement.cpp
+++ b/dom/html/nsGenericHTMLElement.cpp
@@ -2845,17 +2845,17 @@ nsGenericHTMLFormElementWithState::Resto
     history->RemoveState(mStateKey);
     return result;
   }
 
   return false;
 }
 
 void
-nsGenericHTMLFormElementWithState::NodeInfoChanged()
+nsGenericHTMLFormElementWithState::NodeInfoChanged(nsIDocument* aOldDoc)
 {
   mStateKey.SetIsVoid(true);
 }
 
 nsSize
 nsGenericHTMLElement::GetWidthHeightForImage(RefPtr<imgRequestProxy>& aImageRequest)
 {
   nsSize size(0,0);
--- a/dom/html/nsGenericHTMLElement.h
+++ b/dom/html/nsGenericHTMLElement.h
@@ -1394,17 +1394,17 @@ public:
    *         value of RestoreState() otherwise.
    */
   bool RestoreFormControlState();
 
   /**
    * Called when we have been cloned and adopted, and the information of the
    * node has been changed.
    */
-  virtual void NodeInfoChanged() override;
+  virtual void NodeInfoChanged(nsIDocument* aOldDoc) override;
 
 protected:
   /* Generates the state key for saving the form state in the session if not
      computed already. The result is stored in mStateKey on success */
   nsresult GenerateStateKey();
 
   /* Used to store the key to that element in the session. Is void until
      GenerateStateKey has been used */
new file mode 100644
--- /dev/null
+++ b/dom/svg/crashtests/1329093-1.html
@@ -0,0 +1,9 @@
+<html>
+
+<head>
+</head>
+<body>
+Loading the below iframe should not crash Firefox in Stylo mode.
+<iframe style="display: none" src='data:text/html,<svg height="100" width="100"><circle cx="50" cy="50" r="40" stroke="yellow" stroke-width="2" fill="green"/> </svg>'></iframe>
+</body>
+</html>
\ No newline at end of file
new file mode 100644
--- /dev/null
+++ b/dom/svg/crashtests/1329093-2.html
@@ -0,0 +1,28 @@
+<html class="reftest-wait">
+
+<head>
+</head>
+<body>
+
+Loading the below iframe should not crash Firefox in Stylo mode.
+<svg height="100" width="100" id="svgElement">
+    <circle cx="50" cy="50" r="40" stroke="yellow" stroke-width="2" fill="green"/>
+</svg>
+
+<iframe src="" id="myFrame"></iframe>
+<div style="display: none" id="triggerRestyle"></div>
+<script type="text/javascript">
+let frame = document.getElementById("myFrame");
+frame.onload = function() {
+    let baz = frame.contentDocument.adoptNode(document.getElementById("svgElement"));
+    frame.contentDocument.body.appendChild(baz);
+    baz = null;
+    frame.remove();
+    frame = null;
+    SpecialPowers.gc();
+    let color = getComputedStyle(document.getElementById('triggerRestyle')).color;
+    document.documentElement.className = "";
+}
+</script>
+</body>
+</html>
--- a/dom/svg/crashtests/crashtests.list
+++ b/dom/svg/crashtests/crashtests.list
@@ -83,8 +83,10 @@ load 1282985-1.svg
 load zero-size-image.svg
 load 1322286.html
 load 1329849-1.svg
 load 1329849-2.svg
 load 1329849-3.svg
 load 1329849-4.svg
 load 1329849-5.svg
 load 1329849-6.svg
+load 1329093-1.html
+load 1329093-2.html
--- a/dom/svg/nsSVGElement.cpp
+++ b/dom/svg/nsSVGElement.cpp
@@ -92,16 +92,17 @@ nsSVGEnumMapping nsSVGElement::sSVGUnitT
 
 nsSVGElement::nsSVGElement(already_AddRefed<mozilla::dom::NodeInfo>& aNodeInfo)
   : nsSVGElementBase(aNodeInfo)
 {
 }
 
 nsSVGElement::~nsSVGElement()
 {
+  OwnerDoc()->UnscheduleSVGForPresAttrEvaluation(this);
 }
 
 JSObject*
 nsSVGElement::WrapNode(JSContext *aCx, JS::Handle<JSObject*> aGivenProto)
 {
   return SVGElementBinding::Wrap(aCx, this, aGivenProto);
 }
 
@@ -298,19 +299,18 @@ nsSVGElement::AfterSetAttr(int32_t aName
              "Unexpected use of nsMappedAttributes within SVG");
 
   // If this is an svg presentation attribute we need to map it into
   // the content declaration block.
   // XXX For some reason incremental mapping doesn't work, so for now
   // just delete the style rule and lazily reconstruct it as needed).
   if (aNamespaceID == kNameSpaceID_None && IsAttributeMapped(aName)) {
     mContentDeclarationBlock = nullptr;
-    // TODO we should be doing this lazily by caching these on the styleset
     if (OwnerDoc()->GetStyleBackendType() == StyleBackendType::Servo) {
-      UpdateContentDeclarationBlock(StyleBackendType::Servo);
+      OwnerDoc()->ScheduleSVGForPresAttrEvaluation(this);
     }
   }
 
   if (IsEventAttributeName(aName) && aValue) {
     MOZ_ASSERT(aValue->Type() == nsAttrValue::eString,
                "Expected string value for script body");
     nsresult rv = SetEventHandler(GetEventNameForAttr(aName),
                                   aValue->GetStringValue());
@@ -902,16 +902,24 @@ nsSVGElement::GetAttributeChangeHint(con
 }
 
 bool
 nsSVGElement::IsNodeOfType(uint32_t aFlags) const
 {
   return !(aFlags & ~eCONTENT);
 }
 
+void
+nsSVGElement::NodeInfoChanged(nsIDocument* aOldDoc)
+{
+  aOldDoc->UnscheduleSVGForPresAttrEvaluation(this);
+  mContentDeclarationBlock = nullptr;
+  OwnerDoc()->ScheduleSVGForPresAttrEvaluation(this);
+}
+
 NS_IMETHODIMP
 nsSVGElement::WalkContentStyleRules(nsRuleWalker* aRuleWalker)
 {
 #ifdef DEBUG
 //  printf("nsSVGElement(%p)::WalkContentStyleRules()\n", this);
 #endif
   if (!mContentDeclarationBlock) {
     UpdateContentDeclarationBlock(StyleBackendType::Gecko);
--- a/dom/svg/nsSVGElement.h
+++ b/dom/svg/nsSVGElement.h
@@ -109,16 +109,22 @@ public:
   virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute,
                              bool aNotify) override;
 
   virtual nsChangeHint GetAttributeChangeHint(const nsIAtom* aAttribute,
                                               int32_t aModType) const override;
 
   virtual bool IsNodeOfType(uint32_t aFlags) const override;
 
+  /**
+   * We override the default to unschedule computation of Servo declaration blocks
+   * when adopted across documents.
+   */
+  virtual void NodeInfoChanged(nsIDocument* aOldDoc) override;
+
   NS_IMETHOD WalkContentStyleRules(nsRuleWalker* aRuleWalker) override;
   void WalkAnimatedContentStyleRules(nsRuleWalker* aRuleWalker);
 
   NS_IMETHOD_(bool) IsAttributeMapped(const nsIAtom* aAttribute) const override;
 
   static const MappedAttributeEntry sFillStrokeMap[];
   static const MappedAttributeEntry sGraphicsMap[];
   static const MappedAttributeEntry sTextContentElementsMap[];
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@@ -194,16 +194,18 @@ ServoStyleSet::GetContext(already_AddRef
 }
 
 void
 ServoStyleSet::ResolveMappedAttrDeclarationBlocks()
 {
   if (nsHTMLStyleSheet* sheet = mPresContext->Document()->GetAttributeStyleSheet()) {
     sheet->CalculateMappedServoDeclarations();
   }
+
+  mPresContext->Document()->ResolveScheduledSVGPresAttrs();
 }
 
 void
 ServoStyleSet::PreTraverse()
 {
   ResolveMappedAttrDeclarationBlocks();
 
   // Process animation stuff that we should avoid doing during the parallel