New issue
Advanced search Search tips

Issue 754878 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

BrowserCommandControllerInteractiveTest.ShortcutsShouldTakeEffectInJsFullscreen is flaky on ChromeOS

Project Member Reported by zijiehe@chromium.org, Aug 11 2017

Issue description

Components: Tests>Disabled
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea37f8134f69e0d53747218534f57b00d408f0c3

commit ea37f8134f69e0d53747218534f57b00d408f0c3
Author: Zijie He <zijiehe@chromium.org>
Date: Fri Aug 11 23:30:10 2017

Disable ShortcutsShouldTakeEffectInJsFullscreen on ChromeOS

This test is flaky on Chrome OS. Refer to the bug for details.

Bug: chromium:754878
Change-Id: I7ece0e2836e439a3041eef93442b63c32f024efd
TBR: msw@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/611868
Commit-Queue: Zijie He <zijiehe@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493881}
[modify] https://crrev.com/ea37f8134f69e0d53747218534f57b00d408f0c3/chrome/browser/ui/browser_command_controller_interactive_browsertest.cc

Though the root cause is known, the failure always starts from an
"ERROR:tab_manager_delegate_chromeos.cc(81)] Set OOM score error: "
Current implementation triggers a weird error log,

[108966:108966:0811/181428.662853:INFO:CONSOLE(1)]
"Failed to execute 'requestFullscreen' on 'Element':
API can only be initiated by a user gesture.",
source: data:text/html,<html><head><script>document.addEventListener(
'keydown', () => { document.body.webkitRequestFullscreen(); });</script>
</head><body></body></html> (1)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/850df0ea5ed41b0e2d7cb8030ca30cd8ec9b4061

commit 850df0ea5ed41b0e2d7cb8030ca30cd8ec9b4061
Author: Zijie He <zijiehe@chromium.org>
Date: Fri Aug 18 23:24:24 2017

Deflaky BrowserCommandControllerInteractiveTest.ShortcutsShouldTakeEffectInJsFullscreen

It looks weird that this change can actually deflaky the test case.
After adding a check in e.code, this test case becomes more reliable on
ChromeOS: I have executed it for 100 times and no flakiness was caught.

My gut feeling is reloading the page for several times may randomly trigger
OOM error, so the execution of webkitRequestFullscreen() fails. The
failure happens less frequently if no console log

[108966:108966:0811/181428.662853:INFO:CONSOLE(1)]
"Failed to execute 'requestFullscreen' on 'Element':
API can only be initiated by a user gesture.",
source: data:text/html,<html><head><script>document.addEventListener(
'keydown', () => { document.body.webkitRequestFullscreen(); });</script>
</head><body></body></html> (1)

printed.

Bug: chromium:754878
Change-Id: I65a4a98d774fef064717c3f4e77eb8e36b233d50
Reviewed-on: https://chromium-review.googlesource.com/611772
Commit-Queue: Zijie He <zijiehe@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495740}
[modify] https://crrev.com/850df0ea5ed41b0e2d7cb8030ca30cd8ec9b4061/chrome/browser/ui/browser_command_controller_interactive_browsertest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/20c5daa418c4c7006bcc921ff96607711e180e6a

commit 20c5daa418c4c7006bcc921ff96607711e180e6a
Author: Zijie He <zijiehe@chromium.org>
Date: Sat Aug 19 01:47:36 2017

Enable BrowserCommandControllerInteractiveTest.ShortcutsShouldTakeEffectInJsFullscreen

After change https://chromium-review.googlesource.com/c/611772, this test is
more reliable on Chrome OS. I would like to enable it.

Bug: chromium:754878
Change-Id: Id234ee7e27c0acd4a01699f6a2d24acfdf499b87
Reviewed-on: https://chromium-review.googlesource.com/622128
Commit-Queue: Zijie He <zijiehe@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495784}
[modify] https://crrev.com/20c5daa418c4c7006bcc921ff96607711e180e6a/chrome/browser/ui/browser_command_controller_interactive_browsertest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/19852bf58dc51246ae9608184a3a3ec7a7c29d79

commit 19852bf58dc51246ae9608184a3a3ec7a7c29d79
Author: Zijie He <zijiehe@chromium.org>
Date: Sat Aug 19 06:56:10 2017

Revert "Enable BrowserCommandControllerInteractiveTest.ShortcutsShouldTakeEffectInJsFullscreen"

This reverts commit 20c5daa418c4c7006bcc921ff96607711e180e6a.

Reason for revert: This test is still flaky on Chrome OS. See https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vMjBjNWRhYTQxOGM0YzcwMDZiY2M5MjFmZjk2NjA3NzExZTE4MGU2YQw.
It looks like this flakiness won't be able to workaround by changes in the test case. Maybe someone in ChromeOS team can help.

Original change's description:
> Enable BrowserCommandControllerInteractiveTest.ShortcutsShouldTakeEffectInJsFullscreen
> 
> After change https://chromium-review.googlesource.com/c/611772, this test is
> more reliable on Chrome OS. I would like to enable it.
> 
> Bug: chromium:754878
> Change-Id: Id234ee7e27c0acd4a01699f6a2d24acfdf499b87
> Reviewed-on: https://chromium-review.googlesource.com/622128
> Commit-Queue: Zijie He <zijiehe@chromium.org>
> Reviewed-by: Michael Wasserman <msw@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#495784}

TBR=msw@chromium.org,zijiehe@chromium.org

Change-Id: I10754c7c1fc084538cf96f896b63cf3502894a85
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:754878
Reviewed-on: https://chromium-review.googlesource.com/622527
Reviewed-by: Zijie He <zijiehe@chromium.org>
Commit-Queue: Zijie He <zijiehe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495807}
[modify] https://crrev.com/19852bf58dc51246ae9608184a3a3ec7a7c29d79/chrome/browser/ui/browser_command_controller_interactive_browsertest.cc

Owner: ----
Components: Tests>Disabled
Labels: Test-Disabled
Cc: linds...@chromium.org
Labels: Hotlist-DisableReview
This has not been updated since Aug. Can we just delete the test if its unowned and disabled since aug?

Comment 12 by zijiehe@google.com, Mar 20 2018

Maybe Joe (joedow@) can have a look. He is now working on the keyboard features of the Chrome.
Owner: joedow@chromium.org
Status: Assigned (was: Untriaged)
This bug has an owner, thus, it's been triaged. Changing status to "assigned".

Sign in to add a comment