Instant Extended: Suppress mouse events for bottom web content

Instant Extended: Suppress mouse events for bottom web content

When the omnibox overlay is open both the overlay's web contents and
the active tab's web contents get mouse events.

This causes problems like the cursor changing due to links below the
omnibox overlay.

A similar problem was fixed for the overlapping find bar by asvitkine
in crrev.com/6676094. This CL piggybacks on that fix by also checking
for overlapping web contents.

BUG=238846
TEST=Navigate to google.com. Type "http://youtube.com" in the omnibox.
Mouse over the top of the overlay. Without my fix the cursor changes
due to the links on google.com. With my fix the cursor does not change.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=203222

[sail, 2013-05-22 01:51:40]

thakis: Please review

[Alexei Svitkine (slow), 2013-05-22 03:34:43]

https://codereview.chromium.org/15315005/diff/2001/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/15315005/diff/2001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_mac.mm:1731: if ([view
isKindOfClass:[self class]] && ![view isEqual:self]) {
Is there a reason to use -isEqual: here rather than just ==?

https://codereview.chromium.org/15315005/diff/2001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_mac.mm:1732: // The cursor
is over an overlapping render widget.
Please expand the comment to mention that given both views will be doing this
check, that events will be processed by one returned by -hitTest:.

[sail, 2013-05-22 17:54:10]

https://codereview.chromium.org/15315005/diff/2001/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/15315005/diff/2001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_mac.mm:1731: if ([view
isKindOfClass:[self class]] && ![view isEqual:self]) {
On 2013/05/22 03:34:43, Alexei Svitkine wrote:
> Is there a reason to use -isEqual: here rather than just ==?

This a normal Objective-C convention. For example, if you call -[NSArray
indexOfObject:] it will use isEqual: not ==.

https://codereview.chromium.org/15315005/diff/2001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_mac.mm:1732: // The cursor
is over an overlapping render widget.
On 2013/05/22 03:34:43, Alexei Svitkine wrote:
> Please expand the comment to mention that given both views will be doing this
> check, that events will be processed by one returned by -hitTest:.

Done.

[Alexei Svitkine (slow), 2013-05-22 18:06:52]

lgtm

[commit-bot: I haz the power, 2013-05-22 18:14:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/15315005/7001

[commit-bot: I haz the power, 2013-05-22 18:28:06]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[sail, 2013-05-22 18:28:23]

thakis: Need content/* OWNERS review

[sail, 2013-05-25 13:47:40]

+avi

[Nico, 2013-05-28 20:06:40]

lgtm

[commit-bot: I haz the power, 2013-05-28 20:11:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/15315005/7001

[Avi (use Gerrit), 2013-05-28 20:11:06]

LGTM

though that whole "non web content" part makes me go 'wha?'

https://codereview.chromium.org/15315005/diff/7001/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/15315005/diff/7001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_mac.mm:1732: [view
performSelector:nonWebContentViewSelector]) {
What the heck?

[commit-bot: I haz the power, 2013-05-28 20:32:09]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Alexei Svitkine (slow), 2013-05-28 20:37:59]

https://codereview.chromium.org/15315005/diff/7001/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/15315005/diff/7001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_mac.mm:1732: [view
performSelector:nonWebContentViewSelector]) {
On 2013/05/28 20:11:06, Avi wrote:
> What the heck?

You can blame Mento for this, if you want. ;)

I think this was my first Chromium CL - I believe I originally just did a
-isKindOfClass: check against the find bar but we wanted a more general
solution, so the suggestion was for views to implement -nonWebContentView to
indicate they block events from going to the renderer. (vs. views that are part
of web content, like a plugin container iirc)

[Avi (use Gerrit), 2013-05-28 20:41:54]

https://codereview.chromium.org/15315005/diff/7001/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/15315005/diff/7001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_mac.mm:1732: [view
performSelector:nonWebContentViewSelector]) {
:( I would hope for something a little more magical, like saying, "hey, if I'm
in the background then don't look at me". (shrug) Oh well, I'm not objecting to
this CL due to this...

[Alexei Svitkine (slow), 2013-05-28 20:44:29]

https://codereview.chromium.org/15315005/diff/7001/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/15315005/diff/7001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_mac.mm:1732: [view
performSelector:nonWebContentViewSelector]) {
On 2013/05/28 20:41:54, Avi wrote:
> :( I would hope for something a little more magical, like saying, "hey, if I'm
> in the background then don't look at me".

(It does that too using the -hitTest: above. The problem is differentiating a
plugin container view vs. a find bar view, both in the foreground w.r.t. to the
render view - but the plugin view wanting events to go to the renderer, while
the find bar not wanting this.)

[Avi (use Gerrit), 2013-05-28 20:47:52]

https://codereview.chromium.org/15315005/diff/7001/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/15315005/diff/7001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_mac.mm:1732: [view
performSelector:nonWebContentViewSelector]) {
On 2013/05/28 20:44:29, Alexei Svitkine wrote:
> On 2013/05/28 20:41:54, Avi wrote:
> > :( I would hope for something a little more magical, like saying, "hey, if
I'm
> > in the background then don't look at me".
> 
> (It does that too using the -hitTest: above. The problem is differentiating a
> plugin container view vs. a find bar view, both in the foreground w.r.t. to
the
> render view - but the plugin view wanting events to go to the renderer, while
> the find bar not wanting this.)

Oh. Eeew.

Does this change now that we have compositing everywhere? Do we still have
plugin containers?

[Alexei Svitkine (slow), 2013-05-28 20:58:11]

https://codereview.chromium.org/15315005/diff/7001/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/15315005/diff/7001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_view_mac.mm:1732: [view
performSelector:nonWebContentViewSelector]) {
On 2013/05/28 20:47:52, Avi wrote:
> On 2013/05/28 20:44:29, Alexei Svitkine wrote:
> > On 2013/05/28 20:41:54, Avi wrote:
> > > :( I would hope for something a little more magical, like saying, "hey, if
> I'm
> > > in the background then don't look at me".
> > 
> > (It does that too using the -hitTest: above. The problem is differentiating
a
> > plugin container view vs. a find bar view, both in the foreground w.r.t. to
> the
> > render view - but the plugin view wanting events to go to the renderer,
while
> > the find bar not wanting this.)
> 
> Oh. Eeew.
> 
> Does this change now that we have compositing everywhere? Do we still have
> plugin containers?

No clue.

[commit-bot: I haz the power, 2013-05-30 17:23:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/15315005/7001

[Nico, 2013-05-30 17:40:47]

On Tue, May 28, 2013 at 1:58 PM, <asvitkine@chromium.org> wrote:

>
> https://codereview.chromium.**org/15315005/diff/7001/**
>
content/browser/renderer_host/**render_widget_host_view_mac.mm<https://codereview.chromium.org/15315005/diff/7001/content/browser/renderer_host/render_widget_host_view_mac.mm>
> File
content/browser/renderer_host/**render_widget_host_view_mac.mm<http://render_widget_host_view_mac.mm>
> (right):
>
> https://codereview.chromium.**org/15315005/diff/7001/**
> content/browser/renderer_host/**render_widget_host_view_mac.**
>
mm#newcode1732<https://codereview.chromium.org/15315005/diff/7001/content/browser/renderer_host/render_widget_host_view_mac.mm#newcode1732>
>
content/browser/renderer_host/**render_widget_host_view_mac.**mm:1732<http://render_widget_host_view_mac.mm:1732>:
> [view
> performSelector:**nonWebContentViewSelector]) {
> On 2013/05/28 20:47:52, Avi wrote:
>
>> On 2013/05/28 20:44:29, Alexei Svitkine wrote:
>> > On 2013/05/28 20:41:54, Avi wrote:
>> > > :( I would hope for something a little more magical, like saying,
>>
> "hey, if
>
>> I'm
>> > > in the background then don't look at me".
>> >
>> > (It does that too using the -hitTest: above. The problem is
>>
> differentiating a
>
>> > plugin container view vs. a find bar view, both in the foreground
>>
> w.r.t. to
>
>> the
>> > render view - but the plugin view wanting events to go to the
>>
> renderer, while
>
>> > the find bar not wanting this.)
>>
>
>  Oh. Eeew.
>>
>
>  Does this change now that we have compositing everywhere? Do we still
>>
> have
>
>> plugin containers?
>>
>
I don't think so.


>
> No clue.
>
>
https://codereview.chromium.**org/15315005/<https://codereview.chromium.org/1...
>

[commit-bot: I haz the power, 2013-05-30 19:42:21]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-05-30 19:50:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/15315005/7001

[commit-bot: I haz the power, 2013-05-30 21:16:39]

Change committed as 203222