<webview>: Synchronize WebViewFocusInteractiveTest.FocusAndVisibility

<webview>: Synchronize WebViewFocusInteractiveTest.FocusAndVisibility

Fix flakiness present on mac and re-enable the test for mac.

The guest js was drastically simplified to emit only two events to the guest,
button was focused and keyup.

The embedder then keeps the test in sync with each keypress and then
asserts that the guest button was focused or not.

There is a difference between oopif and browser plugin webviews that the
embedder in browserplugin might see all keyboard events and must
suppress those meant for the guest and only reply once the guest has
processed the keyboard input.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
BUG=670008, 610819
Review-Url: https://codereview.chromium.org/2648263006
Cr-Commit-Position: refs/heads/master@{#446130}
Committed: https://chromium.googlesource.com/chromium/src/+/f34dd22518c310f8b94bc75f4b82292f055a5f6c

[avallee, 2017-01-25 17:08:30]

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

[commit-bot: I haz the power, 2017-01-25 17:08:59]

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-25 17:10:29]

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

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

Dry run: Try jobs failed on following builders:
  linux_site_isolati on master.tryserver.chromium.linux (JOB_FAILED, no build
URL)

[avallee, 2017-01-25 17:11:17]

Description was changed from

==========
<webview>: Synchronize WebViewFocusInteractiveTest.FocusAndVisibility

Fix flakiness present on mac and re-enable the test for mac.

The guest js was drastically simplified to emit only two events to the guest,
button was focused and keyup.

The embedder then keeps the test in sync with each keypress and then
asserts that the guest button was focused or not.

There is a difference between oopif and browser plugin webviews that the
embedder in browserplugin might see all keyboard events and must
suppress those meant for the guest and only reply once the guest has
processed the keyboard input.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolati
BUG=670008,610819
==========

to

==========
<webview>: Synchronize WebViewFocusInteractiveTest.FocusAndVisibility

Fix flakiness present on mac and re-enable the test for mac.

The guest js was drastically simplified to emit only two events to the guest,
button was focused and keyup.

The embedder then keeps the test in sync with each keypress and then
asserts that the guest button was focused or not.

There is a difference between oopif and browser plugin webviews that the
embedder in browserplugin might see all keyboard events and must
suppress those meant for the guest and only reply once the guest has
processed the keyboard input.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
BUG=670008,610819
==========

[avallee, 2017-01-25 17:13:01]

avallee@chromium.org changed reviewers:
+ ekaramad@chromium.org, wjmaclean@chromium.org

[avallee, 2017-01-25 17:13:02]

Ehsan: test has changed a lot, let me know if you think this still looks
correct.

James: general review/owners.

[avallee, 2017-01-25 17:13:37]

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

[commit-bot: I haz the power, 2017-01-25 17:14:08]

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

[wjmaclean, 2017-01-25 17:21:39]

lgtm

[EhsanK, 2017-01-25 17:57:58]

Thanks for fixing/cleaning this test! LGTM with nits.

https://codereview.chromium.org/2648263006/diff/1/chrome/browser/apps/guest_v...
File chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc (right):

https://codereview.chromium.org/2648263006/diff/1/chrome/browser/apps/guest_v...
chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc:1374: // On
mac, the event listener seems one key even behind and deadlocks. Send an
s/even/event?
Also is there a short explanation on how this is happening on Mac to add to the
comment?

https://codereview.chromium.org/2648263006/diff/1/chrome/browser/apps/guest_v...
chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc:1389: // Now
make the <webview> invisible.
Given that the next couple of lines first reset the test, maybe change this
comment to "Reset the test and now make the <webview> invisible."

https://codereview.chromium.org/2648263006/diff/1/chrome/test/data/extensions...
File
chrome/test/data/extensions/platform_apps/web_view/focus_visibility/window.js
(right):

https://codereview.chromium.org/2648263006/diff/1/chrome/test/data/extensions...
chrome/test/data/extensions/platform_apps/web_view/focus_visibility/window.js:13:
window.addEventListener('keyup', function() {
Could you please add a short comment about why we have this exception here? I
assume (based on yesterdays explanation) for non-OOPIF we would end up with two
key-up ACKs from both the embedder and the guest?

[commit-bot: I haz the power, 2017-01-25 18:16:48]

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

[commit-bot: I haz the power, 2017-01-25 18:16:49]

Dry run: This issue passed the CQ dry run.

[avallee, 2017-01-25 18:49:45]

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

[avallee, 2017-01-25 18:50:18]

Ehsan: fixed the issues.

https://codereview.chromium.org/2648263006/diff/1/chrome/browser/apps/guest_v...
File chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc (right):

https://codereview.chromium.org/2648263006/diff/1/chrome/browser/apps/guest_v...
chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc:1374: // On
mac, the event listener seems one key even behind and deadlocks. Send an
On 2017/01/25 17:57:58, EhsanK wrote:
> s/even/event?
> Also is there a short explanation on how this is happening on Mac to add to
the
> comment?

Done.

https://codereview.chromium.org/2648263006/diff/1/chrome/browser/apps/guest_v...
chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc:1389: // Now
make the <webview> invisible.
On 2017/01/25 17:57:58, EhsanK wrote:
> Given that the next couple of lines first reset the test, maybe change this
> comment to "Reset the test and now make the <webview> invisible."

Done.

https://codereview.chromium.org/2648263006/diff/1/chrome/test/data/extensions...
File
chrome/test/data/extensions/platform_apps/web_view/focus_visibility/window.js
(right):

https://codereview.chromium.org/2648263006/diff/1/chrome/test/data/extensions...
chrome/test/data/extensions/platform_apps/web_view/focus_visibility/window.js:13:
window.addEventListener('keyup', function() {
On 2017/01/25 17:57:58, EhsanK wrote:
> Could you please add a short comment about why we have this exception here? I
> assume (based on yesterdays explanation) for non-OOPIF we would end up with
two
> key-up ACKs from both the embedder and the guest?

Without the check non-oopif would double/early-ack the events (essentially being
pointless since we care about when the webview acks).

In oopif, this condition blocks on transition into the webview since the tab is
sent to the embedder, but activeElement is the webview. So in oopif, always
dispatch the reply.

[commit-bot: I haz the power, 2017-01-25 18:50:31]

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

[EhsanK, 2017-01-25 19:31:37]

Thanks! LGTM.

https://codereview.chromium.org/2648263006/diff/1/chrome/browser/apps/guest_v...
File chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc (right):

https://codereview.chromium.org/2648263006/diff/1/chrome/browser/apps/guest_v...
chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc:1374: // On
mac, the event listener seems one key even behind and deadlocks. Send an
On 2017/01/25 18:50:18, avallee wrote:
> On 2017/01/25 17:57:58, EhsanK wrote:
> > s/even/event?
> > Also is there a short explanation on how this is happening on Mac to add to
> the
> > comment?
> 
> Done.

Acknowledged.

https://codereview.chromium.org/2648263006/diff/1/chrome/browser/apps/guest_v...
chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc:1389: // Now
make the <webview> invisible.
On 2017/01/25 18:50:18, avallee wrote:
> On 2017/01/25 17:57:58, EhsanK wrote:
> > Given that the next couple of lines first reset the test, maybe change this
> > comment to "Reset the test and now make the <webview> invisible."
> 
> Done.

Acknowledged.

https://codereview.chromium.org/2648263006/diff/1/chrome/test/data/extensions...
File
chrome/test/data/extensions/platform_apps/web_view/focus_visibility/window.js
(right):

https://codereview.chromium.org/2648263006/diff/1/chrome/test/data/extensions...
chrome/test/data/extensions/platform_apps/web_view/focus_visibility/window.js:13:
window.addEventListener('keyup', function() {
On 2017/01/25 18:50:18, avallee wrote:
> On 2017/01/25 17:57:58, EhsanK wrote:
> > Could you please add a short comment about why we have this exception here?
I
> > assume (based on yesterdays explanation) for non-OOPIF we would end up with
> two
> > key-up ACKs from both the embedder and the guest?
> 
> Without the check non-oopif would double/early-ack the events (essentially
being
> pointless since we care about when the webview acks).
> 
> In oopif, this condition blocks on transition into the webview since the tab
is
> sent to the embedder, but activeElement is the webview. So in oopif, always
> dispatch the reply.

Thanks!

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

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

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

Dry run: This issue passed the CQ dry run.

[avallee, 2017-01-25 21:46:52]

The CQ bit was checked by avallee@chromium.org

[avallee, 2017-01-25 21:46:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
wjmaclean@chromium.org
Link to the patchset: https://codereview.chromium.org/2648263006/#ps20001
(title: "Address ehsank's comments.")

[commit-bot: I haz the power, 2017-01-25 21:47:39]

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-25 21:52:40]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485380812732770,
"parent_rev": "4afe10c428a371521326f42b312cdcdf9e87a7dd", "commit_rev":
"f34dd22518c310f8b94bc75f4b82292f055a5f6c"}

[commit-bot: I haz the power, 2017-01-25 21:53:17]

Description was changed from

==========
<webview>: Synchronize WebViewFocusInteractiveTest.FocusAndVisibility

Fix flakiness present on mac and re-enable the test for mac.

The guest js was drastically simplified to emit only two events to the guest,
button was focused and keyup.

The embedder then keeps the test in sync with each keypress and then
asserts that the guest button was focused or not.

There is a difference between oopif and browser plugin webviews that the
embedder in browserplugin might see all keyboard events and must
suppress those meant for the guest and only reply once the guest has
processed the keyboard input.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
BUG=670008,610819
==========

to

==========
<webview>: Synchronize WebViewFocusInteractiveTest.FocusAndVisibility

Fix flakiness present on mac and re-enable the test for mac.

The guest js was drastically simplified to emit only two events to the guest,
button was focused and keyup.

The embedder then keeps the test in sync with each keypress and then
asserts that the guest button was focused or not.

There is a difference between oopif and browser plugin webviews that the
embedder in browserplugin might see all keyboard events and must
suppress those meant for the guest and only reply once the guest has
processed the keyboard input.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
BUG=670008,610819

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

[commit-bot: I haz the power, 2017-01-25 21:53:18]

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