Bug 45573 – [chromium] Fix incorrect drag node/selection painting.

• Component: Platform
• Status: RESOLVED FIXED

[Daniel Cheng]

If you dragged a selection near the bottom of the page, the selection would not be renderer correctly (it would be clipped as if the page hadn't been scrolled down).
I also updated dragImageForNode to use RenderLayer::paint() as well, since that allows us to remove PaintBehavior from the RTTI helper class. I'm not 100% sure if the new code in Frame::dragImageForNode correct though--it uses document()->renderer() and node->renderer(), which is a little weird. However, if I just used RenderObject* renderer = node->renderer(), it would throw the following assert during the paint:
ASSERTION FAILED: rootLayer == m_clipRectsRoot

[Daniel Cheng]

Created attachment 67260 [details]
Patch

[Eric Seidel (no email)]

James is really a better reviewer for this than I am.

Is this how other ports do this?  We seem to have a bunch of code which does manual painting.  I know SVG code uses some paint-into-ImageBuffer code for SVGImage and masks.  I wonder how many other places do/do-not want scroll clipping and if we should be creating a simple API for all these clients to use instead of grabbing at layers directly.

[James Robinson]

I still don't understand what exact semantics you are going for here, so it's hard to judge whether the code does what you want.  Where are the tests for this?

[Simon Fraser (smfr)]

Comment on attachment 67260 [details]
Patch

I think this will do the wrong thing with iframes and compositing. Iframes pull the paint behavior flags from their parent iframe (see -[WebFrame _drawRect:] in WebKit.

Rather than paint via the RenderLayer, why not call m_view->paintContents, which does not intersect with the scrollview's frame?

It would also be nice to share more code here with Mac's Frame::imageFromRect().

[Daniel Cheng]

Created attachment 67716 [details]
Patch

[Daniel Cheng]

Comment on attachment 67716 [details]
Patch

(In reply to comment #3)
> I still don't understand what exact semantics you are going for here, so it's hard to judge whether the code does what you want.  Where are the tests for this?

There are no tests. The problem is ScrollView::paint() seemed to be clipping as if the page was never scrolled down.

(In reply to comment #4)
> (From update of attachment 67260 [details])
> I think this will do the wrong thing with iframes and compositing. Iframes pull the paint behavior flags from their parent iframe (see -[WebFrame _drawRect:] in WebKit.
> 
> Rather than paint via the RenderLayer, why not call m_view->paintContents, which does not intersect with the scrollview's frame?
> 

I didn't know about this function. I've updated the patch.

> It would also be nice to share more code here with Mac's Frame::imageFromRect().

I based the implementation of these two functions off of Frame::imageFromRect(). However, they seemed to have a lot of Cocoa-specific magic, so I'm not sure how I could share any more code. Ideas are welcome.

[WebKit Commit Bot]

Comment on attachment 67716 [details]
Patch

Rejecting patch 67716 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--wait-for-httpd', '--quiet']" exit_code: 1
Running build-dumprendertree
Compiling Java tests
make: Nothing to be done for `default'.
Running tests from /Projects/CommitQueue/LayoutTests
Testing 21296 test cases.
websocket/tests/handshake-fail-by-sub-protocol-mismatch.html -> failed

Exiting early after 1 failures. 21268 tests run.
529.38s total testing time

21267 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
33 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/4063026

[Tony Chang]

Trying commit queue again because it doesn't seem that this code is run by websocket tests.

[WebKit Commit Bot]

Comment on attachment 67716 [details]
Patch

Clearing flags on attachment: 67716

Committed r67645: <http://trac.webkit.org/changeset/67645>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.