Remove RenderWidgetHostImpl::SuppressEventsUntilKeyDown()

Remove RenderWidgetHostImpl::SuppressEventsUntilKeyDown()

This revert https://codereview.chromium.org/12221133 where the
suppression on focus was originally added. No tests were added, but the
manual test described now passes without this code. There is of course
some chance that it is needed for some other case, so this CL should be
revert at the first sign of trouble.

The reason for this change is that the suppression on focus causes the
PrintScreen KeyUp event to be dropped.

BUG=668969
Review-Url: https://codereview.chromium.org/2643243002
Cr-Commit-Position: refs/heads/master@{#446023}
Committed: https://chromium.googlesource.com/chromium/src/+/e5fd75af8171718a84d387486cfba0ed225c6f5f

[foolip, 2017-01-20 12:25:35]

foolip@chromium.org changed reviewers:
+ sadrul@chromium.org

[foolip, 2017-01-20 12:25:35]

PTAL. (I can't run the try jobs on this before the first CL has landed.)

[sadrul, 2017-01-23 18:18:14]

https://codereview.chromium.org/2643243002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_view_aura.cc (left):

https://codereview.chromium.org/2643243002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_aura.cc:1751:
host_->SuppressEventsUntilKeyDown();
How does this play with ctrl+tab to move focus? i.e. with this code, I assume
the page will not see the key-ups for ctrl and tab. Without this code, the page
might see these events? Is that going to be a problem?

[foolip, 2017-01-24 04:41:51]

Description was changed from

==========
Remove RenderWidgetHostImpl::SuppressEventsUntilKeyDown()

This reverts https://codereview.chromium.org/2641313002 and
https://codereview.chromium.org/12221133 where the suppression on focus
was originally added. No tests were added, but the manual test describes
now passes without this code. There is of course some chance that it is
needed for some other case, so this CL should be revert at the first
sign of trouble.

BUG=668969
==========

to

==========
Remove RenderWidgetHostImpl::SuppressEventsUntilKeyDown()

This revert https://codereview.chromium.org/12221133 where the
suppression on focus was originally added. No tests were added, but the
manual test describes now passes without this code. There is of course
some chance that it is needed for some other case, so this CL should be
revert at the first sign of trouble.

BUG=668969
==========

[foolip, 2017-01-24 04:43:23]

Description was changed from

==========
Remove RenderWidgetHostImpl::SuppressEventsUntilKeyDown()

This revert https://codereview.chromium.org/12221133 where the
suppression on focus was originally added. No tests were added, but the
manual test describes now passes without this code. There is of course
some chance that it is needed for some other case, so this CL should be
revert at the first sign of trouble.

BUG=668969
==========

to

==========
Remove RenderWidgetHostImpl::SuppressEventsUntilKeyDown()

This revert https://codereview.chromium.org/12221133 where the
suppression on focus was originally added. No tests were added, but the
manual test describes now passes without this code. There is of course
some chance that it is needed for some other case, so this CL should be
revert at the first sign of trouble.

The reason for this change is that the suppression on focus causes the
PrintScreen KeyUp event to be dropped.

BUG=668969
==========

[foolip, 2017-01-24 05:25:21]

I've updated this CL to no longer depend on
https://codereview.chromium.org/2641313002/

Unfortunately it's not easy to write a meaningful test for the lack of
PrintScreen suppression when the mechanism that suppressed it is gone. Other
ideas for how to lock down the behavior?

https://codereview.chromium.org/2643243002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_view_aura.cc (left):

https://codereview.chromium.org/2643243002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_view_aura.cc:1751:
host_->SuppressEventsUntilKeyDown();
On 2017/01/23 18:18:13, sadrul wrote:
> How does this play with ctrl+tab to move focus? i.e. with this code, I assume
> the page will not see the key-ups for ctrl and tab. Without this code, the
page
> might see these events? Is that going to be a problem?

Aha, another unintentional change caused by
https://codereview.chromium.org/2482853002

I've tested switching between two pages with this code in two tabs:
<!doctype html>
<pre></pre>
<script>
const pre = document.querySelector('pre');
const log = msg => pre.textContent += msg + '\n';
const listen = type => document.body.addEventListener(type, event =>
log(event.type + ': ' + event.keyCode));
listen('keydown');
listen('keypress');
listen('keyup');
</script>

Before https://codereview.chromium.org/2482853002 (Chrome stable) the keydown
events for for ctrl would reach the first page and the keyup event the second
page. The same is true of the tab key, except that it doesn't seem to arrive in
the first cycle. Not sure why, probably not related to this code.

After that change, on Chrome canary, only the keydown events for ctrl show up in
any tab.

So, removing SuppressEventsUntilKeyDown() should make the behavior more similar
to before.

[foolip, 2017-01-24 09:50:34]

The CQ bit was checked by foolip@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 09:50:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 10:36:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-24 10:36:27]

Dry run: This issue passed the CQ dry run.

[sadrul, 2017-01-25 14:23:15]

lgtm

[foolip, 2017-01-25 14:37:01]

The CQ bit was checked by foolip@chromium.org

[commit-bot: I haz the power, 2017-01-25 14:37:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[foolip, 2017-01-25 14:37:33]

Description was changed from

==========
Remove RenderWidgetHostImpl::SuppressEventsUntilKeyDown()

This revert https://codereview.chromium.org/12221133 where the
suppression on focus was originally added. No tests were added, but the
manual test describes now passes without this code. There is of course
some chance that it is needed for some other case, so this CL should be
revert at the first sign of trouble.

The reason for this change is that the suppression on focus causes the
PrintScreen KeyUp event to be dropped.

BUG=668969
==========

to

==========
Remove RenderWidgetHostImpl::SuppressEventsUntilKeyDown()

This revert https://codereview.chromium.org/12221133 where the
suppression on focus was originally added. No tests were added, but the
manual test described now passes without this code. There is of course
some chance that it is needed for some other case, so this CL should be
revert at the first sign of trouble.

The reason for this change is that the suppression on focus causes the
PrintScreen KeyUp event to be dropped.

BUG=668969
==========

[commit-bot: I haz the power, 2017-01-25 15:24:37]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485355021492430,
"parent_rev": "6e262965967f5cd06e86045a51bae65c6d818bd4", "commit_rev":
"e5fd75af8171718a84d387486cfba0ed225c6f5f"}

[commit-bot: I haz the power, 2017-01-25 15:25:10]

Description was changed from

==========
Remove RenderWidgetHostImpl::SuppressEventsUntilKeyDown()

This revert https://codereview.chromium.org/12221133 where the
suppression on focus was originally added. No tests were added, but the
manual test described now passes without this code. There is of course
some chance that it is needed for some other case, so this CL should be
revert at the first sign of trouble.

The reason for this change is that the suppression on focus causes the
PrintScreen KeyUp event to be dropped.

BUG=668969
==========

to

==========
Remove RenderWidgetHostImpl::SuppressEventsUntilKeyDown()

This revert https://codereview.chromium.org/12221133 where the
suppression on focus was originally added. No tests were added, but the
manual test described now passes without this code. There is of course
some chance that it is needed for some other case, so this CL should be
revert at the first sign of trouble.

The reason for this change is that the suppression on focus causes the
PrintScreen KeyUp event to be dropped.

BUG=668969

Review-Url: https://codereview.chromium.org/2643243002
Cr-Commit-Position: refs/heads/master@{#446023}
Committed:
https://chromium.googlesource.com/chromium/src/+/e5fd75af8171718a84d387486cfb...
==========

[commit-bot: I haz the power, 2017-01-25 15:25:11]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/e5fd75af8171718a84d387486cfb...