Merge "Fix handling of INSUFFICIENT_RESOURCES in LoadEntry"

libwvdrmengine/cdm/core/include/usage_table_header.h

@@ -79,6 +79,11 @@ class UsageTableHeader {
   // for the objects that DeleteEntry depends on.
   void DeleteEntryForTest(uint32_t usage_entry_number);
 
+  // TODO(rfrias): Move to utility class
+  static int64_t GetRandomInRange(size_t upper_bound_exclusive);
+  static int64_t GetRandomInRangeWithExclusion(size_t upper_bound_exclusive,
+                                               size_t exclude);
+
  private:
   CdmResponseType MoveEntry(uint32_t from /* usage entry number */,
                             const CdmUsageEntry& from_usage_entry,
@@ -127,9 +132,6 @@ class UsageTableHeader {
 
   metrics::CryptoMetrics alternate_crypto_metrics_;
 
-  // TODO(rfrias): Move to utility class
-  uint32_t GetRandomInRange(size_t upper_bound_inclusive);
-
   // Test related declarations
   friend class UsageTableHeaderTest;
 

libwvdrmengine/cdm/core/src/usage_table_header.cpp

@@ -152,8 +152,9 @@ CdmResponseType UsageTableHeader::AddEntry(
        retry_count < kMaxCryptoRetries &&
            status == INSUFFICIENT_CRYPTO_RESOURCES_3;
        ++retry_count) {
-    uint32_t entry_number_to_delete =
+    int64_t entry_number_to_delete =
         GetRandomInRange(usage_entry_info_.size());
+    if (entry_number_to_delete < 0) break;
     DeleteEntry(entry_number_to_delete, file_handle_.get(), metrics);
 
     status = crypto_session->CreateUsageEntry(usage_entry_number);
@@ -215,17 +216,18 @@ CdmResponseType UsageTableHeader::LoadEntry(CryptoSession* crypto_session,
       crypto_session->LoadUsageEntry(usage_entry_number, usage_entry);
 
   // If loading a usage entry fails due to insufficient resources, release a
-  // random entry and try again.
+  // random entry different from |usage_entry_number| and try again. If there
+  // are no more entries to release, we fail.
   for (uint32_t retry_count = 0;
        retry_count < kMaxCryptoRetries &&
            status == INSUFFICIENT_CRYPTO_RESOURCES_3;
        ++retry_count) {
-    uint32_t entry_number_to_delete =
+    // Get a random entry from the other entries.
-        GetRandomInRange(usage_entry_info_.size());
+    int64_t entry_number_to_delete = GetRandomInRangeWithExclusion(
-    if (usage_entry_number != entry_number_to_delete) {
+        usage_entry_info_.size(), usage_entry_number);
-      DeleteEntry(entry_number_to_delete, file_handle_.get(), metrics);
+    if (entry_number_to_delete < 0) break;
-      status = crypto_session->LoadUsageEntry(usage_entry_number, usage_entry);
+    DeleteEntry(entry_number_to_delete, file_handle_.get(), metrics);
-    }
+    status = crypto_session->LoadUsageEntry(usage_entry_number, usage_entry);
   }
 
   return status;
@@ -707,9 +709,26 @@ bool UsageTableHeader::UpgradeUsageInfoFromUsageTable(
   return NO_ERROR;
 }
 
-uint32_t UsageTableHeader::GetRandomInRange(size_t upper_bound_inclusive) {
+int64_t UsageTableHeader::GetRandomInRange(size_t upper_bound_exclusive) {
-  if (upper_bound_inclusive == 0) return 0;
+  if (upper_bound_exclusive == 0) {
-  return rand() / (RAND_MAX / upper_bound_inclusive  + 1);
+    LOGE("upper_bound_exclusive must be > 0.");
+    return -1;
+  }
+  return (int)((double)rand() / ((double)RAND_MAX + 1) * upper_bound_exclusive);
+}
+
+int64_t UsageTableHeader::GetRandomInRangeWithExclusion(
+    size_t upper_bound_exclusive, size_t exclude) {
+  if ((upper_bound_exclusive <= 1) ||
+      (exclude >= upper_bound_exclusive)) {
+    LOGE(
+        "upper_bound_exclusive must be > 1 and exclude must be < "
+        "upper_bound_exclusive.");
+    return -1;
+  }
+  uint32_t random = GetRandomInRange(upper_bound_exclusive - 1);
+  if (random >= exclude) random++;
+  return random;
 }
 
 // TODO(fredgc): remove when b/65730828 is addressed

libwvdrmengine/cdm/core/test/usage_table_header_unittest.cpp

@@ -282,9 +282,12 @@ class MockUsageTableHeader : public UsageTableHeader {
 
 // gmock methods
 using ::testing::_;
+using ::testing::AllOf;
 using ::testing::AtMost;
 using ::testing::ElementsAreArray;
+using ::testing::Ge;
 using ::testing::Invoke;
+using ::testing::Lt;
 using ::testing::NotNull;
 using ::testing::Return;
 using ::testing::SaveArg;
@@ -977,14 +980,16 @@ TEST_F(UsageTableHeaderTest,
   MockUsageTableHeader* mock_usage_table_header = SetUpMock();
   Init(kSecurityLevelL1, kUsageTableHeader, k10UsageEntryInfoVector);
 
-  uint32_t usage_entry_number_to_load = rand() /
+  int64_t usage_entry_number_to_load =
-      (RAND_MAX / k10UsageEntryInfoVector.size() + 1);
+      MockUsageTableHeader::GetRandomInRange(k10UsageEntryInfoVector.size());
+  ASSERT_THAT(usage_entry_number_to_load,
+              AllOf(Ge(0), Lt((int64_t)k10UsageEntryInfoVector.size())));
   CdmUsageEntry usage_entry_to_load = kUsageEntry;
 
   // Setup expectations
   EXPECT_CALL(*mock_usage_table_header,
               DeleteEntry(_, device_files_, NotNull()))
-      .Times(AtMost(1))
+      .Times(1)
       .WillRepeatedly(
           DoAll(Invoke(this, &UsageTableHeaderTest::DeleteEntry),
                 Return(NO_ERROR)));
@@ -1008,14 +1013,16 @@ TEST_F(UsageTableHeaderTest,
   MockUsageTableHeader* mock_usage_table_header = SetUpMock();
   Init(kSecurityLevelL1, kUsageTableHeader, k10UsageEntryInfoVector);
 
-  uint32_t usage_entry_number_to_load = rand() /
+  int64_t usage_entry_number_to_load =
-      (RAND_MAX / k10UsageEntryInfoVector.size() + 1);
+      MockUsageTableHeader::GetRandomInRange(k10UsageEntryInfoVector.size());
+  ASSERT_THAT(usage_entry_number_to_load,
+              AllOf(Ge(0), Lt((int64_t)k10UsageEntryInfoVector.size())));
   CdmUsageEntry usage_entry_to_load = kUsageEntry;
 
   // Setup expectations
   EXPECT_CALL(*mock_usage_table_header,
               DeleteEntry(_, device_files_, NotNull()))
-      .Times(AtMost(2))
+      .Times(2)
       .WillRepeatedly(
           DoAll(Invoke(this, &UsageTableHeaderTest::DeleteEntry),
                 Return(NO_ERROR)));
@@ -1039,21 +1046,23 @@ TEST_F(UsageTableHeaderTest, LoadEntry_LoadUsageEntryFailsThrice) {
   MockUsageTableHeader* mock_usage_table_header = SetUpMock();
   Init(kSecurityLevelL1, kUsageTableHeader, k10UsageEntryInfoVector);
 
-  uint32_t usage_entry_number_to_load = rand() /
+  int64_t usage_entry_number_to_load =
-      (RAND_MAX / k10UsageEntryInfoVector.size() + 1);
+      MockUsageTableHeader::GetRandomInRange(k10UsageEntryInfoVector.size());
+  ASSERT_THAT(usage_entry_number_to_load,
+              AllOf(Ge(0), Lt((int64_t)k10UsageEntryInfoVector.size())));
   CdmUsageEntry usage_entry_to_load = kUsageEntry;
 
   // Setup expectations
   EXPECT_CALL(*mock_usage_table_header,
               DeleteEntry(_, device_files_, NotNull()))
-      .Times(AtMost(3))
+      .Times(3)
       .WillRepeatedly(
           DoAll(Invoke(this, &UsageTableHeaderTest::DeleteEntry),
                 Return(NO_ERROR)));
 
   EXPECT_CALL(*crypto_session_,
               LoadUsageEntry(usage_entry_number_to_load, usage_entry_to_load))
-      .Times(AtMost(4))
+      .Times(4)
       .WillRepeatedly(Return(INSUFFICIENT_CRYPTO_RESOURCES_3));
 
   // Now invoke the method under test