[webkit-dev] Can node->renderer()->node() == node ever be false?

Dimitri Glazkov dglazkov at chromium.org
Tue Sep 14 21:16:45 PDT 2010


Ok, after following down a few rabbit holes, I can with some degree of
certainty say that node->renderer()->node() == 0 can only at the end
of RenderWidget::destroy(), but that is clearly not the case we're
fishing for.

Even if we modify the loop to start at the parent, we're jumping into
the render tree to find a node to fire an event on, which is unhealthy
for various layering reasons.

In summary, to quote the great protagonists from a TV show:
"Exterminate! Exterminate!"

:DG<

On Tue, Sep 14, 2010 at 7:19 PM, Dimitri Glazkov <dglazkov at chromium.org> wrote:
> Sounds good, I will look into this. FWIW, when I remove this loop, I
> see no regressions in layout tests, including the tests added with the
> same commit as the loop.
>
> :DG<
>
> On Tue, Sep 14, 2010 at 5:55 PM, Maciej Stachowiak <mjs at apple.com> wrote:
>>
>> On Sep 14, 2010, at 5:41 PM, Dimitri Glazkov wrote:
>>
>>> Sorry, I meant node->renderer()->node() != 0. My bad. This loop will
>>> always exit in in the first iteration.
>>
>> It definitely is possible for renderers to have a null result from node(). I do not know for sure that it's impossible for node->renderer()->node() to be null under any circumstances. Anonymous renderers and inline continuations are among the ways a null node pointers. It might be that in all such circumstances, the renderer won't be returned by any node's renderer() method. It would be worth some analysis.
>>
>>  - Maciej
>>
>>>
>>> :DG<
>>>
>>> On Tue, Sep 14, 2010 at 4:45 PM, Adam Roben <aroben at apple.com> wrote:
>>>> On Sep 14, 2010, at 6:46 PM, Dimitri Glazkov wrote:
>>>>
>>>>> I've been looking at this line here and it doesn't seem to make sense
>>>>> to me: http://trac.webkit.org/browser/trunk/WebCore/page/EventHandler.cpp#L2153
>>>>>
>>>>> It looks like the loop in question will always exit early, because it
>>>>> short-circuits to node->renderer()->node() == node, which seems like
>>>>> it always will be true. At least, that's what the layout tests say
>>>>> when I remove it.
>>>>
>>>> I don't see anything in that loop that is equivalent to node->renderer()->node() == node. All I see are null-checks. Note that line 2154 declares a new variable with the name "node".
>>>>
>>>> I don't know anything else about this code or what you're asking, though.
>>>>
>>>> -Adam
>>>>
>>>>
>>> _______________________________________________
>>> webkit-dev mailing list
>>> webkit-dev at lists.webkit.org
>>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>
>>
>


More information about the webkit-dev mailing list