Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(618)

Issue 2837133002: To M59: Add MediaError.message (Closed)

Created:
3 years, 8 months ago by wolenetz
Modified:
3 years, 8 months ago
CC:
chromium-reviews
Target Ref:
refs/branch-heads/3071
Project:
chromium
Visibility:
Public.

Description

To M59: Add MediaError.message This improves the level of information available for web authors to use when debugging media playback errors. * Adds support for the MediaError.message field [1] [2] * Changes MediaLog::PipelineStatusToString() to return just a stringified version of the enum identifier, suitable for use as a UA-specific-code when included in MediaError.message. * Changes MediaLog::GetErrorMessage() to produce messages in the form: [[Stringified pipeline error code, if any, without any colon][: The first MEDIA_ERROR_LOG_ENTRY from the log, if any, since that is most likely to contain the most precise error information]] The produced message does not contain any newlines. See the updated media_browsertest.cc for basic examples. * When the message would otherwise be empty for various cases in HTMLMediaElement, and more meaningful information beyond the basic standard MediaError codes and HTMLMediaElement networkStates is available, reports basic additional information (prefixed by error code "MEDIA_ELEMENT_ERROR: ".) This should assist edge case differentiation. See the updated media_browsertest.cc for basic examples. * Test coverage: * Adds basic layout test coverage test similar to that added in [3]. * Adds basic content browser test coverage for a small variety of errors, since these messages' format and content are specific to Chrom*. The format of the message string may change in future (for example, see discussion in [4]). Also, the variety of error codes in the message prior to the first ':' will likely change in future, and the descriptive error message detail following the ':' will likely change to include more cases or more consistent format in future. [1] - https://github.com/whatwg/html/issues/2085 [2] - https://github.com/whatwg/html/pull/2086 [3] - https://github.com/w3c/web-platform-tests/pull/4638 [4] - https://github.com/whatwg/html/issues/2531 [Intent to implement and ship] - https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/yDIyWozG1xk BUG=601086, 710617 Review-Url: https://codereview.chromium.org/2660003003 Cr-Commit-Position: refs/heads/master@{#466359} (cherry picked from commit ed8e7091b5b0a51a8c0781f95813a5929c1f8110) TBR=jochen@chromium.org,sandersd@chromium.org,mlamouri@chromium.org,mcasas@chromium.org,dalecurtis@chromium.org,foolip@chromium.org Review-Url: https://codereview.chromium.org/2837133002 . Cr-Commit-Position: refs/branch-heads/3071@{#173} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} Committed: https://chromium.googlesource.com/chromium/src/+/e9ea56b10a39d0ec3c0fa81962d6cb0702020586

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+277 lines, -87 lines) Patch
M content/browser/media/media_browsertest.h View 1 chunk +5 lines, -0 lines 0 comments Download
M content/browser/media/media_browsertest.cc View 4 chunks +37 lines, -0 lines 0 comments Download
M content/renderer/media/render_media_log.h View 3 chunks +8 lines, -4 lines 0 comments Download
M content/renderer/media/render_media_log.cc View 2 chunks +19 lines, -11 lines 0 comments Download
M content/renderer/media/webmediaplayer_ms.h View 1 chunk +1 line, -1 line 0 comments Download
M content/renderer/media/webmediaplayer_ms.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M content/renderer/media_capture_from_element/html_video_element_capturer_source_unittest.cc View 1 chunk +4 lines, -1 line 0 comments Download
M media/base/media_log.h View 1 chunk +16 lines, -2 lines 0 comments Download
M media/base/media_log.cc View 4 chunks +48 lines, -35 lines 0 comments Download
M media/blink/webmediaplayer_impl.h View 1 chunk +1 line, -1 line 0 comments Download
M media/blink/webmediaplayer_impl.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M media/test/data/player.html View 6 chunks +43 lines, -4 lines 0 comments Download
M third_party/WebKit/LayoutTests/media/video-error-does-not-exist.html View 2 chunks +4 lines, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/virtual/stable/webexposed/global-interface-listing-expected.txt View 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt View 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt View 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt View 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLMediaElement.h View 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLMediaElement.cpp View 10 chunks +55 lines, -15 lines 0 comments Download
M third_party/WebKit/Source/core/html/media/MediaError.h View 2 chunks +14 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/html/media/MediaError.idl View 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/testing/EmptyWebMediaPlayer.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/platform/testing/EmptyWebMediaPlayer.cpp View 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/public/platform/WebMediaPlayer.h View 1 chunk +6 lines, -1 line 0 comments Download

Messages

Total messages: 2 (1 generated)
wolenetz
3 years, 8 months ago (2017-04-24 20:21:01 UTC) #2
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as
e9ea56b10a39d0ec3c0fa81962d6cb0702020586.

Powered by Google App Engine
This is Rietveld 408576698