media: Enable encrypted media content browsertests when MojoCdm is used

media/base/key_systems.cc

 // Copyright 2013 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 "media/base/key_systems.h"
 
 #include <stddef.h>
 
 #include <memory>
 
 #include "base/containers/hash_tables.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/strings/string_util.h"
 #include "base/threading/thread_checker.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "media/base/key_system_names.h"
 #include "media/base/key_system_properties.h"
 #include "media/base/media.h"
+#include "media/base/media_switches.h"
 #include "ppapi/features/features.h"
 #include "media/base/media_client.h"
 #include "third_party/widevine/cdm/widevine_cdm_common.h"
 
 namespace media {
 
 const char kClearKeyKeySystem[] = "org.w3.clearkey";
 
 // These names are used by UMA. Do not change them!
 const char kClearKeyKeySystemNameForUMA[] = "ClearKey";
@@ skipping 360 matching lines @@
     // If distinctive identifiers are not supported, then no other features can
     // require them.
     if (properties->GetDistinctiveIdentifierSupport() ==
         EmeFeatureSupport::NOT_SUPPORTED) {
       DCHECK(properties->GetPersistentLicenseSessionSupport() !=
              EmeSessionTypeSupport::SUPPORTED_WITH_IDENTIFIER);
       DCHECK(properties->GetPersistentReleaseMessageSessionSupport() !=
              EmeSessionTypeSupport::SUPPORTED_WITH_IDENTIFIER);
     }
 
     // Distinctive identifiers and persistent state can only be reliably blocked

[ddorwin, 2016/11/10 01:24:29]

I wonder if we should move this to a CanBlock() helper function so we can just
return a bool at each point, possibly synchronized with the comment (rather than
as one comment at the top) and to make it clear that this code is only dealing
with determining can_block.

[xhwang, 2016/11/10 07:11:28]

Good point. I like it. Done.

     // (and therefore be safely configurable) for Pepper-hosted key systems. For
     // other platforms assume the CDM can and will do anything, except for the
     // following two cases:
     // 1) AES decryptor, and
-    // 2) External Clear Key key system on Android, only enabled for testing.
+    // 2) External Clear Key key system when MojoCdm is used, only enabled for
+    //    testing.
     bool can_block = properties->UseAesDecryptor();
 #if BUILDFLAG(ENABLE_PEPPER_CDMS)
     DCHECK_EQ(properties->UseAesDecryptor(),
               properties->GetPepperType().empty());
     if (!properties->GetPepperType().empty())
       can_block = true;
-#elif defined(OS_ANDROID)
-    if (IsExternalClearKey(properties->GetKeySystemName()))
+#endif
+    if (base::FeatureList::IsEnabled(media::kExternalClearKeyForTesting) &&

[ddorwin, 2016/10/28 19:03:47]

Does this work with ENABLE_PEPPER_CDMS? Do we expect it to be used for Pepper
ECK?

I guess use of this flag would cause us to say it is blockable. But we must
still register it as ALWAYS_ENABLED? 

[xhwang, 2016/11/03 07:14:44]

In browser_tests we do set kExternalClearKeyForTesting so this check will be
true when we run ECK tests. So for ECK can_block is always true because:
- For pepper ECK, all pepper-based key system "can block", see line 404 and 415.
- For mojo ECK, it's implemented using AesDecryptor, so it also "can block", see
line 408.

The tricky part is that pepper ECK will also execute this line, which I feel
okay. I can put this in a defined(ENABLE_MOJO_CDM) block if you like.

[ddorwin, 2016/11/10 01:24:29]

The code is inconsistent with the comment, which says this is limited to
MojoCdm. They should probably be consistent. (Refactoring this and/or the
comments, as suggested above, would also hep us avoid introducing such
inconsistencies.)

[xhwang, 2016/11/10 07:11:28]

Done.

+        IsExternalClearKey(properties->GetKeySystemName()))
       can_block = true;
-#endif
+
     if (!can_block) {
       DCHECK(properties->GetDistinctiveIdentifierSupport() ==
              EmeFeatureSupport::ALWAYS_ENABLED);
       DCHECK(properties->GetPersistentStateSupport() ==
              EmeFeatureSupport::ALWAYS_ENABLED);
     }
 
     DCHECK_EQ(key_system_properties_map_.count(properties->GetKeySystemName()),
               0u)
         << "Key system '" << properties->GetKeySystemName()
@@ skipping 295 matching lines @@
                                uint32_t mask) {
   KeySystemsImpl::GetInstance()->AddCodecMask(media_type, codec, mask);
 }
 
 MEDIA_EXPORT void AddMimeTypeCodecMask(const std::string& mime_type,
                                        uint32_t mask) {
   KeySystemsImpl::GetInstance()->AddMimeTypeCodecMask(mime_type, mask);
 }
 
 }  // namespace media