Apply a CursorRect to the extent of the render widget view for web page cursors.

Apply a CursorRect to the extent of the render widget view for web page cursors.

BUG=114517
TEST=None


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=140495

[Avi (use Gerrit), 2012-05-24 15:14:49]

I like it, assuming it doesn't break cursor handling for things like plugins...

https://chromiumcodereview.appspot.com/10450012/diff/1/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_mac.h (right):

https://chromiumcodereview.appspot.com/10450012/diff/1/content/browser/render...
content/browser/renderer_host/render_widget_host_view_mac.h:70: NSCursor*
current_cursor_;
Rather than the retains/releases in the .mm, you should use scoped_nsobject
here.

https://chromiumcodereview.appspot.com/10450012/diff/1/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://chromiumcodereview.appspot.com/10450012/diff/1/content/browser/render...
content/browser/renderer_host/render_widget_host_view_mac.mm:549: WebCursor
web_cursor = cursor;
Why this copy? If GetNativeCursor() isn't const, can you make it?

https://chromiumcodereview.appspot.com/10450012/diff/1/content/browser/render...
content/browser/renderer_host/render_widget_host_view_mac.mm:2865: [[self
window] invalidateCursorRectsForView:self];
That is... elegant. Wow.

[dominicc (has gone to gerrit), 2012-05-31 09:36:24]

In looking at this I have noticed a few other cursor bugs I'll file, like WebKit
sets the cursor when it loads a cursor, even if it is not pointing at something
with that cursor. And there is still some flakiness when mousing from outside
the window into the window. Debugging that is hard because it is timing and
focus dependent. This patch makes the situation appreciably better, at least.

On 2012/05/24 15:14:49, Avi wrote:
> I like it, assuming it doesn't break cursor handling for things like
plugins...

I played around with the YouTube video player and it seems fine – the cursor
turns into a hand when pointing at the play/pause button, etc.

This still needs a test. Any pointers? grepping did not turn up anything.

>
https://chromiumcodereview.appspot.com/10450012/diff/1/content/browser/render...
> content/browser/renderer_host/render_widget_host_view_mac.h:70: NSCursor*
> current_cursor_;
> Rather than the retains/releases in the .mm, you should use scoped_nsobject
> here.

Done.

>
https://chromiumcodereview.appspot.com/10450012/diff/1/content/browser/render...
> content/browser/renderer_host/render_widget_host_view_mac.mm:549: WebCursor
> web_cursor = cursor;
> Why this copy? If GetNativeCursor() isn't const, can you make it?

Happy to try… this is going to touch multiple members in every platform.

[Avi (use Gerrit), 2012-05-31 14:11:03]

On 2012/05/31 09:36:24, dominicc wrote:
> I played around with the YouTube video player and it seems fine – the cursor
> turns into a hand when pointing at the play/pause button, etc.

Nice.

> This still needs a test. Any pointers? grepping did not turn up anything.

Can't think of anything; this is one of those things that I'm not sure how to
test.

> > Why this copy? If GetNativeCursor() isn't const, can you make it?
> 
> Happy to try… this is going to touch multiple members in every platform.

That's OK with me.

[dominicc (has gone to gerrit), 2012-06-01 05:04:58]

> > > Why this copy? If GetNativeCursor() isn't const, can you make it?
> > 
> > Happy to try… this is going to touch multiple members in every platform.
> 
> That's OK with me.

Looks like Windows GetNativeCursor isn’t amenable to being const, because it
creates and caches a HICON; GTK isn’t amenable to being const because it caches
custom cursors for the purpose of unrefing them later.

[Avi (use Gerrit), 2012-06-01 14:33:37]

I'd argue that it's logically const, and if they're caching stuff they should
have mutable member vars.

But this code LGTM as-is; so much nicer than what we do now.

[commit-bot: I haz the power, 2012-06-04 04:39:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dominicc@chromium.org/10450012/4001

[commit-bot: I haz the power, 2012-06-04 04:59:17]

Try job failure for 10450012-4001 (retry) on mac_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-06-05 04:10:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dominicc@chromium.org/10450012/16001

[commit-bot: I haz the power, 2012-06-05 07:35:45]

Change committed as 140495