Add MediaError.message

third_party/WebKit/Source/core/html/media/MediaError.h

 /*
  * Copyright (C) 2007 Apple Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 11 matching lines @@
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #ifndef MediaError_h
 #define MediaError_h
 
 #include "bindings/core/v8/ScriptWrappable.h"
 #include "core/CoreExport.h"
 #include "platform/heap/Handle.h"
+#include "wtf/text/WTFString.h"
 
 namespace blink {
 
-class CORE_EXPORT MediaError final : public GarbageCollected<MediaError>,
-                                     public ScriptWrappable {
+class JSONObject;
+
+class CORE_EXPORT MediaError final
+    : public GarbageCollectedFinalized<MediaError>,
+      public ScriptWrappable {
   DEFINE_WRAPPERTYPEINFO();
 
  public:
   enum ErrorCode {
     kMediaErrAborted = 1,
     kMediaErrNetwork,
     kMediaErrDecode,
     kMediaErrSrcNotSupported,
   };
 
-  static MediaError* Create(ErrorCode code) { return new MediaError(code); }
+  static MediaError* Create(ErrorCode code, const JSONObject* messages) {
+    return new MediaError(code, messages);
+  }
 
   ErrorCode code() const { return code_; }
 
+  // Returns either an empty string, or a serialized JSON object describing
+  // additional error messages. Each object property is named as an error type,
+  // with value being an array of error messages of that type.
+  String message() const { return message_; }

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

That's quite unexpected: why are we passing a JSONObject? Usually error messages
are simple strings. Returning a serialised JSON object seems like slightly
working around the spec. Could we have a `messages` IDL instead or simply return
the most recent error? How likely it is that the most recent error would be
noise?

[foolip, 2017/04/12 17:21:52]

Yes, if we usually return a string that can be passed to JSON.parse(), then
people may start doing just that. If exceptions are thrown in other browsers as
a result, there would be frowning.

How much structure is there to this error message, is it feasible to just format
the JSON into something more human readable and less machine readable?

If returning an arbitrary object is very valuable, then we should revisit the
spec and have this return a dictionary or similar instead.

[wolenetz, 2017/04/12 17:29:57]

The format and content of the "message" attribute is implementation-specific.
Considering the current state of our media-internals error logging mechanisms,
and allowing for future extensibility/modification and easy parsing by web apps
of Chrome's message details, I chose a serialized JSON object rather than just
some weird concatenation of strings.

In the Chromium-side implementation, we hold onto just 1 PIPELINE_ERROR (at
most), and potentially multiple MEDIA_LOG(ERROR) event logs. The reason for the
latter is that, for example, an MSE stream parser might first detect "parse
failure" with some detailed MEDIA_LOG(ERROR) emission describing the details of
the parse failure, and when this failure bubbles up into the demuxer, the
pipeline, and eventually the webmediaplayer implementation, additional higher
level MEDIA_LOG(ERROR) emissions may also occur. These later emissions are
typically more general, so preserving the series of logs allows the greatest
exposure of error logging detail to help identify the root cause.

None of this was an attempt to force a message structure or content into the
spec or other implementations, rather to expose the information that we may have
to assist debugging errors that occur when using our implementation. It may well
result that other implementations (like Edge) might have completely different,
though just as valid for this use case (identifying implementation-specific
error details to assist media error debugging), format and content of their
errors (such as perhaps just stringifying the msExtendedCode value that they
already have in Edge).

See also related discussion on the spec issue
(https://github.com/whatwg/html/issues/2085)

[foolip, 2017/04/12 17:40:40]

Can you share an example of what one of the more verbose error messages will
look like? Is it just a list of messages concatenated, or is there real
structure/nesting to the serialized object?

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

Per discussion threads, redone as a human-readable, newline-delimited single
string. There's also spec discussion to, instead of newline-delimited single
string, use an array of strings (https://github.com/whatwg/html/issues/2531),
though that is getting push-back from at least Firefox initially.

+
   DEFINE_INLINE_TRACE() {}
 
  private:
-  MediaError(ErrorCode code) : code_(code) {}
+  MediaError(ErrorCode, const JSONObject*);
 
   ErrorCode code_;
+  String message_;
 };
 
 }  // namespace blink
 
 #endif  // MediaError_h