New issue
Advanced search Search tips

Issue 783301 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: ----



Sign in to add a comment

imagecapture/MediaStreamTrack-getSettings.html in webkit_layout_tests failing on chromium.win/Win7 Tests (dbg)(1)

Project Member Reported by yzshen@chromium.org, Nov 9 2017

Issue description

imagecapture/MediaStreamTrack-getSettings.html in webkit_layout_tests failing on chromium.win/Win7 Tests (dbg)(1)

Builders failed on: 
- Win7 Tests (dbg)(1): 
  https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29


The log showed:
imagecapture/MediaStreamTrack-getSettings.html failed unexpectedly (asserts failed)

I didn't find any CL to blame. Miguel, would you please help to triage this bug? Thanks!


 

Comment 1 by tasak@google.com, Nov 10 2017

I think, this is caused by the test's flakiness. Looking at the code, it has:

---
  // |videoTrack|s settings retrieval, just like the actual capture, is a
  // process kicked right after creation, we introduce a small delay to
  // allow for those to be collected.
  await new Promise(resolve => setTimeout(resolve, 100));
---

The test waits 100msec and expects that MediaTrackSettings has been already updated.

So... I tested the following patch locally:

-  await new Promise(resolve => setTimeout(resolve, 100));
+  await new Promise(resolve => setTimeout(resolve, 500));

and ran out/Debug/content_shell --run-layout-test third_party/WebKit/LayoutTests/imagecapture/MediaStream-getSettings.html:

----
  Content-Type: text/plain
  This is a testharness.js-based test.
  PASS exercises MediaStreamTrack.getSettings()
  Harness: the test ran to completion.
----

I reverted the above hacky patch and applied the following:

--- a/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
+++ b/third_party/WebKit/Source/modules/imagecapture/ImageCapture.cpp
@@ -556,6 +556,7 @@ void ImageCapture::ClearMediaTrackConstraints() {
 
 void ImageCapture::GetMediaTrackSettings(MediaTrackSettings& settings) const {
   // Merge any present |settings_| members into |settings|.
+  fprintf(stderr, "ImageCapture::GetMediaTrackSettings(%p)\n", this);
 
   if (settings_.hasWhiteBalanceMode())
     settings.setWhiteBalanceMode(settings_.whiteBalanceMode());
@@ -702,6 +703,8 @@ void ImageCapture::OnMojoTakePhoto(ScriptPromiseResolver* resolver,
 
 void ImageCapture::UpdateMediaTrackCapabilities(
     media::mojom::blink::PhotoStatePtr photo_state) {
+  fprintf(stderr, "ImageCapture::UpdateMediaTrackCapabilities(%p, %d)\n",
+          this, !photo_state);
   if (!photo_state)
     return;

So,
---
  #READY

  ....
  ImageCapture::GetMediaTrackSettings(0x1ca56443e0a0)
  ImageCapture::UpdateMediaTrackCapabilities(0x1ca56443e0a0, 0)
  Content-Type: text/plain
  This is a testharness.js-based test.
  FAIL exercises MediaStreamTrack.getSettings() assert_equals: whiteBalanceMode expected (string) 
  "continuous" but got (undefined) undefined
  Harness: the test ran to completion.
---

  ImageCapture::GetMediaTrackSettings(0x1ca56443e0a0) ==> returns undefined
  ImageCapture::UpdateMediaTrackCapabilities(0x1ca56443e0a0, 0) ==> sets "continuous"

... we should mark the test as flaky.

Comment 2 by tasak@google.com, Nov 10 2017

imagecapture/MediaStreamTrack-getCapabilities.html has the same issue.

https://chromium-review.googlesource.com/c/chromium/src/+/766189 is in the CQ to mark these tests as flaky.
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 13 2017

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

commit bfd292e6512ec25b8640723eb8abe1702c414512
Author: Colin Blundell <blundell@chromium.org>
Date: Mon Nov 13 14:06:26 2017

Mark certain imagecapture/MediaStreamTrack tests as flaky

These tests are exhibiting flake on the main waterfall, as detailed on
the bugs listed below.

TBR=mcasas@chromium.org

Bug: 783301, 783800
Change-Id: I0770da4c7e7caf62ba65b3939f7417df7782ae36
Reviewed-on: https://chromium-review.googlesource.com/766189
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515942}
[modify] https://crrev.com/bfd292e6512ec25b8640723eb8abe1702c414512/third_party/WebKit/LayoutTests/TestExpectations

Labels: -Sheriff-Chromium
Labels: Pri-2
Setting defect without priority to Pri-2.

Sign in to add a comment