New issue
Advanced search Search tips

Issue 675851 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: 2017-11-30
OS: ----
Pri: 1
Type: Bug-Regression



Sign in to add a comment

TabCaptureApiTest.FullscreenEvents is flaky

Project Member Reported by yoichio@chromium.org, Dec 20 2016

Issue description

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 20 2016

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

commit 996b83c7e4a2ab246ba1b4e5f51dc11cd610f9c4
Author: Yoichi Osato <yoichio@chromium.org>
Date: Tue Dec 20 06:33:21 2016

Disable TabCaptureApiTest.FullscreenEvents test

TBR=OWNER
BUG=675851

Review-Url: https://codereview.chromium.org/2589103002 .
Cr-Commit-Position: refs/heads/master@{#439729}

[modify] https://crrev.com/996b83c7e4a2ab246ba1b4e5f51dc11cd610f9c4/chrome/browser/extensions/api/tab_capture/tab_capture_apitest.cc

Status: Fixed (was: Assigned)

Comment 4 by m...@chromium.org, Dec 29 2016

Cc: yoichio@chromium.org
Labels: M-57 Pri-1 Type-Bug-Regression
Owner: m...@chromium.org
Status: Started (was: Fixed)
Labels: -Sheriff-Chromium
Removing from the sheriff queue, since this has been triaged and assigned.
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 3 2017

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

commit c52ce18fd8f6db97c62737f0924bd61a2727c5d3
Author: miu <miu@chromium.org>
Date: Tue Jan 03 23:30:19 2017

Fix TabCaptureApiTest.FullscreenEvents flakiness

Fixes flakiness caused by a subtle distinction in document fullscreen
state change: The tabCapture.onStatusChanged API can sometimes report a
fullscreen state change before the document itself is aware the change
occurred. This is because the tabCapture API's listener reports on the
browser's fullscreen state, whereas the document is only aware of its
own internal fullscreen state (which is updated only after the browser
notifies it of a change).

The flakiness was due to the C++ side of the test being notified that
the page had entered fullscreen before the page itself was aware of it.
This caused the simulated mouse click (to toggle fullscreen mode) to be
made early, and then both the C++ and JS sides of the test became
out-of-sync and froze, each waiting on the other to do something.

This change fixes the issue by always using the document's fullscreen
change listeners, to ensure the out-of-sync condition cannot occur.
This change also "freshens up" the JavaScript code in this test, using
newer ES6 syntax.

BUG=675851

Review-Url: https://codereview.chromium.org/2604133002
Cr-Commit-Position: refs/heads/master@{#441252}

[modify] https://crrev.com/c52ce18fd8f6db97c62737f0924bd61a2727c5d3/chrome/browser/extensions/api/tab_capture/tab_capture_apitest.cc
[modify] https://crrev.com/c52ce18fd8f6db97c62737f0924bd61a2727c5d3/chrome/test/data/extensions/api_test/tab_capture/fullscreen_test.js

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 4 2017

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

commit df9fdf81fa2e0b50372d6cad7333d6135501db24
Author: samuong <samuong@chromium.org>
Date: Wed Jan 04 03:19:18 2017

Revert of Fix TabCaptureApiTest.FullscreenEvents flakiness (patchset #2 id:20001 of https://codereview.chromium.org/2604133002/ )

Reason for revert:
TabCaptureApiTest.FullscreenEvents is now timing out consistently on the Win7 Tests (dbg)(1) bot (build #56178 and later) so I don't think we're ready to re-enable it yet: https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29

Original issue's description:
> Fix TabCaptureApiTest.FullscreenEvents flakiness
>
> Fixes flakiness caused by a subtle distinction in document fullscreen
> state change: The tabCapture.onStatusChanged API can sometimes report a
> fullscreen state change before the document itself is aware the change
> occurred. This is because the tabCapture API's listener reports on the
> browser's fullscreen state, whereas the document is only aware of its
> own internal fullscreen state (which is updated only after the browser
> notifies it of a change).
>
> The flakiness was due to the C++ side of the test being notified that
> the page had entered fullscreen before the page itself was aware of it.
> This caused the simulated mouse click (to toggle fullscreen mode) to be
> made early, and then both the C++ and JS sides of the test became
> out-of-sync and froze, each waiting on the other to do something.
>
> This change fixes the issue by always using the document's fullscreen
> change listeners, to ensure the out-of-sync condition cannot occur.
> This change also "freshens up" the JavaScript code in this test, using
> newer ES6 syntax.
>
> BUG=675851
>
> Committed: https://crrev.com/c52ce18fd8f6db97c62737f0924bd61a2727c5d3
> Cr-Commit-Position: refs/heads/master@{#441252}

TBR=xjz@chromium.org,miu@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=675851

Review-Url: https://codereview.chromium.org/2611593005
Cr-Commit-Position: refs/heads/master@{#441307}

[modify] https://crrev.com/df9fdf81fa2e0b50372d6cad7333d6135501db24/chrome/browser/extensions/api/tab_capture/tab_capture_apitest.cc
[modify] https://crrev.com/df9fdf81fa2e0b50372d6cad7333d6135501db24/chrome/test/data/extensions/api_test/tab_capture/fullscreen_test.js

Cc: foolip@chromium.org
I investigated this a bit in  issue 685940  but failed to reproduce the flakiness. Since the test is disabled I won't do anything more. My planned changes in  issue 402376  might affect this test, so trying to enable it after that has been fixed might be a good idea. It need not block on that though.

Comment 9 by m...@chromium.org, May 5 2017

NextAction: 2017-06-30
Status: Assigned (was: Started)
The NextAction date has arrived: 2017-06-30

Comment 11 by m...@chromium.org, Jul 1 2017

NextAction: 2017-11-30
The NextAction date has arrived: 2017-11-30

Sign in to add a comment