Attachment 341392 Details for Bug 186011 – Fixes the bug

[patch] Fixes the bug

bug-186011-20180526015539.patch (text/plain), 5.41 KB, created by Ryosuke Niwa on 2018-05-26 01:55:39 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-05-26 01:55:39 PDT

Size: 5.41 KB

patch

obsolete

>Subversion Revision: 232103
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 21a24ef87cef3b3db8d4a5d90352cb461742ba34..1163aa09a556cffaa812383392702e398d23fdf8 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,26 @@
>+2018-05-26  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        iOS WK1: Occasional crash in sanitizedMarkupForFragmentInDocument
>+        https://bugs.webkit.org/show_bug.cgi?id=186011
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The crash was caused by the HTML parser in sanitizedMarkupForFragmentInDocument yielding in the web thread
>+        when _WebThreadLock() sets webThreadShouldYield to true in the main thread.
>+
>+        No new tests. This is occasionally caught by existing tests.
>+
>+        * editing/markup.cpp:
>+        (WebCore::createPageForSanitizingWebContent): Fixed the bug by making the HTML parser never yield.
>+        Also release-assert that the body is never null here.
>+        (WebCore::sanitizedMarkupForFragmentInDocument): Removed superflous call to WTFMove since appendChild
>+        takes a reference, not a Ref.
>+        * inspector/InspectorOverlay.cpp:
>+        (WebCore::InspectorOverlay::overlayPage): Deployed the same fix.
>+        * loader/DocumentWriter.cpp:
>+        (WebCore::DocumentWriter::insertDataSynchronously): Added.
>+        * loader/DocumentWriter.h:
>+
> 2018-05-22  Myles C. Maxfield  <mmaxfield@apple.com>
> 
>         Text can wrap between hyphens and commas
>diff --git a/Source/WebCore/editing/markup.cpp b/Source/WebCore/editing/markup.cpp
>index 5551eecf9223d369a0af74d24947437e0bebbf73..315fb15e4329a53441ae3da4a4004ccf3d78ead2 100644
>--- a/Source/WebCore/editing/markup.cpp
>+++ b/Source/WebCore/editing/markup.cpp
>@@ -189,10 +189,12 @@ std::unique_ptr<Page> createPageForSanitizingWebContent()
>     FrameLoader& loader = frame.loader();
>     static char markup[] = "<!DOCTYPE html><html><body></body></html>";
>     ASSERT(loader.activeDocumentLoader());
>-    loader.activeDocumentLoader()->writer().setMIMEType("text/html");
>-    loader.activeDocumentLoader()->writer().begin();
>-    loader.activeDocumentLoader()->writer().addData(markup, sizeof(markup));
>-    loader.activeDocumentLoader()->writer().end();
>+    auto& writer = loader.activeDocumentLoader()->writer();
>+    writer.setMIMEType("text/html");
>+    writer.begin();
>+    writer.insertDataSynchronously(String(markup));
>+    writer.end();
>+    RELEASE_ASSERT(page->mainFrame().document()->body());
> 
>     return page;
> }
>@@ -859,7 +861,7 @@ String sanitizedMarkupForFragmentInDocument(Ref<DocumentFragment>&& fragment, Do
> 
>     auto bodyElement = makeRefPtr(document.body());
>     ASSERT(bodyElement);
>-    bodyElement->appendChild(WTFMove(fragment));
>+    bodyElement->appendChild(fragment.get());
> 
>     auto range = Range::create(document);
>     range->selectNodeContents(*bodyElement);
>diff --git a/Source/WebCore/inspector/InspectorOverlay.cpp b/Source/WebCore/inspector/InspectorOverlay.cpp
>index e5c890214c9c49035d01331463b6df1d003eba91..6e70a1098dcb0c2a9ecf30a7be9f6219d42826c7 100644
>--- a/Source/WebCore/inspector/InspectorOverlay.cpp
>+++ b/Source/WebCore/inspector/InspectorOverlay.cpp
>@@ -729,10 +729,11 @@ Page* InspectorOverlay::overlayPage()
>     frame.view()->setCanHaveScrollbars(false);
>     frame.view()->setTransparent(true);
>     ASSERT(loader.activeDocumentLoader());
>-    loader.activeDocumentLoader()->writer().setMIMEType("text/html");
>-    loader.activeDocumentLoader()->writer().begin();
>-    loader.activeDocumentLoader()->writer().addData(reinterpret_cast<const char*>(InspectorOverlayPage_html), sizeof(InspectorOverlayPage_html));
>-    loader.activeDocumentLoader()->writer().end();
>+    auto& writer = loader.activeDocumentLoader()->writer();
>+    writer.setMIMEType("text/html");
>+    writer.begin();
>+    writer.insertDataSynchronously(String(reinterpret_cast<const char*>(InspectorOverlayPage_html), sizeof(InspectorOverlayPage_html)));
>+    writer.end();
> 
> #if OS(WINDOWS)
>     evaluateInOverlay("setPlatform", "windows");
>diff --git a/Source/WebCore/loader/DocumentWriter.cpp b/Source/WebCore/loader/DocumentWriter.cpp
>index d9437a01699a1a6ba0aed80cc0a2e6dd8de13f17..25451ec00d98c222581fe85b89bf92b57c62518b 100644
>--- a/Source/WebCore/loader/DocumentWriter.cpp
>+++ b/Source/WebCore/loader/DocumentWriter.cpp
>@@ -253,6 +253,14 @@ void DocumentWriter::addData(const char* bytes, size_t length)
>     m_parser->appendBytes(*this, bytes, length);
> }
> 
>+void DocumentWriter::insertDataSynchronously(const String& markup)
>+{
>+    ASSERT(m_state != NotStartedWritingState);
>+    ASSERT(m_state != FinishedWritingState);
>+    ASSERT(m_parser);
>+    m_parser->insert(markup);
>+}
>+
> void DocumentWriter::end()
> {
>     ASSERT(m_frame->page());
>diff --git a/Source/WebCore/loader/DocumentWriter.h b/Source/WebCore/loader/DocumentWriter.h
>index 9f03691a38d77a50bc0e429e159fc0af38f52ee0..385fb6407ba419bbb8667cbc50997c1b0d4bf214 100644
>--- a/Source/WebCore/loader/DocumentWriter.h
>+++ b/Source/WebCore/loader/DocumentWriter.h
>@@ -51,8 +51,9 @@ public:
>     bool begin();
>     bool begin(const URL&, bool dispatchWindowObjectAvailable = true, Document* ownerDocument = nullptr);
>     void addData(const char* bytes, size_t length);
>+    void insertDataSynchronously(const String&); // For an internal use only to prevent the parser from yielding.
>     WEBCORE_EXPORT void end();
>-    
>+
>     void setFrame(Frame* frame) { m_frame = frame; }
> 
>     WEBCORE_EXPORT void setEncoding(const String& encoding, bool userChosen);

Actions: View | Formatted Diff | Diff

Attachments on bug 186011: 341392