[webkit-reviews] review denied: [Bug 78475] FrameTree.h should be invisible from Frame.h : [Attachment 126752] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Feb 13 14:02:46 PST 2012


Eric Seidel <eric at webkit.org> has denied  review:
Bug 78475: FrameTree.h should be invisible from Frame.h
https://bugs.webkit.org/show_bug.cgi?id=78475

Attachment 126752: Patch
https://bugs.webkit.org/attachment.cgi?id=126752&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
My appologies.

I was aware of all that you list, with the exception of the Frame::parent
design choice at time of review.

I trust that Morita would be able to see the build failures himself and fix
them accordingly.

I share your concern of the possibility of performance issues, but I put a lot
of faith in our perf bots to catch possible regressions.   It's difficult for
me to understand how this could have much perf impact, although I believe you
that others have shown it to in the past.  We don't allocate that many Frame
objects, and I have a difficutl time imagining that the pointer indirection to
the FrameTree could be that hot.  Do you have a specific test which you'd like
to see this shown as clean on?	We have PLT bots
(http://build.chromium.org/f/chromium/perf/dashboard/overview.html) as well as
http://webkit-perf.appspot.com/ which process every change, and should catch
any regression.

Based on Darin's comments, it sounds like he would like to see the EWS bots
pass before any further review.  I can't comment on the debatability of the
parent accessor on Frame, but we could do some digging to find the old
FrameTree bugs and read.


More information about the webkit-reviews mailing list