Add MediaError.message

content/browser/media/media_browsertest.cc

Index: content/browser/media/media_browsertest.cc
diff --git a/content/browser/media/media_browsertest.cc b/content/browser/media/media_browsertest.cc
index 0e6650bde4b1a9f49dd8ab979868b345125a30eb..42daf094e6d97c6201b0464ac84ee1c9e9486490 100644
--- a/content/browser/media/media_browsertest.cc
+++ b/content/browser/media/media_browsertest.cc
@@ -17,6 +17,7 @@
 #include "media/base/test_data_util.h"
 #include "media/media_features.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
+#include "url/url_util.h"
 
 namespace content {
 
@@ -70,6 +71,14 @@ std::string MediaBrowserTest::RunTest(const GURL& gurl,
   return base::UTF16ToASCII(result);
 }
 
+std::string MediaBrowserTest::EncodeErrorMessage(

[mlamouri (slow - plz ping), 2017/04/12 15:56:41]

Where is this called?

[wolenetz, 2017/04/13 23:59:10]

Oops! :) Especially with newlines in the message, I'll correctly call this now
from ::RunErrorMessageTest() like I was originally intending.

Done.

+    const std::string& original_message) {
+  url::RawCanonOutputT<char> buffer;
+  url::EncodeURIComponent(original_message.data(), original_message.size(),
+                          &buffer);
+  return std::string(buffer.data(), buffer.length());
+}
+
 void MediaBrowserTest::AddTitlesToAwait(content::TitleWatcher* title_watcher) {
   title_watcher->AlsoWaitForTitle(base::ASCIIToUTF16(kEnded));
   title_watcher->AlsoWaitForTitle(base::ASCIIToUTF16(kError));
@@ -102,6 +111,17 @@ class MediaTest : public testing::WithParamInterface<bool>,
     RunMediaTestPage("player.html", query_params, kEnded, http);
   }
 
+  void TestErrorMessage(const std::string& tag,

[sandersd (OOO until July 31), 2017/04/12 01:09:37]

Rename as RunErrorMessageTest to match other variants?

[wolenetz, 2017/04/13 23:59:10]

Done.

+                        const std::string& media_file,
+                        const std::string& expected_error_substring,
+                        bool http) {
+    base::StringPairs query_params;
+    query_params.push_back(std::make_pair(tag, media_file));
+    query_params.push_back(
+        std::make_pair("error_substr", expected_error_substring));
+    RunMediaTestPage("player.html", query_params, kErrorEvent, http);
+  }
+
   void RunVideoSizeTest(const char* media_file, int width, int height) {
     std::string expected;
     expected += base::IntToString(width);
@@ -259,6 +279,26 @@ IN_PROC_BROWSER_TEST_P(MediaTest, VideoTulipWebm) {
   PlayVideo("tulip2.webm", GetParam());
 }
 
+IN_PROC_BROWSER_TEST_P(MediaTest, VideoErrorMissingResource) {
+  TestErrorMessage("video", "nonexistent_file.webm",
+                   "{\"MediaElementError\":[\"Format error\"]}", GetParam());
+}
+
+IN_PROC_BROWSER_TEST_P(MediaTest, VideoErrorEmptySrcAttribute) {
+  TestErrorMessage("video", "",
+                   "{\"MediaElementError\":[\"Empty src attribute\"]}",
+                   GetParam());
+}
+
+IN_PROC_BROWSER_TEST_P(MediaTest, VideoNoSupportedStreams) {
+  TestErrorMessage("video", "no_streams.webm",
+                   "{\"MEDIA_ERROR_LOG_ENTRY\":[\"{\\\"error\\\":"
+                   "\\\"FFmpegDemuxer: no supported "
+                   "streams\\\"}\"],\"PIPELINE_ERROR\":[\"demuxer: no "
+                   "supported streams\"]}",

[sandersd (OOO until July 31), 2017/04/12 01:09:37]

This format seems overly complex and exposes rather more internal detail than I
was hoping. The object structure here is:

{
  "MEDIA_ERROR_LOG_ENTRY": [ "{\"error\":\"FFmpegDemuxer: no supported
streams\"}" ],
  "PIPELINE_ERROR": [ "demuxer: no supported streams" ]
}

I would propose:

{
  "cause": "demuxer: no supported streams",
  "error_log": [ "FFmpegDemuxer: no supported streams" ]
}

[wolenetz, 2017/04/13 23:59:10]

Done. See related discussion thread
https://codereview.chromium.org/2660003003/#msg28

+                   GetParam());
+}
+
 // Covers tear-down when navigating away as opposed to browser exiting.
 IN_PROC_BROWSER_TEST_F(MediaTest, Navigate) {
   PlayVideo("bear.webm", false);