Make sure KeySystems is up to date before selecting key system

UpdateIfNeeded() was only called in GetInstace(), but
KeySystemConfigSelector caches the KeySystems pointer.  Calls to
SelectConfig() that happened after installing a new key system still saw
the initial list of key systems.

To fix this, we expose UpdateIfNeeded() in the KeySystems interface so
that KeySystemConfigSelector can call it in SelectConfig() to make sure
the information on supported key systems is up to date.

Bug: 1033932
Change-Id: I7bdbe7ead15907fa6605a445f6bf9d7842f3c1ff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1967188
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Commit-Queue: Wojciech Dzierżanowski <wdzierzanowski@opera.com>
Auto-Submit: Wojciech Dzierżanowski <wdzierzanowski@opera.com>
Cr-Commit-Position: refs/heads/master@{#733688}

media/base/key_systems.cc

@@ -10,6 +10,7 @@
 #include <unordered_map>
 
 #include "base/logging.h"
+#include "base/no_destructor.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/threading/thread_checker.h"
@@ -232,8 +233,6 @@ class KeySystemsImpl : public KeySystems {
  public:
   static KeySystemsImpl* GetInstance();
 
-  void UpdateIfNeeded();
-
   // These two functions are for testing purpose only.
   void AddCodecMaskForTesting(EmeMediaType media_type,
                               const std::string& codec,
@@ -242,6 +241,8 @@ class KeySystemsImpl : public KeySystems {
                                       uint32_t mask);
 
   // Implementation of KeySystems interface.
+  void UpdateIfNeeded() override;
+
   bool IsSupportedKeySystem(const std::string& key_system) const override;
 
   bool CanUseAesDecryptor(const std::string& key_system) const override;
@@ -277,6 +278,8 @@ class KeySystemsImpl : public KeySystems {
       const std::string& key_system) const override;
 
  private:
+  friend class base::NoDestructor<KeySystemsImpl>;
+
   KeySystemsImpl();
   ~KeySystemsImpl() override;
 
@@ -328,9 +331,9 @@ class KeySystemsImpl : public KeySystems {
 };
 
 KeySystemsImpl* KeySystemsImpl::GetInstance() {
-  static KeySystemsImpl* key_systems = new KeySystemsImpl();
+  static base::NoDestructor<KeySystemsImpl> key_systems;
   key_systems->UpdateIfNeeded();
-  return key_systems;
+  return key_systems.get();
 }
 
 // Because we use a thread-safe static, the key systems info must be populated

media/base/key_systems.h

@@ -28,6 +28,9 @@ class MEDIA_EXPORT KeySystems {
  public:
   static KeySystems* GetInstance();
 
+  // Refreshes the list of available key systems if it may be out of date.
+  virtual void UpdateIfNeeded() = 0;
+
   // Returns whether |key_system| is a supported key system.
   virtual bool IsSupportedKeySystem(const std::string& key_system) const = 0;
 

media/blink/key_system_config_selector.cc

@@ -302,7 +302,7 @@ class KeySystemConfigSelector::ConfigState {
 };
 
 KeySystemConfigSelector::KeySystemConfigSelector(
-    const KeySystems* key_systems,
+    KeySystems* key_systems,
     MediaPermission* media_permission)
     : key_systems_(key_systems),
       media_permission_(media_permission),
@@ -909,6 +909,8 @@ void KeySystemConfigSelector::SelectConfig(
     return;
   }
 
+  key_systems_->UpdateIfNeeded();
+
   std::string key_system_ascii = key_system.Ascii();
   if (!key_systems_->IsSupportedKeySystem(key_system_ascii)) {
     not_supported_cb.Run();

media/blink/key_system_config_selector.h

@@ -33,7 +33,7 @@ class MediaPermission;
 
 class MEDIA_BLINK_EXPORT KeySystemConfigSelector {
  public:
-  KeySystemConfigSelector(const KeySystems* key_systems,
+  KeySystemConfigSelector(KeySystems* key_systems,
                           MediaPermission* media_permission);
 
   ~KeySystemConfigSelector();
@@ -96,7 +96,7 @@ class MEDIA_BLINK_EXPORT KeySystemConfigSelector {
       const blink::WebMediaKeySystemMediaCapability::EncryptionScheme
           encryption_scheme);
 
-  const KeySystems* key_systems_;
+  KeySystems* const key_systems_;
   MediaPermission* media_permission_;
 
   // A callback used to check whether a media type is supported. Only set in

media/blink/key_system_config_selector_unittest.cc

@@ -191,6 +191,8 @@ class FakeKeySystems : public KeySystems {
  public:
   ~FakeKeySystems() override = default;
 
+  void UpdateIfNeeded() override { ++update_count; }
+
   bool IsSupportedKeySystem(const std::string& key_system) const override {
     // Based on EME spec, Clear Key key system is always supported.
     return key_system == kSupportedKeySystem ||
@@ -333,6 +335,8 @@ class FakeKeySystems : public KeySystems {
   // the default values may be changed.
   EmeFeatureSupport persistent_state = EmeFeatureSupport::NOT_SUPPORTED;
   EmeFeatureSupport distinctive_identifier = EmeFeatureSupport::REQUESTABLE;
+
+  int update_count = 0;
 };
 
 class FakeMediaPermission : public MediaPermission {
@@ -504,6 +508,17 @@ TEST_F(KeySystemConfigSelectorTest, UsableConfig) {
   EXPECT_FALSE(cdm_config_.use_hw_secure_codecs);
 }
 
+// KeySystemConfigSelector should make sure it uses up-to-date KeySystems.
+TEST_F(KeySystemConfigSelectorTest, UpdateKeySystems) {
+  configs_.push_back(UsableConfiguration());
+
+  ASSERT_EQ(key_systems_->update_count, 0);
+  SelectConfig();
+  EXPECT_EQ(key_systems_->update_count, 1);
+  SelectConfig();
+  EXPECT_EQ(key_systems_->update_count, 2);
+}
+
 TEST_F(KeySystemConfigSelectorTest, Label) {
   auto config = UsableConfiguration();
   config.label = "foo";