[Webkit-unassigned] [Bug 146538] Memory leak for a protected element having pending events in ImageLoader

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=146538

--- Comment #5 from Brady Eidson <beidson at apple.com> ---
(In reply to comment #3)
> (In reply to comment #2)
> > Comment on attachment 255984 [details]
> > Use RefPtr not calling ref/deref() directly
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=255984&action=review
> > 
> > > Source/WebCore/loader/ImageLoader.cpp:363
> > > +            m_protectedElement= &element();
> > 
> > Type: Missing a space before the =.
> > 
> > (Really bummed that check-webkit-style didn't get this)
> > 
> > > Source/WebCore/loader/ImageLoader.cpp:372
> > > +    m_protectedElement.clear();
> > 
> > This should be:
> > m_protectedElement = nullptr;
> 
> 
> m_protectedElement is RefPtr.
> clear() will clear the internal pointer to nullptr.

I know.

Assigning a nullptr to a RefPtr is functionally the same as calling clear() on it.

But when somebody not familiar with the code is reading it, and they come across a call to ".clear()", they don't necessarily know you're null'ing out a RefPtr. They might think you're calling "clear()" on some object that has a whole bunch of behavior inside of its ::clear() method.

But by assigning nullptr, it's obvious to anybody reading the code "This is a pointer - it might be a raw pointer or one of our various types of smart pointers, but I know it's a pointer and this code is null'ing it out"

This request is about readability by people not familiar with the code.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150702/2d7386f9/attachment.html>