Bug 1329093 - Part 4: stylo: Delay SVG mapped attr resolution till later; draft
authorManish Goregaokar <manishearth@gmail.com>
Wed, 22 Feb 2017 17:19:04 -0800
changeset 496352 f5172deee8bb63cbffa008d5d64a4dd16e03fe1e
parent 496351 24718fd4e0e2a3a6e4d181cf5a9b8794cfab8d82
child 496353 2f1a13fb08b4bec00aa99f4d3e0619fe808b545b
push id48571
push userbmo:manishearth@gmail.com
push dateFri, 10 Mar 2017 01:48:52 +0000
bugs1329093
milestone55.0a1
Bug 1329093 - Part 4: stylo: Delay SVG mapped attr resolution till later; 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