From 1899d5c3b008c6c4d118984933a8460cd5c0ce82 Mon Sep 17 00:00:00 2001 From: KongQun Yang Date: Thu, 8 May 2014 16:34:45 -0700 Subject: [PATCH] Add back Initialize which starts key fetching thread The key production thread was started in constructor. The default http fetcher might have already been used to fetch keys before it is modified by set_http_fetcher. This could lead to flaky unittest. Change-Id: I977b6450862d87ffeb5cb219bcd46b33d877e550 --- app/packager_common.cc | 20 ++++++++++++----- media/base/widevine_encryption_key_source.cc | 20 ++++++++++++++--- media/base/widevine_encryption_key_source.h | 5 +++++ ...widevine_encryption_key_source_unittest.cc | 22 +++++++++++++++---- 4 files changed, 54 insertions(+), 13 deletions(-) diff --git a/app/packager_common.cc b/app/packager_common.cc index c09bdf6c65..a9368ed5c1 100644 --- a/app/packager_common.cc +++ b/app/packager_common.cc @@ -66,12 +66,20 @@ scoped_ptr CreateEncryptionKeySource() { } } - encryption_key_source.reset(new WidevineEncryptionKeySource( - FLAGS_key_server_url, - FLAGS_content_id, - FLAGS_policy, - signer.Pass(), - FLAGS_crypto_period_duration == 0 ? kDisableKeyRotation : 0)); + scoped_ptr widevine_encryption_key_source( + new WidevineEncryptionKeySource( + FLAGS_key_server_url, + FLAGS_content_id, + FLAGS_policy, + signer.Pass(), + FLAGS_crypto_period_duration == 0 ? kDisableKeyRotation : 0)); + Status status = widevine_encryption_key_source->Initialize(); + if (!status.ok()) { + LOG(ERROR) << "Widevine encryption key source failed to initialize: " + << status.ToString(); + return scoped_ptr(); + } + encryption_key_source = widevine_encryption_key_source.Pass(); } else if (FLAGS_enable_fixed_key_encryption) { encryption_key_source = EncryptionKeySource::CreateFromHexStrings( FLAGS_key_id, FLAGS_key, FLAGS_pssh, ""); diff --git a/media/base/widevine_encryption_key_source.cc b/media/base/widevine_encryption_key_source.cc index 41139b7297..18aa3b14db 100644 --- a/media/base/widevine_encryption_key_source.cc +++ b/media/base/widevine_encryption_key_source.cc @@ -144,16 +144,28 @@ WidevineEncryptionKeySource::WidevineEncryptionKeySource( key_pool_(kDefaultCryptoPeriodCount, key_rotation_enabled_ ? first_crypto_period_index : 0) { DCHECK(signer_); - key_production_thread_.Start(); } WidevineEncryptionKeySource::~WidevineEncryptionKeySource() { key_pool_.Stop(); - key_production_thread_.Join(); + if (key_production_thread_.HasBeenStarted()) + key_production_thread_.Join(); +} + +Status WidevineEncryptionKeySource::Initialize() { + DCHECK(!key_production_thread_.HasBeenStarted()); + key_production_thread_.Start(); + + // Perform a GetKey request to find out common encryption request status. + // It also primes the key_pool if successful. + return key_rotation_enabled_ ? GetCryptoPeriodKey(first_crypto_period_index_, + TRACK_TYPE_SD, NULL) + : GetKey(TRACK_TYPE_SD, NULL); } Status WidevineEncryptionKeySource::GetKey(TrackType track_type, EncryptionKey* key) { + DCHECK(key_production_thread_.HasBeenStarted()); DCHECK(!key_rotation_enabled_); return GetKeyInternal(0u, track_type, key); } @@ -162,6 +174,7 @@ Status WidevineEncryptionKeySource::GetCryptoPeriodKey( uint32 crypto_period_index, TrackType track_type, EncryptionKey* key) { + DCHECK(key_production_thread_.HasBeenStarted()); DCHECK(key_rotation_enabled_); return GetKeyInternal(crypto_period_index, track_type, key); } @@ -195,7 +208,8 @@ Status WidevineEncryptionKeySource::GetKeyInternal( return Status(error::INTERNAL_ERROR, "Cannot find key of type " + TrackTypeToString(track_type)); } - *key = *encryption_key_map[track_type]; + if (key) + *key = *encryption_key_map[track_type]; return Status::OK; } diff --git a/media/base/widevine_encryption_key_source.h b/media/base/widevine_encryption_key_source.h index 4bad3e66fb..5fc1788471 100644 --- a/media/base/widevine_encryption_key_source.h +++ b/media/base/widevine_encryption_key_source.h @@ -39,6 +39,11 @@ class WidevineEncryptionKeySource : public EncryptionKeySource { int first_crypto_period_index); virtual ~WidevineEncryptionKeySource(); + /// Initialize the key source. Must be called before calling GetKey or + /// GetCryptoPeriodKey. + /// @return OK on success, an error status otherwise. + Status Initialize(); + /// @name EncryptionKeySource implementation overrides. /// @{ virtual Status GetKey(TrackType track_type, EncryptionKey* key) OVERRIDE; diff --git a/media/base/widevine_encryption_key_source_unittest.cc b/media/base/widevine_encryption_key_source_unittest.cc index bcd71ad12f..d9870b0940 100644 --- a/media/base/widevine_encryption_key_source_unittest.cc +++ b/media/base/widevine_encryption_key_source_unittest.cc @@ -167,6 +167,10 @@ TEST_F(WidevineEncryptionKeySourceTest, GenerateSignatureFailure) { .WillOnce(Return(false)); CreateWidevineEncryptionKeySource(kDisableKeyRotation); + ASSERT_EQ(Status(error::INTERNAL_ERROR, "Signature generation failed."), + widevine_encryption_key_source_->Initialize()); + + // GetKey should return the same failure. EncryptionKey encryption_key; ASSERT_EQ(Status(error::INTERNAL_ERROR, "Signature generation failed."), widevine_encryption_key_source_->GetKey( @@ -191,6 +195,10 @@ TEST_F(WidevineEncryptionKeySourceTest, HttpPostFailure) { .WillOnce(Return(kMockStatus)); CreateWidevineEncryptionKeySource(kDisableKeyRotation); + ASSERT_EQ(kMockStatus, + widevine_encryption_key_source_->Initialize()); + + // GetKey should return the same failure. EncryptionKey encryption_key; ASSERT_EQ(kMockStatus, widevine_encryption_key_source_->GetKey( @@ -208,6 +216,7 @@ TEST_F(WidevineEncryptionKeySourceTest, LicenseStatusOK) { .WillOnce(DoAll(SetArgPointee<2>(mock_response), Return(Status::OK))); CreateWidevineEncryptionKeySource(kDisableKeyRotation); + ASSERT_OK(widevine_encryption_key_source_->Initialize()); EncryptionKey encryption_key; const std::string kTrackTypes[] = {"SD", "HD", "AUDIO"}; @@ -241,6 +250,8 @@ TEST_F(WidevineEncryptionKeySourceTest, RetryOnTransientError) { Return(Status::OK))); CreateWidevineEncryptionKeySource(kDisableKeyRotation); + ASSERT_OK(widevine_encryption_key_source_->Initialize()); + EncryptionKey encryption_key; ASSERT_OK(widevine_encryption_key_source_->GetKey( EncryptionKeySource::TRACK_TYPE_SD, &encryption_key)); @@ -263,10 +274,8 @@ TEST_F(WidevineEncryptionKeySourceTest, NoRetryOnUnknownError) { .WillOnce(DoAll(SetArgPointee<2>(mock_response), Return(Status::OK))); CreateWidevineEncryptionKeySource(kDisableKeyRotation); - EncryptionKey encryption_key; - Status status = widevine_encryption_key_source_->GetKey( - EncryptionKeySource::TRACK_TYPE_SD, &encryption_key); - ASSERT_EQ(error::SERVER_ERROR, status.error_code()); + ASSERT_EQ(error::SERVER_ERROR, + widevine_encryption_key_source_->Initialize().error_code()); } namespace { @@ -339,6 +348,7 @@ TEST_F(WidevineEncryptionKeySourceTest, KeyRotationTest) { } CreateWidevineEncryptionKeySource(kFirstCryptoPeriodIndex); + ASSERT_OK(widevine_encryption_key_source_->Initialize()); EncryptionKey encryption_key; @@ -375,6 +385,8 @@ class WidevineEncryptionKeySourceDeathTest TEST_F(WidevineEncryptionKeySourceDeathTest, GetCryptoPeriodKeyOnNonKeyRotationSource) { CreateWidevineEncryptionKeySource(kDisableKeyRotation); + widevine_encryption_key_source_->Initialize(); + EncryptionKey encryption_key; EXPECT_DEBUG_DEATH( widevine_encryption_key_source_->GetCryptoPeriodKey( @@ -384,6 +396,8 @@ TEST_F(WidevineEncryptionKeySourceDeathTest, TEST_F(WidevineEncryptionKeySourceDeathTest, GetKeyOnKeyRotationSource) { CreateWidevineEncryptionKeySource(0); + widevine_encryption_key_source_->Initialize(); + EncryptionKey encryption_key; EXPECT_DEBUG_DEATH(widevine_encryption_key_source_->GetKey( EncryptionKeySource::TRACK_TYPE_SD, &encryption_key),