Implement robustness strings in requestMediaKeySystemAccess().

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/bind.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_util.h"
@@ skipping 13 matching lines @@
 // These names are used by UMA.
 const char kKeySystemSupportUMAPrefix[] =
     "Media.EME.RequestMediaKeySystemAccess.";
 
 enum ConfigurationSupport {
   CONFIGURATION_NOT_SUPPORTED,
   CONFIGURATION_REQUIRES_PERMISSION,
   CONFIGURATION_SUPPORTED,
 };
 
+// Accumulates configuration rules to determine if a feature (additional
+// configuration rule) can be added to an accumulated configuration.
+class ConfigState {
+ public:
+  ConfigState(bool was_permission_requested, bool is_permission_granted)
+      : was_permission_requested_(was_permission_requested),
+        is_permission_granted_(is_permission_granted),
+        is_identifier_required_(false),
+        is_identifier_recommended_(false){
+  }
+
+  bool IsPermissionGranted() const {
+    return is_permission_granted_;
+  }
+
+  // Permission is possible if it has not been denied.
+  bool IsPermissionPossible() const {
+    return is_permission_granted_ || !was_permission_requested_;
+  }
+
+  bool IsIdentifierRequired() const {
+    return is_identifier_required_;
+  }
+
+  bool IsIdentifierRecommended() const {
+    return is_identifier_recommended_;
+  }
+
+  // Checks whether a rule is compatible with all previously added rules.
+  bool IsRuleSupported(EmeConfigRule rule) const {
+    switch (rule) {
+      case EmeConfigRule::NOT_SUPPORTED:
+        return false;
+      case EmeConfigRule::IDENTIFIER_REQUIRED:
+        return IsPermissionPossible();
+      case EmeConfigRule::IDENTIFIER_RECOMMENDED:
+        return true;
+      case EmeConfigRule::SUPPORTED:
+        return true;
+    }
+    NOTREACHED();
+    return false;
+  }
+
+  // Checks whether a rule is compatible with all previously added rules, and
+  // can be accepted without needing to add it to the configuration state. This
+  // allows considering more rules after the configuration state is final (that
+  // is, after distinctiveIdentifier has been resolved).
+  bool IsRuleSupportedWithCurrentState(EmeConfigRule rule) const {

[ddorwin, 2015/03/19 18:08:35]

"CurrentState" implies that the other method (above) does not check the current
state.  Both check is_permission_granted_. The only difference is that this one
ignores was_permission_requested_.

It seems that perhaps this one is ...WithoutRequestingPermission.

[sandersd (OOO until July 31), 2015/03/19 20:05:09]

I am trying to remove all unnecessary references to 'permission'. The goal here
is to express that if a rule passes this test, then you can add the associated
option to the accumulated config without adding the rule to the config_state and
the two will still be in sync.

+    switch (rule) {
+      case EmeConfigRule::NOT_SUPPORTED:
+        return false;
+      case EmeConfigRule::IDENTIFIER_REQUIRED:
+        return is_permission_granted_;
+      case EmeConfigRule::IDENTIFIER_RECOMMENDED:
+        return true;
+      case EmeConfigRule::SUPPORTED:
+        return true;
+    }
+    NOTREACHED();
+    return false;
+  }
+
+  // Add a rule to the accumulated configuration state.
+  void AddRule(EmeConfigRule rule) {
+    switch (rule) {
+      case EmeConfigRule::NOT_SUPPORTED:
+        return;
+      case EmeConfigRule::IDENTIFIER_REQUIRED:
+        is_identifier_required_ = true;
+        return;
+      case EmeConfigRule::IDENTIFIER_RECOMMENDED:
+        is_identifier_recommended_ = true;
+        return;
+      case EmeConfigRule::SUPPORTED:
+        return;
+    }
+    NOTREACHED();
+  }
+
+ private:
+  // Was a permission to usa a distinctive identifier requested?

[ddorwin, 2015/03/19 18:08:35]

nit: Ending in '?' is a bit weird. Perhaps "Whether... was requested." or
"Whether a request... has been made."

Ditto below, especially "Do we..."

[sandersd (OOO until July 31), 2015/03/19 20:05:09]

Done.

+  bool was_permission_requested_;

[ddorwin, 2015/03/19 18:08:35]

These first two can be const. That might help readability too.

[sandersd (OOO until July 31), 2015/03/19 20:05:09]

Done.

+
+  // Do we know that permission to use a distinctive identifier has been
+  // granted?
+  bool is_permission_granted_;
+
+  // Have we added a configuration option that requires a distinctive
+  // identifier?
+  bool is_identifier_required_;
+
+  // Have we added a configuration option that recommends a distinctive
+  // identifier?
+  bool is_identifier_recommended_;
+};
+
+static EmeRobustness ConvertRobustness(const blink::WebString& robustness) {
+  if (robustness.isEmpty())
+    return EmeRobustness::EMPTY;
+  if (robustness == "SW_SECURE_CRYPTO")
+    return EmeRobustness::SW_SECURE_CRYPTO;
+  if (robustness == "SW_SECURE_DECODE")
+    return EmeRobustness::SW_SECURE_DECODE;
+  if (robustness == "HW_SECURE_CRYPTO")
+    return EmeRobustness::HW_SECURE_CRYPTO;
+  if (robustness == "HW_SECURE_DECODE")
+    return EmeRobustness::HW_SECURE_DECODE;
+  if (robustness == "HW_SECURE_ALL")
+    return EmeRobustness::HW_SECURE_ALL;
+  return EmeRobustness::INVALID;
+}
+
 static bool IsSupportedContentType(const std::string& key_system,
                                    const std::string& mime_type,
                                    const std::string& codecs) {
   // TODO(sandersd): Move contentType parsing from Blink to here so that invalid
   // parameters can be rejected. http://crbug.com/417561
   // TODO(sandersd): Pass in the media type (audio or video) and check that the
   // container type matches. http://crbug.com/457384
   std::string container = base::StringToLowerASCII(mime_type);
 
   // Check that |codecs| are supported by the CDM. This check does not handle
@@ skipping 15 matching lines @@
   // TODO(sandersd): Reject ambiguous codecs. http://crbug.com/374751
   codec_vector.clear();
   net::ParseCodecString(codecs, &codec_vector, false);
   return net::AreSupportedMediaCodecs(codec_vector);
 }
 
 static bool GetSupportedCapabilities(
     const std::string& key_system,
     const blink::WebVector<blink::WebMediaKeySystemMediaCapability>&
         capabilities,
+    EmeMediaType media_type,
     std::vector<blink::WebMediaKeySystemMediaCapability>*
-        media_type_capabilities) {
+        media_type_capabilities,
+    ConfigState* config_state) {
   // From
   // https://w3c.github.io/encrypted-media/#get-supported-capabilities-for-media-type
   // 1. Let accumulated capabilities be partial configuration.
   //    (Skipped as there are no configuration-based codec restrictions.)
   // 2. Let media type capabilities be empty.
   DCHECK_EQ(media_type_capabilities->size(), 0ul);
   // 3. For each value in capabilities:
   for (size_t i = 0; i < capabilities.size(); i++) {
     // 3.1. Let contentType be the value's contentType member.
     // 3.2. Let robustness be the value's robustness member.
     const blink::WebMediaKeySystemMediaCapability& capability = capabilities[i];
     // 3.3. If contentType is the empty string, return null.
     if (capability.mimeType.isEmpty())
       return false;
     // 3.4-3.11. (Implemented by IsSupportedContentType().)
     if (!base::IsStringASCII(capability.mimeType) ||
         !base::IsStringASCII(capability.codecs) ||
         !IsSupportedContentType(key_system,
                                 base::UTF16ToASCII(capability.mimeType),
                                 base::UTF16ToASCII(capability.codecs))) {
       continue;
     }
     // 3.12. If robustness is not the empty string, run the following steps:
-    //       (Robustness is not supported.)
-    // TODO(sandersd): Implement robustness. http://crbug.com/442586
     if (!capability.robustness.isEmpty()) {
-      LOG(WARNING) << "Configuration rejected because rubustness strings are "
-                   << "not yet supported.";
-      continue;
+      // 3.12.1. If robustness is an unrecognized value or not supported by
+      //         implementation, continue to the next iteration. String
+      //         comparison is case-sensitive.
+      EmeConfigRule robustness_rule = GetRobustnessConfigRule(
+          key_system, media_type, ConvertRobustness(capability.robustness));
+      if (!config_state->IsRuleSupported(robustness_rule))
+        continue;
+      config_state->AddRule(robustness_rule);
+      // 3.12.2. Add robustness to configuration.
+      //         (It's already added, the original object is used directly.)

[ddorwin, 2015/03/19 18:08:35]

Suggest: It's added when the original object is pushed onto
media_type_capabilities below.

[sandersd (OOO until July 31), 2015/03/19 20:05:09]

Done.

     }
     // 3.13. If the user agent and implementation do not support playback of
     //       encrypted media data as specified by configuration, including all
     //       media types, in combination with accumulated capabilities,
     //       continue to the next iteration.
-    //       (Skipped as there are no configuration-based codec restrictions.)
+    //       (Skipped as there are no configuration-based codec restrictions.
+    //       There will be when the Android security level becomes configurable
+    //       based on robustness. http://crbug.com/467779)
     // 3.14. Add configuration to media type capabilities.
     media_type_capabilities->push_back(capability);
     // 3.15. Add configuration to accumulated capabilities.
-    //       (Skipped as there are no configuration-based codec restrictions.)
+    //       (Skipped as there are no configuration-based codec restrictions.
+    //       Note that this is the local accumulated capabilities, the global
+    //       one is updated by the caller.)
   }
   // 4. If media type capabilities is empty, return null.
   // 5. Return media type capabilities.
   return !media_type_capabilities->empty();
 }
 
 static EmeFeatureRequirement ConvertRequirement(
     blink::WebMediaKeySystemConfiguration::Requirement requirement) {
   switch (requirement) {
     case blink::WebMediaKeySystemConfiguration::Requirement::Required:
       return EME_FEATURE_REQUIRED;
     case blink::WebMediaKeySystemConfiguration::Requirement::Optional:
       return EME_FEATURE_OPTIONAL;
     case blink::WebMediaKeySystemConfiguration::Requirement::NotAllowed:
       return EME_FEATURE_NOT_ALLOWED;
   }
 
   NOTREACHED();
   return EME_FEATURE_NOT_ALLOWED;
 }
 
 static ConfigurationSupport GetSupportedConfiguration(
     const std::string& key_system,
     const blink::WebMediaKeySystemConfiguration& candidate,
-    blink::WebMediaKeySystemConfiguration* accumulated_configuration,
     bool was_permission_requested,
-    bool is_permission_granted) {
-  DCHECK(was_permission_requested || !is_permission_granted);
-
-  // It is possible to obtain user permission unless permission was already
-  // requested and denied.
-  bool is_permission_possible =
-      !was_permission_requested || is_permission_granted;
+    bool is_permission_granted,
+    blink::WebMediaKeySystemConfiguration* accumulated_configuration) {
+  ConfigState config_state(was_permission_requested, is_permission_granted);
 
   // From https://w3c.github.io/encrypted-media/#get-supported-configuration
   // 1. Let accumulated configuration be empty. (Done by caller.)
   // 2. If candidate configuration's initDataTypes attribute is not empty, run
   //    the following steps:
   if (!candidate.initDataTypes.isEmpty()) {
     // 2.1. Let supported types be empty.
     std::vector<blink::WebEncryptedMediaInitDataType> supported_types;
 
     // 2.2. For each value in candidate configuration's initDataTypes attribute:
@@ skipping 40 matching lines @@
 
   // 3. Follow the steps for the value of candidate configuration's
   //    distinctiveIdentifier attribute from the following list:
   //     - "required": If the implementation does not support a persistent
   //       Distinctive Identifier in combination with accumulated configuration,
   //       return null.
   //     - "optional": Continue.
   //     - "not-allowed": If the implementation requires a Distinctive
   //       Identifier in combination with accumulated configuration, return
   //       null.
-  EmeFeatureRequirement di_requirement =
-      ConvertRequirement(candidate.distinctiveIdentifier);
-  if (!IsDistinctiveIdentifierRequirementSupported(key_system, di_requirement,
-                                                   is_permission_possible)) {
+  // We also reject OPTIONAL when distinctive identifiers are ALWAYS_ENABLED and
+  // permission has already been denied. This would happen anyway at step 11.
+  EmeConfigRule di_rule = GetDistinctiveIdentifierConfigRule(
+      key_system, ConvertRequirement(candidate.distinctiveIdentifier));
+  if (!config_state.IsRuleSupported(di_rule))
     return CONFIGURATION_NOT_SUPPORTED;
-  }
+  config_state.AddRule(di_rule);
 
   // 4. Add the value of the candidate configuration's distinctiveIdentifier
   //    attribute to accumulated configuration.
   accumulated_configuration->distinctiveIdentifier =
       candidate.distinctiveIdentifier;
 
   // 5. Follow the steps for the value of candidate configuration's
   //    persistentState attribute from the following list:
   //     - "required": If the implementation does not support persisting state
   //       in combination with accumulated configuration, return null.
   //     - "optional": Continue.
   //     - "not-allowed": If the implementation requires persisting state in
   //       combination with accumulated configuration, return null.
-  EmeFeatureRequirement ps_requirement =
-      ConvertRequirement(candidate.persistentState);
-  if (!IsPersistentStateRequirementSupported(key_system, ps_requirement,
-                                             is_permission_possible)) {
+  EmeConfigRule ps_rule = GetPersistentStateConfigRule(
+      key_system, ConvertRequirement(candidate.persistentState));
+  if (!config_state.IsRuleSupported(ps_rule))
     return CONFIGURATION_NOT_SUPPORTED;
-  }
+  config_state.AddRule(ps_rule);
 
   // 6. Add the value of the candidate configuration's persistentState
   //    attribute to accumulated configuration.
   accumulated_configuration->persistentState = candidate.persistentState;
 
   // 7. If candidate configuration's videoCapabilities attribute is not empty,
   //    run the following steps:
   if (!candidate.videoCapabilities.isEmpty()) {
     // 7.1. Let video capabilities be the result of executing the Get Supported
     //      Capabilities for Media Type algorithm on Video, candidate
     //      configuration's videoCapabilities attribute, and accumulated
     //      configuration.
     // 7.2. If video capabilities is null, return null.
     std::vector<blink::WebMediaKeySystemMediaCapability> video_capabilities;
     if (!GetSupportedCapabilities(key_system, candidate.videoCapabilities,
-                                  &video_capabilities)) {
+                                  EmeMediaType::VIDEO, &video_capabilities,
+                                  &config_state)) {
       return CONFIGURATION_NOT_SUPPORTED;
     }
 
     // 7.3. Add video capabilities to accumulated configuration.
     accumulated_configuration->videoCapabilities = video_capabilities;
   }
 
   // 8. If candidate configuration's audioCapabilities attribute is not empty,
   //    run the following steps:
   if (!candidate.audioCapabilities.isEmpty()) {
     // 8.1. Let audio capabilities be the result of executing the Get Supported
     //      Capabilities for Media Type algorithm on Audio, candidate
     //      configuration's audioCapabilities attribute, and accumulated
     //      configuration.
     // 8.2. If audio capabilities is null, return null.
     std::vector<blink::WebMediaKeySystemMediaCapability> audio_capabilities;
     if (!GetSupportedCapabilities(key_system, candidate.audioCapabilities,
-                                  &audio_capabilities)) {
+                                  EmeMediaType::AUDIO, &audio_capabilities,
+                                  &config_state)) {
       return CONFIGURATION_NOT_SUPPORTED;
     }
 
     // 8.3. Add audio capabilities to accumulated configuration.
     accumulated_configuration->audioCapabilities = audio_capabilities;
   }
 
   // 9. If accumulated configuration's distinctiveIdentifier value is
   //    "optional", follow the steps for the first matching condition from the
   //    following list:
   //      - If the implementation requires a Distinctive Identifier for any of
   //        the combinations in accumulated configuration, change accumulated
   //        configuration's distinctiveIdentifier value to "required".
   //      - Otherwise, change accumulated configuration's distinctiveIdentifier
   //        value to "not-allowed".
-  //    (Without robustness support, capabilities do not affect this.)
-  // TODO(sandersd): Implement robustness. http://crbug.com/442586
   if (accumulated_configuration->distinctiveIdentifier ==
       blink::WebMediaKeySystemConfiguration::Requirement::Optional) {
-    if (IsDistinctiveIdentifierRequirementSupported(
-            key_system, EME_FEATURE_NOT_ALLOWED, is_permission_possible)) {
-      accumulated_configuration->distinctiveIdentifier =
-          blink::WebMediaKeySystemConfiguration::Requirement::NotAllowed;
-    } else {
+    EmeConfigRule not_allowed_rule =
+        GetDistinctiveIdentifierConfigRule(key_system, EME_FEATURE_NOT_ALLOWED);
+    EmeConfigRule required_rule =
+        GetDistinctiveIdentifierConfigRule(key_system, EME_FEATURE_REQUIRED);
+    bool not_allowed_supported = config_state.IsRuleSupported(not_allowed_rule);
+    bool required_supported = config_state.IsRuleSupported(required_rule);
+    if (not_allowed_supported) {
+      bool prefer_required = config_state.IsIdentifierRequired() ||
+                             (config_state.IsIdentifierRecommended() &&
+                              config_state.IsPermissionPossible());
+      if (required_supported && prefer_required) {
+        accumulated_configuration->distinctiveIdentifier =
+            blink::WebMediaKeySystemConfiguration::Requirement::Required;
+        config_state.AddRule(required_rule);
+        DCHECK(config_state.IsIdentifierRequired());
+      } else {
+        accumulated_configuration->distinctiveIdentifier =
+            blink::WebMediaKeySystemConfiguration::Requirement::NotAllowed;
+        config_state.AddRule(not_allowed_rule);
+      }
+    } else if (required_supported) {
       accumulated_configuration->distinctiveIdentifier =
           blink::WebMediaKeySystemConfiguration::Requirement::Required;
+      config_state.AddRule(required_rule);
+    } else {
+      // We should not have passed step 3.
+      NOTREACHED();
+      return CONFIGURATION_NOT_SUPPORTED;
     }
   }
 
+  // If permission is required but we couldn't enable it, reject the
+  // configuration.
+  if (config_state.IsIdentifierRequired() &&
+      accumulated_configuration->distinctiveIdentifier !=
+      blink::WebMediaKeySystemConfiguration::Requirement::Required) {
+    return CONFIGURATION_NOT_SUPPORTED;
+  }
+
   // 10. If accumulated configuration's persistentState value is "optional",
   //     follow the steps for the first matching condition from the following
   //     list:
   //       - If the implementation requires persisting state for any of the
   //         combinations in accumulated configuration, change accumulated
   //         configuration's persistentState value to "required".
   //       - Otherwise, change accumulated configuration's persistentState value
   //         to "not-allowed".
   if (accumulated_configuration->persistentState ==
       blink::WebMediaKeySystemConfiguration::Requirement::Optional) {
-    if (IsPersistentStateRequirementSupported(
-            key_system, EME_FEATURE_NOT_ALLOWED, is_permission_possible)) {
+    EmeConfigRule not_allowed_rule =
+        GetPersistentStateConfigRule(key_system, EME_FEATURE_NOT_ALLOWED);
+    EmeConfigRule required_rule =
+        GetPersistentStateConfigRule(key_system, EME_FEATURE_REQUIRED);
+    bool not_allowed_supported =
+        config_state.IsRuleSupportedWithCurrentState(not_allowed_rule);

[ddorwin, 2015/03/19 18:08:35]

It's not obvious to me why we use this method here and the other one above. Is
it because we've already decided whether to prompt?

[sandersd (OOO until July 31), 2015/03/19 20:05:09]

That's right. At this point we may accept a rule that requires a distinctive
identifier (note that we block that in KeySystems, so this concern is
theoretical), but it's too late to change the value. Using this method reliably
implements your intended behavior of requiring the user to supply a requirement
for persistentState to work.

The alternative is to provide a way to 'seal' the state, so that the regular
IsRuleSupported() can reject it. I'm not a big fan of this as it complicates the
state machine.

[ddorwin, 2015/03/19 22:11:57]

Should we add a comment pointing this out, either here or somewhere above after
the "point of no return?"

[sandersd (OOO until July 31), 2015/03/19 23:05:43]

Done.

+    bool required_supported =
+        config_state.IsRuleSupportedWithCurrentState(required_rule);
+    if (not_allowed_supported) {
       accumulated_configuration->persistentState =
           blink::WebMediaKeySystemConfiguration::Requirement::NotAllowed;
-    } else {
+    } else if (required_supported) {
       accumulated_configuration->persistentState =
           blink::WebMediaKeySystemConfiguration::Requirement::Required;
+    } else {
+      // We should not have passed step 5.
+      NOTREACHED();
+      return CONFIGURATION_NOT_SUPPORTED;
     }
   }
 
   // 11. If implementation in the configuration specified by the combination of
   //     the values in accumulated configuration is not supported or not allowed
   //     in the origin, return null.
-  di_requirement =
-      ConvertRequirement(accumulated_configuration->distinctiveIdentifier);
-  if (!IsDistinctiveIdentifierRequirementSupported(key_system, di_requirement,
-                                                   is_permission_granted)) {
-    if (was_permission_requested) {
-      // The optional permission was requested and denied.
-      // TODO(sandersd): Avoid the need for this logic - crbug.com/460616.
-      DCHECK(candidate.distinctiveIdentifier ==
-             blink::WebMediaKeySystemConfiguration::Requirement::Optional);
-      DCHECK(di_requirement == EME_FEATURE_REQUIRED);
-      DCHECK(!is_permission_granted);
-      accumulated_configuration->distinctiveIdentifier =
-          blink::WebMediaKeySystemConfiguration::Requirement::NotAllowed;
-    } else {
+  //
+  //     If accumulated configuration's distinctiveIdentifier value is
+  //     "required", [request user consent].
+  if (accumulated_configuration->distinctiveIdentifier ==
+      blink::WebMediaKeySystemConfiguration::Requirement::Required) {
+    if (!config_state.IsPermissionGranted())
       return CONFIGURATION_REQUIRES_PERMISSION;

[ddorwin, 2015/03/19 18:08:35]

Is the case where we've already requested the permission handled by the caller?

If so, is that the best solution? We have all the information here, right?

[sandersd (OOO until July 31), 2015/03/19 20:05:10]

It turned out to remove a lot of cases (and invalid states) to push that
decision up one level, making the code both more readable and more similar to
the spec. I'm happy with this tradeoff.

[ddorwin, 2015/03/19 22:11:57]

Okay. Perhaps note that this is handled by the caller here.

[sandersd (OOO until July 31), 2015/03/19 23:05:43]

Done.

-    }
-  }
-
-  ps_requirement =
-      ConvertRequirement(accumulated_configuration->persistentState);
-  if (!IsPersistentStateRequirementSupported(key_system, ps_requirement,
-                                             is_permission_granted)) {
-    DCHECK(!was_permission_requested);  // Should have failed at step 5.
-    return CONFIGURATION_REQUIRES_PERMISSION;
   }
 
   // 12. Return accumulated configuration.
-  //     (As an extra step, we record the available session types so that
-  //     createSession() can be synchronous.)
+  //
+  // We also record the available session types so that createSession() can be
+  // synchronous.
   std::vector<blink::WebEncryptedMediaSessionType> session_types;
   session_types.push_back(blink::WebEncryptedMediaSessionType::Temporary);
   if (accumulated_configuration->persistentState ==
       blink::WebMediaKeySystemConfiguration::Requirement::Required) {
-    if (IsPersistentLicenseSessionSupported(key_system,
-                                            is_permission_granted)) {
+    if (config_state.IsRuleSupportedWithCurrentState(

[ddorwin, 2015/03/19 18:08:35]

This one is more clear that we're not going to request permission because we
would have exited at 479.

[sandersd (OOO until July 31), 2015/03/19 20:05:09]

Acknowledged.

+            GetPersistentLicenseSessionConfigRule(key_system))) {
       session_types.push_back(
           blink::WebEncryptedMediaSessionType::PersistentLicense);
     }
-    if (IsPersistentReleaseMessageSessionSupported(key_system,
-                                                   is_permission_granted)) {
+    if (config_state.IsRuleSupportedWithCurrentState(
+            GetPersistentReleaseMessageSessionConfigRule(key_system))) {
       session_types.push_back(
           blink::WebEncryptedMediaSessionType::PersistentReleaseMessage);
     }
   }
   accumulated_configuration->sessionTypes = session_types;
 
   return CONFIGURATION_SUPPORTED;
 }
 
 // Report usage of key system to UMA. There are 2 different counts logged:
@@ skipping 103 matching lines @@
     // 7.3.1. Let candidate configuration be the value.
     const blink::WebMediaKeySystemConfiguration& candidate_configuration =
         configurations[i];
     // 7.3.2. Let supported configuration be the result of executing the Get
     //        Supported Configuration algorithm on implementation, candidate
     //        configuration, and origin.
     // 7.3.3. If supported configuration is not null, [initialize and return a
     //        new MediaKeySystemAccess object.]
     blink::WebMediaKeySystemConfiguration accumulated_configuration;
     ConfigurationSupport supported = GetSupportedConfiguration(
-        key_system, candidate_configuration, &accumulated_configuration,
-        was_permission_requested, is_permission_granted);
+        key_system, candidate_configuration, was_permission_requested,
+        is_permission_granted, &accumulated_configuration);
     switch (supported) {
       case CONFIGURATION_NOT_SUPPORTED:
         continue;
       case CONFIGURATION_REQUIRES_PERMISSION:
-        DCHECK(!was_permission_requested);
+        if (was_permission_requested)

[ddorwin, 2015/03/19 18:08:35]

This is the code corresponding to my question at line 479.

[sandersd (OOO until July 31), 2015/03/19 20:05:09]

Acknowledged.

+          continue;
         media_permission_->RequestPermission(
             MediaPermission::PROTECTED_MEDIA_IDENTIFIER,
             GURL(request.securityOrigin().toString()),
             // Try again with |was_permission_requested| true and
             // |is_permission_granted| the value of the permission.
             base::Bind(
                 &WebEncryptedMediaClientImpl::SelectSupportedConfiguration,
                 weak_factory_.GetWeakPtr(), request, true));
         return;
       case CONFIGURATION_SUPPORTED:
@@ skipping 33 matching lines @@
     return reporter;
 
   // Reporter not found, so create one.
   auto result =
       reporters_.add(uma_name, make_scoped_ptr(new Reporter(uma_name)));
   DCHECK(result.second);
   return result.first->second;
 }
 
 }  // namespace media