| Summary: | REGRESSION (r27351): Departure date does not repaint when changed on Google Maps public transit planner | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | mitz | ||||
| Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | ddkilzer | ||||
| Priority: | P1 | Keywords: | InRadar, Regression | ||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Mac | ||||||
| OS: | OS X 10.4 | ||||||
| URL: | http://www.google.com/maps?ie=UTF8&f=d&dirflg=r&saddr=1390+market+st%2C+sf+ca&daddr=1585+Folsom+St%2C+San+Francisco%2C+CA&ttype=dep&date=11%2F17&time=5%3A30pm | ||||||
| Attachments: |
|
||||||
|
Description
mitz
2007-11-17 17:17:09 PST
Regressed in r27351 (subtree layout for overflow areas). Just commenting-out the extra condition in objectIsRelayoutBoundary() fixes the bug. The root cause for the bug is that the code in FrameView::scheduleRelayoutOfSubtree() to handle a new root that is an ancestor or a descendant of the current root is wrong. It uses the DOM tree while in fact it should use the container()-hood graph of render objects. In the Google Maps case, the result is that when scheduleRelayoutOfSubtree() tries to mark the path from the descendant up to the ancestor, it marks all the way up to the root. Consequently, at the end of the subtree layout, the root is still marked for layout, and a second layout happens under invalidateSelection(), exposing a different bug: the inner layout clears the repaint rects set held by the frame, so the repaint never happens. Created attachment 17372 [details] Track the layout root using the render tree and the container() relation In addition to fixing the root cause and the other bug mentioned in comment #3, this patch changes layoutRoot from a RefPtr<Node> to a RenderObject*. It makes more sense to keep track of the layout root in terms of the render tree, and the patch includes an ASSERT that the layout root does not get deleted. No layout test regressions. Comment on attachment 17372 [details]
Track the layout root using the render tree and the container() relation
You might add a comment to:
RenderObject* layoutRoot;
something like:
// weak reference, ASSERT in ~RenderObject assures this is never made stale
I need to ask you a couple questions over IRC.
Comment on attachment 17372 [details]
Track the layout root using the render tree and the container() relation
Looks good. r=me.
|