Bug 74858 - chrome.dll!WebCore::SVGTRefElement::updateReferencedText ReadAV@NULL (e85cb8e140071fa7790cad215b0109dc)
Summary: chrome.dll!WebCore::SVGTRefElement::updateReferencedText ReadAV@NULL (e85cb8e...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows Vista
: P1 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-19 05:48 PST by Berend-Jan Wever
Modified: 2012-02-17 08:50 PST (History)
5 users (show)

See Also:

Attachments
Patch (10.13 KB, patch)
2012-01-09 15:02 PST, Florin Malita 		no flags Details | Formatted Diff | Diff
Reduced test. (326 bytes, image/svg+xml)
2012-01-11 07:46 PST, Florin Malita 		no flags Details
Patch (10.13 KB, patch)
2012-02-15 13:36 PST, Florin Malita 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Berend-Jan Wever 2011-12-19 05:48:46 PST 
Chromium: http://code.google.com/p/chromium/issues/detail?id=108057

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
  <text id="text"></text>
  <tref xlink:href="#text"/>
  <script>
    document.documentElement.replaceChild(
        document.createElement('anything'), // inserted element
        document.getElementById('text'));   // removed element
  </script>
</svg>

stack:          chrome.dll!WebCore::SVGTRefElement::updateReferencedText
                chrome.dll!WebCore::SubtreeModificationEventListener::handleEvent
                chrome.dll!WebCore::EventTarget::fireEventListeners
                chrome.dll!WebCore::EventTarget::fireEventListeners
                chrome.dll!WebCore::Node::handleLocalEvents
                chrome.dll!WebCore::EventDispatcher::dispatchEvent
                chrome.dll!WebCore::EventDispatchMediator::dispatchEvent
                chrome.dll!WebCore::EventDispatcher::dispatchEvent
                chrome.dll!WebCore::ScopedEventQueue::dispatchEvent
                chrome.dll!WebCore::ScopedEventQueue::enqueueEventDispatchMediator
                chrome.dll!WebCore::EventDispatcher::dispatchScopedEvent
                chrome.dll!WebCore::Node::dispatchScopedEvent
                chrome.dll!WebCore::Node::dispatchSubtreeModifiedEvent
                chrome.dll!WebCore::ContainerNode::replaceChild
                chrome.dll!WebCore::Node::replaceChild
                chrome.dll!WebCore::V8Node::replaceChildCallback
                chrome.dll!v8::internal::HandleApiCallHelper<...>
                chrome.dll!v8::internal::Builtin_HandleApiCall
                chrome.dll!v8::internal::Invoke
                chrome.dll!v8::internal::Execution::Call
                ...
Comment 1 Florin Malita 2012-01-09 14:59:08 PST 
The DOMSubtreeModifiedEvent listener is still active on the target's parent after the target is removed.

We need to catch the target removal event and deactivate any associated listeners. I have a first-pass at this following up.

Additionally:

* on target removal, mark the tref as resource-pending again, to reattach if the same id is added at a later time (I believe this is desirable)
* move the DOMSubtreeModifiedEvent to the target element itself to keep things simple (I think we're only interested in catching updates to the target element, and the event appears to be dispatched to it too - hope I'm not missing something here)
Comment 2 Florin Malita 2012-01-09 15:02:43 PST 
Created attachment 121727 [details]
Patch
Comment 3 Florin Malita 2012-01-11 07:46:29 PST 
Created attachment 122019 [details]
Reduced test.
Comment 4 Florin Malita 2012-02-15 13:36:43 PST 
Created attachment 127225 [details]
Patch

Updated after the recent shadow tree changes.
Comment 5 Nikolas Zimmermann 2012-02-15 15:19:08 PST 
Comment on attachment 127225 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=127225&action=review

Looks just great! r=me.

> Source/WebCore/svg/SVGTRefElement.cpp:185
> +    Node* container = shadowRootList()->oldestShadowRoot()->firstChild();

oldestShadowRoot, and shadowRootList are guaranteed non-null?
If yes, there's nothing hold back a cq+.
Comment 6 Florin Malita 2012-02-16 07:30:11 PST 
(In reply to comment #5)
> > Source/WebCore/svg/SVGTRefElement.cpp:185
> > +    Node* container = shadowRootList()->oldestShadowRoot()->firstChild();
> 
> oldestShadowRoot, and shadowRootList are guaranteed non-null?

Yes, SVGTRefElement::create() calls createShadowSubtree() which initializes the shadow root plumbing.


> If yes, there's nothing hold back a cq+.

Thanks!
Comment 7 Nikolas Zimmermann 2012-02-17 07:36:26 PST 
Comment on attachment 127225 [details]
Patch

r=me.
Comment 8 WebKit Review Bot 2012-02-17 08:50:30 PST 
Comment on attachment 127225 [details]
Patch

Clearing flags on attachment: 127225

Committed r108082: <http://trac.webkit.org/changeset/108082>
Comment 9 WebKit Review Bot 2012-02-17 08:50:36 PST 
All reviewed patches have been landed.  Closing bug.