This likely effects all media browser tests (both in content/ and chrome/). For now, adding the MSE component, since it shares much of the infrastructure with the EME tests.

This problem is especially bad in tests that use this JS:

  video.addEventListener('ended', Utils.failTest);

In this case the title is set to ENDED and the test passes. We should change all cases to explicitly set an ended message such as "TEST_ENDED_SUCCESS", and remove reliance on the event types (such as is done by Utils.installTitleEventHandler().

Alternatively, we could change these handlers to add a prefix to events, such as "EVENT_RECEIVED_", and then change the tests to watch for the entire string.

A final alternative is to change TitleWatcher to require a match at start instead of any substring; then we can prefix these and be certain it won't match incorrectly.

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

commit 6eb52c23c889bba614b089ca0952f419af27412e
Author: sandersd <sandersd@chromium.org>
Date: Fri Mar 04 23:03:24 2016

Do not change the case of titles in Utils.setResultInTitle().

The previous behavior causes mentions of the 'ended' event to
immediately end MSE tests (successfully). This is true even if
the actual call was to Utils.failTest().

BUG=592067

Review URL: https://codereview.chromium.org/1755403002

Cr-Commit-Position: refs/heads/master@{#379382}

[modify] https://crrev.com/6eb52c23c889bba614b089ca0952f419af27412e/chrome/browser/media/media_browsertest.cc
[modify] https://crrev.com/6eb52c23c889bba614b089ca0952f419af27412e/media/test/data/eme_player_js/utils.js

Thanks for filing. Yeah, that incorrect "failTest() --> test passed!" was ugly, and still needs further cleanup beyond #3 to help prevent similar recurrence in future; suggestions for further cleanup are in #2.

Note that after #3 there's a mismatch between the chrome/ and content/ browser tests. In chrome, kError is "error". But in content, kError is "ERROR". See  issue 675046  for more details.

sandersd: Where did you see that the TitleWatcher watches for any substring in the title? It seems to me it's actually doing an exact match [1]. I didn't try it, so it's possible I am missing something or looking at the wrong place.

[1] https://cs.chromium.org/chromium/src/content/public/test/browser_test_utils.cc?rcl=1483662145&l=1243

I have no explanation, your analysis is correct. I suppose the best plan is to try to reproduce the original issue and, assuming that doesn't work, your CL is safe.

I went back over this, and I believe I was incorrect in the original review. The full title strings were always compared, but the particular case of interest (when Utils.failTest was bound as an 'ended' event handler) set the title to exactly the event name with no prefix or suffix. (With differences in case that have changed over time.)

Any solution that avoids that should solve the problem.

Thanks for looking into this.

Maybe we should just add some prefix to make sure exactly what we are expecting (this was also suggested in the codereview by ddorwin). For example:

EVENT_ENDED  // all events should have EVENT_ prefix
MESSAGE_FAIL  // all explicit messages should have MESSAGE_ prefix
...

SGTM.

Dear sandersd, 
I am new to chromium. Interested to work on this.
Plz let me know

Submitted WIP patch. PTAL.
https://chromium-review.googlesource.com/#/c/chromium/src/+/753201 

Is it fine if i add prefix for messages on top of this change?

I'm not certain what the best order is here; it's easier to change each test now, when the constants are separate, than it will be once they are combined. The issues with these messages are subtle, and there is a high chance of having to revert a CL somewhere in the process.

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

commit 2fd333613f4913ce8b5d7c0e5c87f4ab727ab27b
Author: cm.sanchi <cm.sanchi@samsung.com>
Date: Tue Nov 14 03:20:36 2017

[media] Move test result message constants to test_data_util

Centralizing these constants is preparatory work for changing
their values (see bug).

Bug: 592067
Change-Id: Ifa9f4c5a943fc0b46c45bde18a4fef16faf2b175
Reviewed-on: https://chromium-review.googlesource.com/753201
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: srirama chandra sekhar <srirama.m@samsung.com>
Cr-Commit-Position: refs/heads/master@{#516166}
[modify] https://crrev.com/2fd333613f4913ce8b5d7c0e5c87f4ab727ab27b/chrome/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/2fd333613f4913ce8b5d7c0e5c87f4ab727ab27b/chrome/browser/media/media_browsertest.cc
[modify] https://crrev.com/2fd333613f4913ce8b5d7c0e5c87f4ab727ab27b/chrome/browser/media/media_browsertest.h
[modify] https://crrev.com/2fd333613f4913ce8b5d7c0e5c87f4ab727ab27b/content/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/2fd333613f4913ce8b5d7c0e5c87f4ab727ab27b/content/browser/media/media_browsertest.cc
[modify] https://crrev.com/2fd333613f4913ce8b5d7c0e5c87f4ab727ab27b/content/browser/media/media_browsertest.h
[modify] https://crrev.com/2fd333613f4913ce8b5d7c0e5c87f4ab727ab27b/content/browser/media/media_color_browsertest.cc
[modify] https://crrev.com/2fd333613f4913ce8b5d7c0e5c87f4ab727ab27b/content/browser/media/media_redirect_browsertest.cc
[modify] https://crrev.com/2fd333613f4913ce8b5d7c0e5c87f4ab727ab27b/content/browser/media/media_source_browsertest.cc
[modify] https://crrev.com/2fd333613f4913ce8b5d7c0e5c87f4ab727ab27b/media/base/test_data_util.cc
[modify] https://crrev.com/2fd333613f4913ce8b5d7c0e5c87f4ab727ab27b/media/base/test_data_util.h

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Closing as Fixed based on Comment #15 check-in.

I don't think the commit above was a complete fix.