REGRESSION (r226385?): Crash in com.apple.WebCore: WebCore::MediaQueryEvaluator:...
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Jan 2018 21:53:26 +0000 (21:53 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Jan 2018 21:53:26 +0000 (21:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181742
<rdar://problem/36334726>

Reviewed by David Kilzer.

Source/WebCore:

Test: fast/media/mediaqueryevaluator-crash.html

* css/MediaQueryEvaluator.cpp:
(WebCore::MediaQueryEvaluator::MediaQueryEvaluator):

Use WeakPtr<Document> instead of a plain Frame pointer.

(WebCore::MediaQueryEvaluator::evaluate const):

Get the frame via document.

* css/MediaQueryEvaluator.h:
* dom/Document.cpp:
(WebCore::Document::prepareForDestruction):

Take care to clear style resolver.

LayoutTests:

* fast/media/mediaqueryevaluator-crash-expected.txt: Added.
* fast/media/mediaqueryevaluator-crash.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@227082 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/media/mediaqueryevaluator-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/media/mediaqueryevaluator-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/MediaQueryEvaluator.cpp
Source/WebCore/css/MediaQueryEvaluator.h
Source/WebCore/dom/Document.cpp

index e1e2b34..2a9a33b 100644 (file)
@@ -1,3 +1,14 @@
+2018-01-17  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION (r226385?): Crash in com.apple.WebCore: WebCore::MediaQueryEvaluator::evaluate const + 32
+        https://bugs.webkit.org/show_bug.cgi?id=181742
+        <rdar://problem/36334726>
+
+        Reviewed by David Kilzer.
+
+        * fast/media/mediaqueryevaluator-crash-expected.txt: Added.
+        * fast/media/mediaqueryevaluator-crash.html: Added.
+
 2018-01-17  Matt Lewis  <jlewis3@apple.com>
 
         Marked inspector/worker/worker-recover-if-inspector-close.html as flaky on macOS
diff --git a/LayoutTests/fast/media/mediaqueryevaluator-crash-expected.txt b/LayoutTests/fast/media/mediaqueryevaluator-crash-expected.txt
new file mode 100644 (file)
index 0000000..7ef22e9
--- /dev/null
@@ -0,0 +1 @@
+PASS
diff --git a/LayoutTests/fast/media/mediaqueryevaluator-crash.html b/LayoutTests/fast/media/mediaqueryevaluator-crash.html
new file mode 100644 (file)
index 0000000..2aa8efd
--- /dev/null
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<iframe id="webtest0"></iframe>
+<script id="webtest1">
+if (window.testRunner)
+    testRunner.dumpAsText();
+framedoc = frames[0].document.documentElement;
+document.body.innerText = 'PASS';
+framedoc.innerHTML = '<title>';
+framedoc.getElementsByTagName('title')[0].innerText = 'a';
+</script>
index 909a5b0..03270ba 100644 (file)
@@ -1,3 +1,28 @@
+2018-01-17  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION (r226385?): Crash in com.apple.WebCore: WebCore::MediaQueryEvaluator::evaluate const + 32
+        https://bugs.webkit.org/show_bug.cgi?id=181742
+        <rdar://problem/36334726>
+
+        Reviewed by David Kilzer.
+
+        Test: fast/media/mediaqueryevaluator-crash.html
+
+        * css/MediaQueryEvaluator.cpp:
+        (WebCore::MediaQueryEvaluator::MediaQueryEvaluator):
+
+        Use WeakPtr<Document> instead of a plain Frame pointer.
+
+        (WebCore::MediaQueryEvaluator::evaluate const):
+
+        Get the frame via document.
+
+        * css/MediaQueryEvaluator.h:
+        * dom/Document.cpp:
+        (WebCore::Document::prepareForDestruction):
+
+        Take care to clear style resolver.
+
 2018-01-17  Youenn Fablet  <youenn@apple.com>
 
         Put fetch request keepAlive behind a runtime flag
index 6fd024a..c771945 100644 (file)
@@ -109,7 +109,7 @@ MediaQueryEvaluator::MediaQueryEvaluator(const String& acceptedMediaType, bool m
 
 MediaQueryEvaluator::MediaQueryEvaluator(const String& acceptedMediaType, const Document& document, const RenderStyle* style)
     : m_mediaType(acceptedMediaType)
-    , m_frame(document.frame())
+    , m_document(const_cast<Document&>(document).createWeakPtr())
     , m_style(style)
 {
 }
@@ -137,7 +137,7 @@ static bool applyRestrictor(MediaQuery::Restrictor r, bool value)
 
 bool MediaQueryEvaluator::evaluate(const MediaQuerySet& querySet, StyleResolver* styleResolver) const
 {
-    LOG_WITH_STREAM(MediaQueries, stream << "MediaQueryEvaluator::evaluate on " << (m_frame && m_frame->document() ? m_frame->document()->url().string() : emptyString()));
+    LOG_WITH_STREAM(MediaQueries, stream << "MediaQueryEvaluator::evaluate on " << (m_document ? m_document->url().string() : emptyString()));
 
     auto& queries = querySet.queryVector();
     if (!queries.size()) {
@@ -769,7 +769,12 @@ static void add(MediaQueryFunctionMap& map, AtomicStringImpl* key, MediaQueryFun
 
 bool MediaQueryEvaluator::evaluate(const MediaQueryExpression& expression) const
 {
-    if (!m_frame || !m_frame->view() || !m_style)
+    if (!m_document)
+        return m_fallbackResult;
+
+    Document& document = *m_document;
+    auto* frame = document.frame();
+    if (!frame || !frame->view() || !m_style)
         return m_fallbackResult;
 
     if (!expression.isValid())
@@ -787,10 +792,9 @@ bool MediaQueryEvaluator::evaluate(const MediaQueryExpression& expression) const
     if (!function)
         return false;
 
-    Document& document = *m_frame->document();
     if (!document.documentElement())
         return false;
-    return function(expression.value(), { m_style, document.documentElement()->renderStyle(), document.renderView(), 1, false }, *m_frame, NoPrefix);
+    return function(expression.value(), { m_style, document.documentElement()->renderStyle(), document.renderView(), 1, false }, *frame, NoPrefix);
 }
 
 bool MediaQueryEvaluator::mediaAttributeMatches(Document& document, const String& attributeValue)
index 7ce2561..8be2f0e 100644 (file)
@@ -28,6 +28,7 @@
 #pragma once
 
 #include "MediaQueryExpression.h"
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
 
@@ -74,7 +75,7 @@ public:
 
 private:
     String m_mediaType;
-    Frame* m_frame { nullptr }; // not owned
+    WeakPtr<Document> m_document;
     const RenderStyle* m_style { nullptr };
     bool m_fallbackResult { false };
 };
index 1b6ee21..fe88676 100644 (file)
@@ -2361,6 +2361,8 @@ void Document::prepareForDestruction()
     if (m_domWindow && m_frame)
         m_domWindow->willDetachDocumentFromFrame();
 
+    styleScope().clearResolver();
+
     if (hasLivingRenderTree())
         destroyRenderTree();