Bug 83539 - Web Inspector: ASSERT attempting to unbind null contentDocument
Summary: Web Inspector: ASSERT attempting to unbind null contentDocument
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-09 19:11 PDT by Joseph Pecoraro
Modified: 2012-04-09 21:42 PDT (History)
11 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (1.63 KB, patch)
2012-04-09 19:16 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2012-04-09 19:11:46 PDT
ASSERT:

    ASSERTION FAILED: !HashTranslator::equal(KeyTraits::emptyValue(), key)
    1   0x3b3c89b void WTF::HashTable<WTF::RefPtr<WebCore::Node>, std::__1::pair<WTF::RefPtr<WebCore::Node>, int>, WTF::PairFirstExtractor<std::__1::pair<WTF::RefPtr<WebCore::Node>, int> >, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::PairHashTraits<WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> > >::checkKey<WTF::HashMapTranslator<WTF::PairHashTraits<WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >, WTF::PtrHash<WTF::RefPtr<WebCore::Node> > >, WebCore::Node*>(WebCore::Node* const&)
    2   0x3b3c387 std::__1::pair<WTF::RefPtr<WebCore::Node>, int>* WTF::HashTable<WTF::RefPtr<WebCore::Node>, std::__1::pair<WTF::RefPtr<WebCore::Node>, int>, WTF::PairFirstExtractor<std::__1::pair<WTF::RefPtr<WebCore::Node>, int> >, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::PairHashTraits<WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> > >::lookup<WTF::HashMapTranslator<WTF::PairHashTraits<WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >, WTF::PtrHash<WTF::RefPtr<WebCore::Node> > >, WebCore::Node*>(WebCore::Node* const&)
    3   0x3b413c4 WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >::inlineGet(WebCore::Node*) const
    4   0x3b2e6b4 WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >::get(WebCore::Node*) const
    5   0x3b231c1 WebCore::InspectorDOMAgent::unbind(WebCore::Node*, WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >*)
    6   0x3b23268 WebCore::InspectorDOMAgent::unbind(WebCore::Node*, WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >*)
    7   0x3b2337c WebCore::InspectorDOMAgent::unbind(WebCore::Node*, WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >*)
    8   0x3b2337c WebCore::InspectorDOMAgent::unbind(WebCore::Node*, WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >*)
    9   0x3b2337c WebCore::InspectorDOMAgent::unbind(WebCore::Node*, WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >*)
    10  0x3b23268 WebCore::InspectorDOMAgent::unbind(WebCore::Node*, WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >*)
    11  0x3b2337c WebCore::InspectorDOMAgent::unbind(WebCore::Node*, WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >*)
    12  0x3b2337c WebCore::InspectorDOMAgent::unbind(WebCore::Node*, WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >*)
    13  0x3b2337c WebCore::InspectorDOMAgent::unbind(WebCore::Node*, WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >*)
    14  0x3b2337c WebCore::InspectorDOMAgent::unbind(WebCore::Node*, WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >*)
    15  0x3b2337c WebCore::InspectorDOMAgent::unbind(WebCore::Node*, WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >*)
    16  0x3b2337c WebCore::InspectorDOMAgent::unbind(WebCore::Node*, WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >*)
    17  0x3b2337c WebCore::InspectorDOMAgent::unbind(WebCore::Node*, WTF::HashMap<WTF::RefPtr<WebCore::Node>, int, WTF::PtrHash<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<WTF::RefPtr<WebCore::Node> >, WTF::HashTraits<int> >*)
    18  0x3b2cc6f WebCore::InspectorDOMAgent::didRemoveDOMNode(WebCore::Node*)
    19  0x3b68a7c WebCore::InspectorInstrumentation::didRemoveDOMNodeImpl(WebCore::InstrumentingAgents*, WebCore::Node*)
    20  0x3252d73 WebCore::InspectorInstrumentation::willRemoveDOMNode(WebCore::Document*, WebCore::Node*)
    21  0x324f9f9 WebCore::dispatchChildRemovalEvents(WebCore::Node*)
    22  0x324cde1 WebCore::willRemoveChild(WebCore::Node*)
    23  0x324a3ab WebCore::ContainerNode::removeChild(WebCore::Node*, int&)
    24  0x4407db1 WebCore::Node::removeChild(WebCore::Node*, int&)
    25  0x35cfda2 WebCore::DOMEditor::RemoveChildAction::redo(int&)
    26  0x35cfc52 WebCore::DOMEditor::RemoveChildAction::perform(int&)
    27  0x3b66d1c WebCore::InspectorHistory::perform(WTF::PassOwnPtr<WebCore::InspectorHistory::Action>, int&)
    28  0x35cb24c WebCore::DOMEditor::removeChild(WebCore::Node*, WebCore::Node*, int&)
    29  0x35cc087 WebCore::DOMEditor::removeChild(WebCore::Node*, WebCore::Node*, WTF::String*)
    30  0x3b26bb2 WebCore::InspectorDOMAgent::removeNode(WTF::String*, int)
    31  0x3b26c26 non-virtual thunk to WebCore::InspectorDOMAgent::removeNode(WTF::String*, int)

InspectorDOMAgent::unbind:

    ...
    if (node->isFrameOwnerElement()) {
        const HTMLFrameOwnerElement* frameOwner = static_cast<const HTMLFrameOwnerElement*>(node);
        if (m_domListener)
            m_domListener->didRemoveDocument(frameOwner->contentDocument());
        unbind(frameOwner->contentDocument(), nodesMap);
    }
    ...

I was unable to reproduce this, but its clear that contentDocument can be null so when we're trying
to unbind it we should avoid this if its null. Note that the bind side does null check:

   ...
   Document* doc = frameOwner->contentDocument();
   if (doc)
       value->setContentDocument(buildObjectForNode(doc, 0, nodesMap));
   ...
Comment 1 Joseph Pecoraro 2012-04-09 19:12:38 PDT
<rdar://problem/11215264>
Comment 2 Joseph Pecoraro 2012-04-09 19:16:03 PDT
Created attachment 136374 [details]
[PATCH] Proposed Fix

I was unable to reproduce this so I couldn't create a test case. =(

In the fix we still call didRemoveDocument because that handles null
and does other work.
Comment 3 Timothy Hatcher 2012-04-09 19:41:35 PDT
Comment on attachment 136374 [details]
[PATCH] Proposed Fix

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

> Source/WebCore/inspector/InspectorDOMAgent.cpp:338
> +            m_domListener->didRemoveDocument(contentDocument);

Is it safe to pass a null contenDocument to didRemoveDocument?
Comment 4 Joseph Pecoraro 2012-04-09 19:56:04 PDT
(In reply to comment #3)
> (From update of attachment 136374 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=136374&action=review
> 
> > Source/WebCore/inspector/InspectorDOMAgent.cpp:338
> > +            m_domListener->didRemoveDocument(contentDocument);
> 
> Is it safe to pass a null contenDocument to didRemoveDocument?

Yep, and I think its preferred since that does other work we might want:

    void InspectorCSSAgent::didRemoveDocument(Document* document)
    {
        if (document)
            m_documentToInspectorStyleSheet.remove(document);
        clearPseudoState(false);
    }
Comment 5 WebKit Review Bot 2012-04-09 21:42:35 PDT
Comment on attachment 136374 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 136374

Committed r113675: <http://trac.webkit.org/changeset/113675>
Comment 6 WebKit Review Bot 2012-04-09 21:42:39 PDT
All reviewed patches have been landed.  Closing bug.