Changeset 250843 in webkit

Timestamp:

Oct 8, 2019 11:39:04 AM (5 years ago)

Author:

Chris Dumez

Message:

Make sure ActiveDOMObject properly deals with detached documents
​https://bugs.webkit.org/show_bug.cgi?id=202596

Reviewed by Geoffrey Garen.

For detached document, the script execution context is their context document.
The ActiveDOMObject constructor taking a Document& would make sure to get the
document's contextDocument. However, if the ActiveDOMObject constructor taking
a ScriptExecutionContext* is called, it would assume this is the right script
execution context, which is unsafe. In this patch, all ActiveDOMObject
constructors now check for detached documents and make sure to use their
context document when necessary.

• dom/ActiveDOMObject.cpp:

(WebCore::suitableScriptExecutionContext):
(WebCore::ActiveDOMObject::ActiveDOMObject):

• dom/ActiveDOMObject.h:
• dom/Document.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/ActiveDOMObject.cpp (modified) (2 diffs)
• dom/ActiveDOMObject.h (modified) (1 diff)
• dom/Document.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-10-08  Chris Dumez  <cdumez@apple.com>
+
+        Make sure ActiveDOMObject properly deals with detached documents
+        https://bugs.webkit.org/show_bug.cgi?id=202596
+
+        Reviewed by Geoffrey Garen.
+
+        For detached document, the script execution context is their context document.
+        The ActiveDOMObject constructor taking a Document& would make sure to get the
+        document's contextDocument. However, if the ActiveDOMObject constructor taking
+        a ScriptExecutionContext* is called, it would assume this is the right script
+        execution context, which is unsafe. In this patch, all ActiveDOMObject
+        constructors now check for detached documents and make sure to use their
+        context document when necessary.
+
+        * dom/ActiveDOMObject.cpp:
+        (WebCore::suitableScriptExecutionContext):
+        (WebCore::ActiveDOMObject::ActiveDOMObject):
+        * dom/ActiveDOMObject.h:
+        * dom/Document.h:
+
 2019-10-08  Ross Kirsling  <ross.kirsling@sony.com>
 

trunk/Source/WebCore/dom/ActiveDOMObject.cpp

@@ -33 +33 @@
 namespace WebCore {
 
-ActiveDOMObject::ActiveDOMObject(ScriptExecutionContext* scriptExecutionContext)
-    : ContextDestructionObserver(scriptExecutionContext)
-    , m_pendingActivityCount(0)
-#if !ASSERT_DISABLED
-    , m_suspendIfNeededWasCalled(false)
-#endif
+static inline ScriptExecutionContext* suitableScriptExecutionContext(ScriptExecutionContext* scriptExecutionContext)
 {
-    ASSERT(!is<Document>(m_scriptExecutionContext) || &downcast<Document>(m_scriptExecutionContext)->contextDocument() == downcast<Document>(m_scriptExecutionContext));
-    if (!m_scriptExecutionContext)
+    // For detached documents, make sure we observe their context document instead.
+    return is<Document>(scriptExecutionContext) ? &downcast<Document>(*scriptExecutionContext).contextDocument() : scriptExecutionContext;
+}
+
+inline ActiveDOMObject::ActiveDOMObject(ScriptExecutionContext* context, CheckedScriptExecutionContextType)
+    : ContextDestructionObserver(context)
+{
+    ASSERT(!is<Document>(context) || &downcast<Document>(context)->contextDocument() == downcast<Document>(context));
+    if (!context)
         return;
 
-    ASSERT(m_scriptExecutionContext->isContextThread());
-    m_scriptExecutionContext->didCreateActiveDOMObject(*this);
+    ASSERT(context->isContextThread());
+    context->didCreateActiveDOMObject(*this);
+}
+
+ActiveDOMObject::ActiveDOMObject(ScriptExecutionContext* scriptExecutionContext)
+    : ActiveDOMObject(suitableScriptExecutionContext(scriptExecutionContext), CheckedScriptExecutionContext)
+{
+}
+
+ActiveDOMObject::ActiveDOMObject(Document* document)
+    : ActiveDOMObject(document ? &document->contextDocument() : nullptr, CheckedScriptExecutionContext)
+{
+}
+
+ActiveDOMObject::ActiveDOMObject(Document& document)
+    : ActiveDOMObject(&document.contextDocument(), CheckedScriptExecutionContext)
+{
 }
 
@@ -92 +109 @@
 }
 
-bool ActiveDOMObject::canSuspendForDocumentSuspension() const
-{
-    return false;
-}
-
 void ActiveDOMObject::suspend(ReasonForSuspension)
 {

trunk/Source/WebCore/dom/ActiveDOMObject.h

@@ -117 +117 @@
 protected:
     explicit ActiveDOMObject(ScriptExecutionContext*);
-    explicit ActiveDOMObject(Document*) = delete;
-    explicit ActiveDOMObject(Document&); // Implemented in Document.h
+    explicit ActiveDOMObject(Document*);
+    explicit ActiveDOMObject(Document&);
     virtual ~ActiveDOMObject();
 
 private:
-    unsigned m_pendingActivityCount;
+    enum CheckedScriptExecutionContextType { CheckedScriptExecutionContext };
+    ActiveDOMObject(ScriptExecutionContext*, CheckedScriptExecutionContextType);
+
+    unsigned m_pendingActivityCount { 0 };
 #if !ASSERT_DISABLED
-    bool m_suspendIfNeededWasCalled;
+    bool m_suspendIfNeededWasCalled { false };
     Ref<Thread> m_creationThread { Thread::current() };
 #endif

trunk/Source/WebCore/dom/Document.h

@@ -2105 +2105 @@
 }
 
-inline ActiveDOMObject::ActiveDOMObject(Document& document)
-    : ActiveDOMObject(static_cast<ScriptExecutionContext*>(&document.contextDocument()))
-{
-}
-
 } // namespace WebCore