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

Issue 839288 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Remove internals.setMediaElementNetworkState()

Project Member Reported by mlamouri@chromium.org, May 3 2018

Issue description

ClusterFuzz keeps calling this method and it's a footgun meant to be used for tests. It's used fairly rarely (3 times, 2 tests) so we may want to invest time to remove it so ClusterFuzz don't blow up another leg.
 
Cc: -dalecur...@chromium.org mlamouri@chromium.org
Owner: dalecur...@chromium.org
Status: Started (was: Available)
Dale, do you have a plan for this? I was planning to have a look given that one test I wrote and the other one is a media controls test. Happy for you to deal with this though :)
Ignore that comment. For some reasons, I missed the email for the review :)
Project Member

Comment 4 by bugdroid1@chromium.org, May 9 2018

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

commit d66554b28a0a09e3a96eb4554eb35ece58a3327f
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Wed May 09 19:44:33 2018

Remove Internals::setMediaElementNetworkState(); always trouble.

ClusterFuzz routinely uses this to create fake bugs and there's only
three tests which use this:
- controls-repaint-for-network-change.html, which is a broken test and
tests nothing (i.e. breaking it still passes)
- media-play-promise.html #playDecodeError, which doesn't need to; it
can just use a real corrupt file.
- media-play-promise.html #playNetworkError, which is also a broken
test and with the addition of a true test reveals that the underlying
behavior being tested is underspecified. Filed http://crbug.com/841063

Code has been added to test network errors via MSEs ability to force a
network error on EOS, but as mentioned the underlying behavior being
tested by the test is broken. For now the test is commented out.

BUG= 839288 , 841063
TEST=existing tests pass.

Change-Id: I0c48f305d24760c6bace7113297423b44e93728b
Reviewed-on: https://chromium-review.googlesource.com/1043493
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557284}
[delete] https://crrev.com/8aa1e54c131a3117043ea16af89340f99497c6ec/third_party/WebKit/LayoutTests/media/controls-repaint-for-network-change-expected.html
[delete] https://crrev.com/8aa1e54c131a3117043ea16af89340f99497c6ec/third_party/WebKit/LayoutTests/media/controls-repaint-for-network-change.html
[modify] https://crrev.com/d66554b28a0a09e3a96eb4554eb35ece58a3327f/third_party/WebKit/LayoutTests/media/media-play-promise-expected.txt
[modify] https://crrev.com/d66554b28a0a09e3a96eb4554eb35ece58a3327f/third_party/WebKit/LayoutTests/media/media-play-promise.html
[add] https://crrev.com/d66554b28a0a09e3a96eb4554eb35ece58a3327f/third_party/WebKit/LayoutTests/media/network-error.js
[modify] https://crrev.com/d66554b28a0a09e3a96eb4554eb35ece58a3327f/third_party/blink/renderer/core/testing/internals.cc
[modify] https://crrev.com/d66554b28a0a09e3a96eb4554eb35ece58a3327f/third_party/blink/renderer/core/testing/internals.h
[modify] https://crrev.com/d66554b28a0a09e3a96eb4554eb35ece58a3327f/third_party/blink/renderer/core/testing/internals.idl

Status: Fixed (was: Started)

Sign in to add a comment