[Webkit-unassigned] [Bug 39138] [chromium] Fix LayerChromium hierarchy update methods
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri May 14 16:54:30 PDT 2010
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.
More information about the webkit-unassigned
mailing list