Add UMA to report requests for key systems using RequestMediaKeyAccess

media/blink/webencryptedmediaclient_impl.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "webencryptedmediaclient_impl.h"
 
 #include "base/logging.h"
+#include "base/metrics/histogram.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "media/base/key_systems.h"
 #include "net/base/mime_util.h"
 #include "third_party/WebKit/public/platform/WebEncryptedMediaRequest.h"
 #include "third_party/WebKit/public/platform/WebMediaKeySystemConfiguration.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/platform/WebVector.h"
 #include "webcontentdecryptionmoduleaccess_impl.h"
 
 namespace media {
 
+// These names are used by UMA.
+const char kKeySystemSupportUMAPrefix[] =
+    "Media.EME.RequestMediaKeySystemAccess.";
+const char kClearKeyKeySystemNameForUMA[] = "ClearKey";
+const char kUnknownKeySystemNameForUMA[] = "Unknown";
+const char kWidevineKeySystemNameForUMA[] = "Widevine";

[xhwang, 2015/01/29 05:23:26]

Can you use GetKeySystemNameForUMA(const std::string& key_system) in
key_systems.h? Usually we don't know anything about "Widevine" in media/ code.
(Android code is an exception, sadly...)

[jrummell, 2015/01/29 22:16:06]

My concern was that histograms.xml needs to know about them, so having a fixed
list that matches up with what is there makes sense. However, switching to lazy
create the reporters.

+
 static bool IsSupportedContentType(
     const std::string& key_system,
     const std::string& mime_type,
     const std::string& codecs) {
   // Per RFC 6838, "Both top-level type and subtype names are case-insensitive."
   // TODO(sandersd): Check that |container| matches the capability:
   //   - audioCapabilitys: audio/mp4 or audio/webm.
   //   - videoCapabilitys: video/mp4 or video/webm.
   // http://crbug.com/429781.
   std::string container = base::StringToLowerASCII(mime_type);
@@ skipping 93 matching lines @@
     accumulated_configuration->videoCapabilities = video_capabilities;
   }
 
   // TODO(sandersd): Prompt for distinctive identifiers and/or persistent state
   // if required. Make sure that future checks are silent.
   // http://crbug.com/446263.
 
   return true;
 }
 
+// Report usage of key system to UMA. There are 2 different counts logged:
+// 1. The key system is requested.
+// 2. The requested key system and options are supported.
+// Each stat is only reported once per renderer process per key system.
+// Note that WebEncryptedMediaClientImpl is only created once by each
+// renderer process.

[xhwang, 2015/01/29 05:23:25]

hmm, this is not true. WebEncryptedMediaClientImpl is created once per
RenderFrame(Impl). There could be multiple render frames (which belong to the
same tab, or different tabs) in one render process.

[jrummell, 2015/01/29 22:16:06]

As discussed offline, once per render frame is OK. Changed the comment.

+class WebEncryptedMediaClientImpl::Reporter {

[xhwang, 2015/01/29 05:23:26]

KeySystemsSupportUMA::Reporter is currently using 4 OneTimeReporter. If we move
WebEncryptedMediaClientImpl::Reporter to a standalone file/class, then
KeySystemsSupportUMA::Reporter can use it and it'll only need two Reporters. But
we probably don't want to touch the prefixed code....

[jrummell, 2015/01/29 22:16:07]

Acknowledged.

+ public:
+  enum KeySystemSupportStatus {
+    KEY_SYSTEM_REQUESTED = 0,
+    KEY_SYSTEM_SUPPORTED = 1,
+    KEY_SYSTEM_COUNT

[xhwang, 2015/01/29 05:23:26]

KEY_SYSTEM_COUNT is misleading, it's really KEY_SYSTEM_SUPPORT_STATUS_COUNT

[jrummell, 2015/01/29 22:16:06]

Done.

+  };
+
+  explicit Reporter(const std::string& key_system_for_uma)
+      : uma_name_(kKeySystemSupportUMAPrefix + key_system_for_uma),
+        is_requested_reported_(false),
+        is_supported_reported_(false) {}

[xhwang, 2015/01/29 05:23:26]

s/requested/request
s/supported/support

[jrummell, 2015/01/29 22:16:07]

Done.

+  ~Reporter() {}
+
+  void ReportRequested() {
+    if (is_requested_reported_)
+      return;
+    Report(KEY_SYSTEM_REQUESTED);
+    is_requested_reported_ = true;
+  }
+
+  void ReportSupported() {

[xhwang, 2015/01/29 05:23:26]

DCHECK(is_requested_reported_) to make sure this is never called without calling
ReportRequested() first.

[jrummell, 2015/01/29 22:16:07]

Done.

+    if (is_supported_reported_)
+      return;
+    Report(KEY_SYSTEM_SUPPORTED);
+    is_supported_reported_ = true;
+  }
+
+ private:
+  void Report(KeySystemSupportStatus status) {
+    // Not using UMA_HISTOGRAM_ENUMERATION directly because UMA_* macros
+    // require the names to be constant throughout the process' lifetime.
+    base::LinearHistogram::FactoryGet(
+        uma_name_, 1, KEY_SYSTEM_COUNT, KEY_SYSTEM_COUNT + 1,
+        base::Histogram::kUmaTargetedHistogramFlag)->Add(status);
+  }
+
+  const std::string uma_name_;
+  bool is_requested_reported_;
+  bool is_supported_reported_;
+};
+
 WebEncryptedMediaClientImpl::WebEncryptedMediaClientImpl(
     scoped_ptr<CdmFactory> cdm_factory)
     : cdm_factory_(cdm_factory.Pass()) {
+  // Generate the possible set of UMA reporters. If this list is updated
+  // then the appropriate list in histograms.xml needs to be updated.
+  AddReporter(kClearKeyKeySystemNameForUMA);
+  AddReporter(kUnknownKeySystemNameForUMA);
+  AddReporter(kWidevineKeySystemNameForUMA);

[xhwang, 2015/01/29 05:23:26]

See comment below about lazy adding these.

[jrummell, 2015/01/29 22:16:07]

Done.

 }
 
 WebEncryptedMediaClientImpl::~WebEncryptedMediaClientImpl() {
 }
 
 void WebEncryptedMediaClientImpl::requestMediaKeySystemAccess(
     blink::WebEncryptedMediaRequest request) {
   // TODO(jrummell): This should be asynchronous.
 
   // Continued from requestMediaKeySystemAccess(), step 7, from
   // https://w3c.github.io/encrypted-media/#requestmediakeysystemaccess
   //
   // 7.1 If keySystem is not one of the Key Systems supported by the user
   //     agent, reject promise with with a new DOMException whose name is
   //     NotSupportedError. String comparison is case-sensitive.
   if (!base::IsStringASCII(request.keySystem())) {
     request.requestNotSupported("Only ASCII keySystems are supported");
     return;
   }
 
   std::string key_system = base::UTF16ToASCII(request.keySystem());
+
+  // Report this request to the appropriate Reporter.
+  Reporter* reporter = GetReporter(key_system);
+  reporter->ReportRequested();
+
   if (!IsConcreteSupportedKeySystem(key_system)) {
     request.requestNotSupported("Unsupported keySystem");
     return;
   }
 
   // 7.2 Let implementation be the implementation of keySystem.
   // 7.3 Follow the steps for the first matching condition from the following
   //     list:
   //       - If supportedConfigurations was not provided, run the Is Key System
   //         Supported? algorithm and if successful, resolve promise with access
   //         and abort these steps.
   // TODO(sandersd): Remove pending the resolution of
   // https://github.com/w3c/encrypted-media/issues/1.
   const blink::WebVector<blink::WebMediaKeySystemConfiguration>&
       configurations = request.supportedConfigurations();
   if (configurations.isEmpty()) {
     request.requestSucceeded(WebContentDecryptionModuleAccessImpl::Create(
         request.keySystem(), request.securityOrigin(), cdm_factory_.get()));
+    reporter->ReportSupported();

[xhwang, 2015/01/29 05:23:26]

nit: report first, then resolve the request.

[jrummell, 2015/01/29 22:16:06]

Done.

     return;
   }
 
   //       - Otherwise, for each value in supportedConfigurations, run the
   //         GetSuppored Configuration algorithm and if successful, resolve
   //         promise with access and abort these steps.
   for (size_t i = 0; i < configurations.size(); i++) {
     const blink::WebMediaKeySystemConfiguration& candidate = configurations[i];
     blink::WebMediaKeySystemConfiguration accumulated_configuration;
     if (GetSupportedConfiguration(key_system, candidate,
                                   request.securityOrigin(),
                                   &accumulated_configuration)) {
       // TODO(sandersd): Pass the accumulated configuration along.
       // http://crbug.com/447059.
       request.requestSucceeded(WebContentDecryptionModuleAccessImpl::Create(
           request.keySystem(), request.securityOrigin(), cdm_factory_.get()));
+      reporter->ReportSupported();

[xhwang, 2015/01/29 05:23:26]

ditto.

[jrummell, 2015/01/29 22:16:06]

Done.

       return;
     }
   }
 
   // 7.4 Reject promise with a new DOMException whose name is NotSupportedError.
   request.requestNotSupported(
       "None of the requested configurations were supported.");
 }
 
+void WebEncryptedMediaClientImpl::AddReporter(const std::string& key_system) {
+  reporters_.set(key_system, scoped_ptr<Reporter>(new Reporter(key_system)));

[xhwang, 2015/01/29 05:23:26]

nit: You can use make_scoped_ptr.

[jrummell, 2015/01/29 22:16:06]

Not needed now.

+}
+
+WebEncryptedMediaClientImpl::Reporter* WebEncryptedMediaClientImpl::GetReporter(
+    const std::string& key_system) {
+  Reporters::iterator reporter_lookup =
+      reporters_.find(GetKeySystemNameForUMA(key_system));
+
+  if (reporter_lookup == reporters_.end()) {
+    // All valid names should have been registered. Since we can't find one,
+    // assume |key_system| becomes "Unknown".
+    NOTREACHED() << "Unable to locate Reporter for " << key_system;
+    reporter_lookup = reporters_.find(kUnknownKeySystemNameForUMA);

[xhwang, 2015/01/29 05:23:25]

If you use GetKeySystemNameForUMA(), you'll get kUnknownKeySystemNameForUMA
automatically if the key system is not recognized. So really you have only a
small set of possible key system for UMA names. With that, you can probably lazy
create reporters in GetReporter(), instead of proactively create all reporters
in the constructor.

[jrummell, 2015/01/29 22:16:07]

Done.

+  }
+
+  return reporter_lookup->second;
+}
+
 }  // namespace media