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

[bugzilla-daemon at webkit.org]

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


Darin Fisher (:fishd, Google) <fishd at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #56112|review?                     |review-
               Flag|                            |




--- Comment #2 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2010-05-14 16:30:33 PST ---
(From update of attachment 56112)
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.

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.

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.

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.