Bug 50342 - Unnecessary invalidation of copyright <span> on IE9 Flying Images demo
Summary: Unnecessary invalidation of copyright <span> on IE9 Flying Images demo
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: James Robinson
Depends on:
Reported: 2010-12-01 12:01 PST by Darin Fisher (:fishd, Google)
Modified: 2010-12-14 17:40 PST (History)
7 users (show)

See Also:

testcase (1.05 KB, text/html)
2010-12-01 12:04 PST, Darin Fisher (:fishd, Google)
no flags Details
Patch (9.17 KB, patch)
2010-12-01 17:31 PST, James Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Fisher (:fishd, Google) 2010-12-01 12:01:48 PST
Unnecessary invalidation of copyright <span> on IE9 Flying Images demo

Watching invalidations that occur on the IE9 Flying Images demo (http://ie.microsoft.com/testdrive/Performance/FlyingImages/Default.html), I observe that the copyright span at the bottom of the page is continually invalidated.

I reduced this down somewhat.  See the attached test case.  In the test case, if I give #board an explicit "left" and "top" coordinate, then the problem goes away.

(If you are running Chromium, you can observe invalidations by running with the --show-paint-rects command line switch.)
Comment 1 Darin Fisher (:fishd, Google) 2010-12-01 12:04:00 PST
Created attachment 75300 [details]
Comment 2 Sam Weinig 2010-12-01 12:49:43 PST
You can also observe this using QuartzDebug on OS X.
Comment 3 Simon Fraser (smfr) 2010-12-01 12:56:26 PST
To debug, see who's calling repaint() on the span
Comment 4 James Robinson 2010-12-01 13:21:06 PST
render tree:

RenderView 0x7ffff7ef1a98              	#document	0x7ffff7ef8000
  RenderBlock 0x7ffff7f30478           	HTML	0x7fffeb51c800
*   RenderBody 0x7ffff7f30ef8          	BODY	0x7fffeb064240
      RenderBlock (positioned) 0x7fffe6e78718	DIV	0x7fffeaa4cb00
        RenderBlock (positioned) 0x7fffe6e78558	DIV	0x7fffeaa4ca80 STYLE=top: -300px; 
      RenderBlock (positioned) 0x7fffe6e78478	SPAN	0x7fffeaa4c980
        RenderText 0x7fffeaa4c918      	#text	0x7fffeaa48540 "Blah, blah"

RenderBlock::layoutInlineChildren() on the <body>'s RenderBlock is marking the <span> as m_normalChildNeedsLayout because the <span> has a percentage width.  This means it later gets marked for repainting even though none of its children actually move.  http://trac.webkit.org/browser/trunk/WebCore/rendering/RenderBlockLineLayout.cpp#L552 seems a little pessimistic.
Comment 5 James Robinson 2010-12-01 17:31:10 PST
Created attachment 75337 [details]
Comment 6 Eric Seidel (no email) 2010-12-02 17:38:02 PST
Comment on attachment 75337 [details]

This looks like a mitz/hyatt patch to review. :)
Comment 7 James Robinson 2010-12-02 17:44:34 PST
Actually hyatt typed out the meat of this patch verbatim in IRC.  I just added the comment, testcase, and verified that this doesn't cause all the other layout tests to explode.
Comment 8 WebKit Commit Bot 2010-12-02 21:58:23 PST
Comment on attachment 75337 [details]

Rejecting patch 75337 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=abarth-cq-sl', 'build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive']" exit_code: 2
Last 500 characters of output:
ning/auto .......
fast/blockflow ................................................
fast/body-propagation/background-color .....................
fast/body-propagation/background-image .........................
fast/body-propagation/overflow ...................
fast/borders ...
fast/borders/border-fit.html -> failed

Exiting early after 1 failures. 5969 tests run.
105.41s total testing time

5968 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
3 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/6751028
Comment 9 Eric Seidel (no email) 2010-12-14 01:28:47 PST
Seems this patch will need an update since it breaks a test.
Comment 10 Eric Seidel (no email) 2010-12-14 15:14:06 PST
Attachment 75337 [details] was posted by a committer and has review+, assigning to James Robinson for commit.
Comment 11 James Robinson 2010-12-14 17:40:08 PST
Comment on attachment 75337 [details]

Clearing flags, patch was reverted.