Attachment 356577 Details for Bug 192392 – Fixes the bug

[patch] Fixes the bug

bug-192392-20181204210100.patch (text/plain), 4.53 KB, created by Ryosuke Niwa on 2018-12-04 21:01:01 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-12-04 21:01:01 PST

Size: 4.53 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 238885)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,21 @@
>+2018-12-05  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Null pointer crash in DocumentOrderedMap::getElementById via FormAssociatedElement::findAssociatedForm
>+        https://bugs.webkit.org/show_bug.cgi?id=192392
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The crash was caused by FormAssociatedElement::findAssociatedForm invoking DocumentOrderedMap::getElementById
>+        and de-referencing nullptr Attribute* via IdTargetObserver before Element::attributeChanged had updated
>+        ElementData::m_idForStyleResolution.
>+
>+        Fixed it by updating m_idForStyleResolution before invoking IdTargetObservers.
>+
>+        Test: fast/dom/remove-id-form-associated-elemet-id-observer-crash.html
>+
>+        * dom/Element.cpp:
>+        (WebCore::Element::attributeChanged): Fixed the bug.
>+
> 2018-12-04  Frederic Wang  <fwang@igalia.com>
> 
>         Always pass scrollingGeometry to update*ScrollingNode functions
>Index: Source/WebCore/dom/Element.cpp
>===================================================================
>--- Source/WebCore/dom/Element.cpp	(revision 238880)
>+++ Source/WebCore/dom/Element.cpp	(working copy)
>@@ -1500,17 +1500,17 @@ void Element::attributeChanged(const Qua
> 
>     if (!valueIsSameAsBefore) {
>         if (name == HTMLNames::idAttr) {
>-            if (!oldValue.isEmpty())
>-                treeScope().idTargetObserverRegistry().notifyObservers(*oldValue.impl());
>-            if (!newValue.isEmpty())
>-                treeScope().idTargetObserverRegistry().notifyObservers(*newValue.impl());
>-
>             AtomicString oldId = elementData()->idForStyleResolution();
>             AtomicString newId = makeIdForStyleResolution(newValue, document().inQuirksMode());
>             if (newId != oldId) {
>                 Style::IdChangeInvalidation styleInvalidation(*this, oldId, newId);
>                 elementData()->setIdForStyleResolution(newId);
>             }
>+
>+            if (!oldValue.isEmpty())
>+                treeScope().idTargetObserverRegistry().notifyObservers(*oldValue.impl());
>+            if (!newValue.isEmpty())
>+                treeScope().idTargetObserverRegistry().notifyObservers(*newValue.impl());
>         } else if (name == classAttr)
>             classAttributeChanged(newValue);
>         else if (name == HTMLNames::nameAttr)
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 238880)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2018-12-05  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Null pointer crash in DocumentOrderedMap::getElementById via FormAssociatedElement::findAssociatedForm
>+        https://bugs.webkit.org/show_bug.cgi?id=192392
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added a regression test.
>+
>+        * fast/dom/remove-id-form-associated-elemet-id-observer-crash-expected.txt: Added.
>+        * fast/dom/remove-id-form-associated-elemet-id-observer-crash.html: Added.
>+
> 2018-12-04  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         Crash in HTMLCollection::updateNamedElementCache
>Index: LayoutTests/fast/dom/remove-id-form-associated-elemet-id-observer-crash-expected.txt
>===================================================================
>--- LayoutTests/fast/dom/remove-id-form-associated-elemet-id-observer-crash-expected.txt	(nonexistent)
>+++ LayoutTests/fast/dom/remove-id-form-associated-elemet-id-observer-crash-expected.txt	(working copy)
>@@ -0,0 +1,3 @@
>+This tests removing the id from an element when there is a form associated element observing the same ID. WebKit should not crash.
>+
>+PASS
>Index: LayoutTests/fast/dom/remove-id-form-associated-elemet-id-observer-crash.html
>===================================================================
>--- LayoutTests/fast/dom/remove-id-form-associated-elemet-id-observer-crash.html	(nonexistent)
>+++ LayoutTests/fast/dom/remove-id-form-associated-elemet-id-observer-crash.html	(working copy)
>@@ -0,0 +1,19 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<p>This tests removing the id from an element when there is a form associated element observing the same ID.
>+WebKit should not crash.</p>
>+<div id="container">
>+<p id="foo"></p>
>+<form id="foo"></form>
>+<fieldset form="foo"></fieldset>
>+</div>
>+<script>
>+if (window.testRunner)
>+    testRunner.dumpAsText();
>+document.querySelector('#container p').removeAttribute('id');
>+container.remove();
>+document.write('PASS');
>+</script>
>+</body>
>+</html>

Flags:

dino: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 192392: 356577 | 356580