[Webkit-unassigned] [Bug 39138] [chromium] Fix LayerChromium hierarchy update methods

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Vangelis Kokkevis <vangelis at chromium.org>  2010-05-14 16:54:29 PST ---
(In reply to comment #2)
> (From update of attachment 56112 [details])
> WebCore/ChangeLog:5
>  +  
> comment #0 from the bug report contains a nice description of the
> issues this change is fixing.  it'd be good to include that info
> here.
> 
Done.

> WebCore/platform/graphics/chromium/LayerChromium.cpp:81
>  +      removeAllSublayers();
> It looks like the point here is to trigger the call to notifySyncRequired?
> The clearing of m_sublayers does not seem to be critical since that will
> happen anyways because we are inside ~LayerChromium.
> 
Good point.  I don't actually need to call notifySyncRequired since to get to the destructor this layer must have already been removed from a parent that would have called the method.

> WebCore/platform/graphics/chromium/LayerChromium.cpp:175
>  +      if (referenceIndex == -1)
> nit: it's good to use ASSERT_NOT_REACHED so you don't have to
> repeat the expression in cases like this.

Done.

> 
> LGTM otherwise.  The ownership changes make sense.  R- for these nits.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.