⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 100046 in webkit


Ignore:
Timestamp:
Nov 11, 2011, 4:34:57 PM (15 years ago)
Author:
commit-queue@webkit.org
Message:

Null deref when no use element exists for SVG element instance
​https://bugs.webkit.org/show_bug.cgi?id=59136

Second attempt, with a fix to handle JS garbage collection.

Patch by Stephen Chenney <​schenney@chromium.org> on 2011-11-11
Reviewed by Nikolas Zimmermann.

Source/WebCore:

Test: svg/custom/element-instance-held-by-js-crash.svg

  • svg/SVGElementInstance.cpp:

(WebCore::SVGElementInstance::~SVGElementInstance): Added call to detach() to clear

anything not yet cleared.

(WebCore::SVGElementInstance::detach): New method to replace old clear methods. This one

clears all the pointers it can, and removes the instance from the corresponding elements
instance list.

  • svg/SVGElementInstance.h: Removed clear methods and replaced with detach.
  • svg/SVGUseElement.cpp:

(WebCore::SVGUseElement::detachInstance): Modified calls to clean up an SVGElementInstance.

LayoutTests:

  • svg/custom/element-instance-held-by-js-crash-expected.txt: Added.
  • svg/custom/element-instance-held-by-js-crash.svg: Added.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r100045 r100046  
     12011-11-11  Stephen Chenney  <schenney@chromium.org>
     2
     3        Null deref when no use element exists for SVG element instance
     4        https://bugs.webkit.org/show_bug.cgi?id=59136
     5
     6        Second attempt, with a fix to handle JS garbage collection.
     7
     8        Reviewed by Nikolas Zimmermann.
     9
     10        * svg/custom/element-instance-held-by-js-crash-expected.txt: Added.
     11        * svg/custom/element-instance-held-by-js-crash.svg: Added.
     12
    1132011-11-11  Florin Malita  <fmalita@google.com>
    214

  • trunk/Source/WebCore/ChangeLog

    r100045 r100046  
     12011-11-11  Stephen Chenney  <schenney@chromium.org>
     2
     3        Null deref when no use element exists for SVG element instance
     4        https://bugs.webkit.org/show_bug.cgi?id=59136
     5
     6        Second attempt, with a fix to handle JS garbage collection.
     7
     8        Reviewed by Nikolas Zimmermann.
     9
     10        Test: svg/custom/element-instance-held-by-js-crash.svg
     11
     12        * svg/SVGElementInstance.cpp:
     13        (WebCore::SVGElementInstance::~SVGElementInstance): Added call to detach() to clear
     14          anything not yet cleared.
     15        (WebCore::SVGElementInstance::detach): New method to replace old clear methods. This one
     16          clears all the pointers it can, and removes the instance from the corresponding elements
     17          instance list.
     18        * svg/SVGElementInstance.h: Removed clear methods and replaced with detach.
     19        * svg/SVGUseElement.cpp:
     20        (WebCore::SVGUseElement::detachInstance): Modified calls to clean up an SVGElementInstance.
     21
    1222011-11-11  Florin Malita  <fmalita@google.com>
    223

  • trunk/Source/WebCore/svg/SVGElementInstance.cpp

    r99861 r100046  
    6262SVGElementInstance::~SVGElementInstance()
    6363{
     64    // Call detach because we may be deleted directly if we are a child of a detached instance.
     65    detach();
     66
    6467#ifndef NDEBUG
    6568    instanceCounter.decrement();
    6669#endif
    6770
    68     // Deregister as instance for passed element.
    69     m_element->removeInstanceMapping(this);
    70 
    71     clearChildren();
     71    m_element = 0;
    7272}
    7373
    74 void SVGElementInstance::clearChildren()
     74void SVGElementInstance::detach()
    7575{
     76    // Clear all pointers. When the node is detached from the shadow DOM it should be removed but,
     77    // due to ref counting, it may not be. So clear everything to avoid dangling pointers.
     78
     79    // Deregister as instance for passed element, if we haven't already.
     80    if (m_element->instancesForElement().contains(this))
     81        m_element->removeInstanceMapping(this);
     82    // DO NOT clear ref to m_element because JavaScriptCore uses it for garbage collection
     83
     84    m_shadowTreeElement = 0;
     85
     86    m_directUseElement = 0;
     87    m_correspondingUseElement = 0;
     88
    7689    removeAllChildrenInContainer<SVGElementInstance, SVGElementInstance>(this);
    7790}

  • trunk/Source/WebCore/svg/SVGElementInstance.h

    r99861 r100046  
    6161    SVGUseElement* directUseElement() const { return m_directUseElement; }
    6262    SVGElement* shadowTreeElement() const { return m_shadowTreeElement.get(); }
    63     void clearChildren();
    64     void clearUseElements()
    65     {
    66         m_directUseElement = 0;
    67         m_correspondingUseElement = 0;
    68     }
     63
     64    void detach();
    6965
    7066    SVGElementInstance* parentNode() const { return parent(); }

  • trunk/Source/WebCore/svg/SVGUseElement.cpp

    r99861 r100046  
    625625    if (!m_targetElementInstance)
    626626        return;
    627     m_targetElementInstance->clearUseElements();
    628     m_targetElementInstance->clearChildren();
     627    m_targetElementInstance->detach();
    629628    m_targetElementInstance = 0;
    630629}
Note: See TracChangeset for help on using the changeset viewer.