Bug 1403024 part 2 - Add more diagnosis information before crashing for adopting element across style backend. r?bholley draft
authorXidorn Quan <me@upsuper.org>
Tue, 26 Sep 2017 10:31:36 +1000
changeset 670178 7c1de0fb77a1fe2f20f36834619cf50be996cd53
parent 670175 f6e0e80c069914e32ac1bc0c4d6c6ababe928b54
child 733163 4737c188d4a0935bd4b3a8b5aca574158acddc53
push id81549
push userxquan@mozilla.com
push dateTue, 26 Sep 2017 00:52:42 +0000
reviewersbholley
bugs1403024
milestone58.0a1
Bug 1403024 part 2 - Add more diagnosis information before crashing for adopting element across style backend. r?bholley MozReview-Commit-ID: LN40EefYqVJ
dom/base/nsDocument.cpp
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -12,16 +12,17 @@
 #include "nsDocument.h"
 #include "nsIDocumentInlines.h"
 #include "mozilla/AnimationComparator.h"
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/AutoRestore.h"
 #include "mozilla/BinarySearch.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/EffectSet.h"
+#include "mozilla/EnumSet.h"
 #include "mozilla/IntegerRange.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/Likely.h"
 #include "mozilla/PresShell.h"
 #include "mozilla/URLExtraData.h"
 #include <algorithm>
 
 #include "mozilla/Logging.h"
@@ -7790,37 +7791,118 @@ nsDOMAttributeMap::BlastSubtreeToPieces(
 
   uint32_t count = aNode->GetChildCount();
   for (uint32_t i = 0; i < count; ++i) {
     BlastSubtreeToPieces(aNode->GetFirstChild());
     aNode->RemoveChildAt(0, false);
   }
 }
 
+enum class StyleDataType
+{
+  InlineStyle,
+  SMILOverride,
+  RestyleBits,
+  ServoData,
+};
+
 // Recursively check whether this node or its descendants contain any
 // pre-existing style declaration or any shared restyle flags.
-static bool
-NodeContainsAnyStyleData(nsINode* aNode)
-{
+static EnumSet<StyleDataType>
+StyleDataTypesWithNode(nsINode* aNode)
+{
+  EnumSet<StyleDataType> result;
   if (aNode->IsElement()) {
     Element* elem = aNode->AsElement();
-    if (elem->GetInlineStyleDeclaration() ||
-        elem->GetSMILOverrideStyleDeclaration() ||
-        elem->HasAnyOfFlags(ELEMENT_SHARED_RESTYLE_BITS) ||
-        elem->HasServoData()) {
-      return true;
+    if (elem->GetInlineStyleDeclaration()) {
+      result += StyleDataType::InlineStyle;
+    }
+    if (elem->GetSMILOverrideStyleDeclaration()) {
+      result += StyleDataType::SMILOverride;
+    }
+    if (elem->HasAnyOfFlags(ELEMENT_SHARED_RESTYLE_BITS)) {
+      result += StyleDataType::RestyleBits;
+    }
+    if (elem->HasServoData()) {
+      result += StyleDataType::ServoData;
     }
   }
   for (nsIContent* child = aNode->GetFirstChild();
        child; child = child->GetNextSibling()) {
-    if (NodeContainsAnyStyleData(child)) {
-      return true;
-    }
-  }
-  return false;
+    result += StyleDataTypesWithNode(child);
+  }
+  return result;
+}
+
+static void
+CheckCrossStyleBackendAdoption(nsIDocument* aOldDoc,
+                               nsIDocument* aNewDoc, nsINode* aAdoptedNode)
+{
+  EnumSet<StyleDataType> styleDataTypes = StyleDataTypesWithNode(aAdoptedNode);
+  if (styleDataTypes.isEmpty()) {
+    return;
+  }
+#ifdef MOZ_CRASHREPORTER
+  // We are adopting node with pre-existing style data across style
+  // backend. We want some more information to help diagnose when that
+  // can happen.
+  nsAutoCString note;
+  nsIDocument* geckoDoc;
+  if (aOldDoc->GetStyleBackendType() == StyleBackendType::Servo) {
+    note.AppendLiteral("Servo -> Gecko");
+    geckoDoc = aNewDoc;
+  } else {
+    note.AppendLiteral("Gecko -> Servo");
+    geckoDoc = aOldDoc;
+  }
+  note.AppendLiteral(", with style data:");
+#define APPEND_STYLE_DATA_TYPE(type_)                   \
+  if (styleDataTypes.contains(StyleDataType::type_)) {  \
+    note.AppendLiteral(" " #type_);                     \
+  }
+  APPEND_STYLE_DATA_TYPE(InlineStyle)
+  APPEND_STYLE_DATA_TYPE(SMILOverride)
+  APPEND_STYLE_DATA_TYPE(RestyleBits)
+  APPEND_STYLE_DATA_TYPE(ServoData)
+#undef APPEND_STYLE_DATA_TYPE
+
+  note.AppendLiteral("\nGecko doc: ");
+  if (nsContentUtils::IsSystemPrincipal(geckoDoc->NodePrincipal())) {
+    note.AppendLiteral("system, ");
+  }
+  if (geckoDoc->IsXULDocument()) {
+    note.AppendLiteral("XUL, ");
+  }
+  note.AppendLiteral("content-type: ");
+  nsAutoString contentType;
+  geckoDoc->GetContentType(contentType);
+  note.Append(contentType);
+  if (nsIURI* geckoURI = geckoDoc->GetOriginalURI()) {
+    note.AppendLiteral(", url: ");
+    if (nsContentUtils::SchemeIs(geckoURI, "http") ||
+        nsContentUtils::SchemeIs(geckoURI, "https") ||
+        nsContentUtils::SchemeIs(geckoURI, "file") ||
+        nsContentUtils::SchemeIs(geckoURI, "ftp")) {
+      nsAutoCString scheme;
+      geckoURI->GetScheme(scheme);
+      note.Append(scheme);
+      note.AppendLiteral("://*");
+    } else if (nsContentUtils::SchemeIs(geckoURI, "data")) {
+      note.AppendLiteral("data:*");
+    } else {
+      nsAutoCString spec;
+      geckoURI->GetSpec(spec);
+      note.Append(spec);
+    }
+  }
+  note.Append('\n');
+  CrashReporter::AppendAppNotesToCrashReport(note);
+#endif // MOZ_CRASHREPORTER
+  MOZ_CRASH("Must not adopt a node with pre-existing style data "
+            "into a document with different style backend");
 }
 
 nsINode*
 nsIDocument::AdoptNode(nsINode& aAdoptedNode, ErrorResult& rv)
 {
   nsINode* adoptedNode = &aAdoptedNode;
 
   // Scope firing mutation events so that we don't carry any state that
@@ -7930,19 +8012,17 @@ nsIDocument::AdoptNode(nsINode& aAdopted
   nsCOMPtr<nsIDocument> oldDocument = adoptedNode->OwnerDoc();
   bool sameDocument = oldDocument == this;
 
   AutoJSContext cx;
   JS::Rooted<JSObject*> newScope(cx, nullptr);
   if (!sameDocument) {
     if (MOZ_UNLIKELY(oldDocument->GetStyleBackendType() != GetStyleBackendType())) {
       NS_WARNING("Adopting node across different style backend");
-      MOZ_RELEASE_ASSERT(!NodeContainsAnyStyleData(adoptedNode),
-                         "Must not adopt a node with pre-existing style data "
-                         "into a document with different style backend");
+      CheckCrossStyleBackendAdoption(oldDocument, this, adoptedNode);
     }
     newScope = GetWrapper();
     if (!newScope && GetScopeObject() && GetScopeObject()->GetGlobalJSObject()) {
       // Make sure cx is in a semi-sane compartment before we call WrapNative.
       // It's kind of irrelevant, given that we're passing aAllowWrapping =
       // false, and documents should always insist on being wrapped in an
       // canonical scope. But we try to pass something sane anyway.
       JSAutoCompartment ac(cx, GetScopeObject()->GetGlobalJSObject());