New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 898230 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Increase test coverage for PiP

Project Member Reported by denizz@google.com, Oct 23

Issue description

Summary of action item:

When Picture-in-Picture Web API shipped in M70, we ran into a major usability issue specifically with Mac Os where controls were unpredictable and disappearing/appearing inconsistently (see crbug/897246 for more details, repro steps and fixes landed). We were able to land fixes and respin M70 stable. However, going forward we'd like to increase test coverage to identify and resolve such issues prior to launch on all applicable platforms. 

 
More specifically, let's add a step to do a manual test pass across CUJs on Beta channel to get an accurate representation of merges prior to shipping a feature. 
Cc: cliffordcheng@chromium.org
We've been working with cliffordcheng@ on this.
We only have covered that for Linux now.
Mac will be added once we land this CL.
https://chromium-review.googlesource.com/c/chromium/src/+/1296811

Doug, maybe we can ask David to also cover it until Windows and Mac are fully covered.
It shouldn't take too much of his time (maybe 30 mins?)
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 8

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

commit 0cfecb52487066ad0cc55ccf18635e6d28bd9b5d
Author: Clifford Cheng <cliffordcheng@chromium.org>
Date: Thu Nov 08 18:50:38 2018

[Media UX] Enable the pixel by pixel comparison test for Mac OS.

1. Tested on Mac and verify everything works as expected.
2. Removed the unnecessary call of GetNativeWindow().
3. The RGB for GPU enabled and GPU disabled test images is slightly different. Additional check is needed.

Bug: 898230
Change-Id: I85bf2450862361572597632159de60db6abf2c22
Reviewed-on: https://chromium-review.googlesource.com/c/1307062
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Clifford Cheng <cliffordcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606552}
[modify] https://crrev.com/0cfecb52487066ad0cc55ccf18635e6d28bd9b5d/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 8

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

commit ba0d48f8b3e768e554f6b30d7ab04e3f0e907e61
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Thu Nov 08 23:52:22 2018

Revert "[Media UX] Enable the pixel by pixel comparison test for Mac OS."

This reverts commit 0cfecb52487066ad0cc55ccf18635e6d28bd9b5d.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 606552 as the
culprit for flakes in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vMGNmZWNiNTI0ODcwNjZhZDBjYzU1Y2NmMTg2MzVlNmQyOGJkOWI1ZAw

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.mac/Mac10.12%20Tests/16586

Sample Failed Step: network_service_browser_tests on Intel GPU on Mac on Mac-10.12.6

Sample Flaky Test: PictureInPicturePixelComparisonBrowserTest.VideoPlay

Original change's description:
> [Media UX] Enable the pixel by pixel comparison test for Mac OS.
> 
> 1. Tested on Mac and verify everything works as expected.
> 2. Removed the unnecessary call of GetNativeWindow().
> 3. The RGB for GPU enabled and GPU disabled test images is slightly different. Additional check is needed.
> 
> Bug: 898230
> Change-Id: I85bf2450862361572597632159de60db6abf2c22
> Reviewed-on: https://chromium-review.googlesource.com/c/1307062
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Commit-Queue: Clifford Cheng <cliffordcheng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#606552}

Change-Id: I10a133230cad3256bf4e5e0c3c925920484ce6a1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 898230,  903580 
Reviewed-on: https://chromium-review.googlesource.com/c/1327612
Cr-Commit-Position: refs/heads/master@{#606647}
[modify] https://crrev.com/ba0d48f8b3e768e554f6b30d7ab04e3f0e907e61/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 13

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

commit bf310bd3ce97495602e136d25826dd2b68b7234b
Author: Clifford Cheng <cliffordcheng@chromium.org>
Date: Tue Nov 13 23:00:56 2018

[Media UX] Enable the pixel by pixel comparison test for Mac OS.

1. Tested on Mac and verify everything works as expected.
2. Removed the unnecessary call of GetNativeWindow().
3. Added a flag to disable GPU for the test. GPU makes an slight impact on image conversion and the output image color.
   It is turned off to prevent noise.

Bug: 898230
Change-Id: If69a900a2e1373075f58c80a2aaef526d856bb08
Reviewed-on: https://chromium-review.googlesource.com/c/1330297
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Clifford Cheng <cliffordcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607794}
[modify] https://crrev.com/bf310bd3ce97495602e136d25826dd2b68b7234b/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 30

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

commit cd7f1f8e6b94495acc668869789bcae171bc0d77
Author: Clifford Cheng <cliffordcheng@chromium.org>
Date: Fri Nov 30 18:55:33 2018

Enable pixel-by-pixel comparison test for Windows.

Fixed all the compile and runtime issues on Windows and enable the test on it.

Bug: 898230
Change-Id: I9e92e49583893898603bffa46f6fc6bdc1ef7e20
Reviewed-on: https://chromium-review.googlesource.com/c/1339240
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Clifford Cheng <cliffordcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612714}
[modify] https://crrev.com/cd7f1f8e6b94495acc668869789bcae171bc0d77/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc

This bug had an unsupported status. Updating to Untriaged so someone will reevaluate.
Status: Untriaged (was: Accepted)
Components: Test
Labels: Pri-2
Status: Assigned (was: Untriaged)
This issue has an owner, a component and a priority, but is still listed as untriaged or unconfirmed. By definition, this bug is triaged. Changing status to "assigned". Please reach out to me if you disagree with how I've done this.

Sign in to add a comment