Update StressTest to handle asynchronous write succeeding unexpectedly

As writing the file is done asynchronously, when the test doesn't wait
for the write to complete but closes the file immediately, it is
possible that the write succeeds. So updating the test to check that the
result of reading the reopened file is either the original contents or
the updated contents (i.e. the file was not corrupted).

As the test succeeded locally before this change, tried with the file
parameters to EXPECT_FILE_READ_EITHER() in both orders.

Bug: 999421
Test: Updated test succeeds
Change-Id: I7c9a3d948b3a93ccc878b28d1fd4f5bed5e1015c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1787590Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Commit-Queue: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694475}

media/cdm/library_cdm/clear_key_cdm/cdm_file_io_test.cc

@@ -47,28 +47,32 @@ const uint32_t kLargeDataSize = 20 * 1024 + 7;
   } while(0);
 
 #define CREATE_FILE_IO \
-  ADD_TEST_STEP(ACTION_CREATE, kSuccess, NULL, 0)
+  ADD_TEST_STEP(ACTION_CREATE, kSuccess, nullptr, 0)
 
 #define OPEN_FILE \
-  ADD_TEST_STEP(ACTION_OPEN, kSuccess, NULL, 0)
+  ADD_TEST_STEP(ACTION_OPEN, kSuccess, nullptr, 0)
 
 #define EXPECT_FILE_OPENED(status) \
-  ADD_TEST_STEP(RESULT_OPEN, status, NULL, 0)
+  ADD_TEST_STEP(RESULT_OPEN, status, nullptr, 0)
 
 #define READ_FILE \
-  ADD_TEST_STEP(ACTION_READ, kSuccess, NULL, 0)
+  ADD_TEST_STEP(ACTION_READ, kSuccess, nullptr, 0)
 
 #define EXPECT_FILE_READ(status, data, data_size) \
   ADD_TEST_STEP(RESULT_READ, status, data, data_size)
 
+#define EXPECT_FILE_READ_EITHER(status, data, data_size, data2, data2_size) \
+  test_case->AddResultReadEither(cdm::FileIOClient::Status::status, (data), \
+                                 (data_size), (data2), (data2_size));
+
 #define WRITE_FILE(data, data_size) \
   ADD_TEST_STEP(ACTION_WRITE, kSuccess, data, data_size)
 
 #define EXPECT_FILE_WRITTEN(status) \
-  ADD_TEST_STEP(RESULT_WRITE, status, NULL, 0)
+  ADD_TEST_STEP(RESULT_WRITE, status, nullptr, 0)
 
 #define CLOSE_FILE \
-  ADD_TEST_STEP(ACTION_CLOSE, kSuccess, NULL, 0)
+  ADD_TEST_STEP(ACTION_CLOSE, kSuccess, nullptr, 0)
 
 // FileIOTestRunner implementation.
 
@@ -150,7 +154,7 @@ void FileIOTestRunner::AddTests() {
 
   START_TEST_CASE("ReadBeforeOpeningFile")
     READ_FILE
-    EXPECT_FILE_READ(kError, NULL, 0)
+    EXPECT_FILE_READ(kError, nullptr, 0)
   END_TEST_CASE
 
   START_TEST_CASE("WriteBeforeOpeningFile")
@@ -162,7 +166,7 @@ void FileIOTestRunner::AddTests() {
     OPEN_FILE
     READ_FILE
     EXPECT_FILE_OPENED(kSuccess)
-    EXPECT_FILE_READ(kError, NULL, 0)
+    EXPECT_FILE_READ(kError, nullptr, 0)
     // After file opened, we can still do normal operations.
     WRITE_FILE(kData, kDataSize)
     EXPECT_FILE_WRITTEN(kSuccess)
@@ -189,7 +193,7 @@ void FileIOTestRunner::AddTests() {
     EXPECT_FILE_WRITTEN(kSuccess)
     READ_FILE
     READ_FILE
-    EXPECT_FILE_READ(kInUse, NULL, 0)
+    EXPECT_FILE_READ(kInUse, nullptr, 0)
     EXPECT_FILE_READ(kSuccess, kData, kDataSize)
     // Read again.
     READ_FILE
@@ -201,7 +205,7 @@ void FileIOTestRunner::AddTests() {
     EXPECT_FILE_OPENED(kSuccess)
     WRITE_FILE(kData, kDataSize)
     READ_FILE
-    EXPECT_FILE_READ(kInUse, NULL, 0)
+    EXPECT_FILE_READ(kInUse, nullptr, 0)
     EXPECT_FILE_WRITTEN(kSuccess)
     // Read again.
     READ_FILE
@@ -214,7 +218,7 @@ void FileIOTestRunner::AddTests() {
     READ_FILE
     WRITE_FILE(kData, kDataSize)
     EXPECT_FILE_WRITTEN(kInUse)
-    EXPECT_FILE_READ(kSuccess, NULL, 0)
+    EXPECT_FILE_READ(kSuccess, nullptr, 0)
     // We can still do normal operations.
     WRITE_FILE(kData, kDataSize)
     EXPECT_FILE_WRITTEN(kSuccess)
@@ -238,7 +242,7 @@ void FileIOTestRunner::AddTests() {
     OPEN_FILE
     EXPECT_FILE_OPENED(kSuccess)
     READ_FILE
-    EXPECT_FILE_READ(kSuccess, NULL, 0)
+    EXPECT_FILE_READ(kSuccess, nullptr, 0)
   END_TEST_CASE
 
   START_TEST_CASE("WriteAndRead")
@@ -253,10 +257,10 @@ void FileIOTestRunner::AddTests() {
   START_TEST_CASE("WriteAndReadEmptyFile")
     OPEN_FILE
     EXPECT_FILE_OPENED(kSuccess)
-    WRITE_FILE(NULL, 0)
+    WRITE_FILE(nullptr, 0)
     EXPECT_FILE_WRITTEN(kSuccess)
     READ_FILE
-    EXPECT_FILE_READ(kSuccess, NULL, 0)
+    EXPECT_FILE_READ(kSuccess, nullptr, 0)
   END_TEST_CASE
 
   START_TEST_CASE("WriteAndReadLargeData")
@@ -275,10 +279,10 @@ void FileIOTestRunner::AddTests() {
     EXPECT_FILE_WRITTEN(kSuccess)
     READ_FILE
     EXPECT_FILE_READ(kSuccess, kData, kDataSize)
-    WRITE_FILE(NULL, 0)
+    WRITE_FILE(nullptr, 0)
     EXPECT_FILE_WRITTEN(kSuccess)
     READ_FILE
-    EXPECT_FILE_READ(kSuccess, NULL, 0)
+    EXPECT_FILE_READ(kSuccess, nullptr, 0)
   END_TEST_CASE
 
   START_TEST_CASE("OverwriteWithSmallerData")
@@ -353,7 +357,7 @@ void FileIOTestRunner::AddTests() {
     EXPECT_FILE_OPENED(kSuccess)
     // Read file which doesn't exist.
     READ_FILE
-    EXPECT_FILE_READ(kSuccess, NULL, 0)
+    EXPECT_FILE_READ(kSuccess, nullptr, 0)
     // Write kData to file.
     WRITE_FILE(kData, kDataSize)
     EXPECT_FILE_WRITTEN(kSuccess)
@@ -376,11 +380,11 @@ void FileIOTestRunner::AddTests() {
     READ_FILE
     EXPECT_FILE_READ(kSuccess, kData, kDataSize)
     // Overwrite file with zero bytes.
-    WRITE_FILE(NULL, 0)
+    WRITE_FILE(nullptr, 0)
     EXPECT_FILE_WRITTEN(kSuccess)
     // Read file.
     READ_FILE
-    EXPECT_FILE_READ(kSuccess, NULL, 0)
+    EXPECT_FILE_READ(kSuccess, nullptr, 0)
   END_TEST_CASE
 
   START_TEST_CASE("OpenAfterOpen")
@@ -482,7 +486,12 @@ void FileIOTestRunner::AddTests() {
       OPEN_FILE
       EXPECT_FILE_OPENED(kSuccess)
       READ_FILE
-      EXPECT_FILE_READ(kSuccess, kData, kDataSize)
+      // As Write() is async, it is possible that the second write above
+      // succeeds before the file is closed. So check that the contents
+      // is either data set.
+      EXPECT_FILE_READ_EITHER(kSuccess,
+                              kData, kDataSize,
+                              kBigData, kBigDataSize)
       CLOSE_FILE
     }
   END_TEST_CASE
@@ -530,6 +539,16 @@ void FileIOTest::AddTestStep(StepType type,
   test_steps_.push_back(TestStep(type, status, data, data_size));
 }
 
+void FileIOTest::AddResultReadEither(Status status,
+                                     const uint8_t* data,
+                                     uint32_t data_size,
+                                     const uint8_t* data2,
+                                     uint32_t data2_size) {
+  DCHECK_NE(data_size, data2_size);
+  test_steps_.push_back(TestStep(FileIOTest::RESULT_READ, status, data,
+                                 data_size, data2, data2_size));
+}
+
 void FileIOTest::Run(const CompletionCB& completion_cb) {
   FILE_IO_DVLOG(3) << "Run " << test_name_;
   completion_cb_ = completion_cb;
@@ -538,7 +557,7 @@ void FileIOTest::Run(const CompletionCB& completion_cb) {
 }
 
 void FileIOTest::OnOpenComplete(Status status) {
-  OnResult(TestStep(RESULT_OPEN, status, NULL, 0));
+  OnResult(TestStep(RESULT_OPEN, status));
 }
 
 void FileIOTest::OnReadComplete(Status status,
@@ -548,7 +567,7 @@ void FileIOTest::OnReadComplete(Status status,
 }
 
 void FileIOTest::OnWriteComplete(Status status) {
-  OnResult(TestStep(RESULT_WRITE, status, NULL, 0));
+  OnResult(TestStep(RESULT_WRITE, status));
 }
 
 bool FileIOTest::IsResult(const TestStep& test_step) {
@@ -570,12 +589,19 @@ bool FileIOTest::IsResult(const TestStep& test_step) {
 
 bool FileIOTest::MatchesResult(const TestStep& a, const TestStep& b) {
   DCHECK(IsResult(a) && IsResult(b));
+  DCHECK(!b.data2);
+
   if (a.type != b.type || a.status != b.status)
     return false;
 
   if (a.type != RESULT_READ || a.status != cdm::FileIOClient::Status::kSuccess)
     return true;
 
+  // If |a| specifies a data2, compare it first. If the size matches, compare
+  // the contents.
+  if (a.data2 && b.data_size == a.data2_size)
+    return std::equal(a.data2, a.data2 + a.data2_size, b.data);
+
   return (a.data_size == b.data_size &&
           std::equal(a.data, a.data + a.data_size, b.data));
 }
@@ -590,7 +616,8 @@ void FileIOTest::RunNextStep() {
     TestStep test_step = test_steps_.front();
     test_steps_.pop_front();
 
-    cdm::FileIO* file_io = file_io_stack_.empty() ? NULL : file_io_stack_.top();
+    cdm::FileIO* file_io =
+        file_io_stack_.empty() ? nullptr : file_io_stack_.top();
 
     switch (test_step.type) {
       case ACTION_CREATE:

media/cdm/library_cdm/clear_key_cdm/cdm_file_io_test.h

@@ -75,19 +75,34 @@ class FileIOTest : public cdm::FileIOClient {
                    Status status,
                    const uint8_t* data,
                    uint32_t data_size);
+  // Adds a test step in this test that expects a successful read of either
+  // |data| or |data2|. |this| object doesn't take the ownership of |data| or
+  // |data2|, which should be valid throughout the lifetime of |this| object.
+  void AddResultReadEither(Status status,
+                           const uint8_t* data,
+                           uint32_t data_size,
+                           const uint8_t* data2,
+                           uint32_t data2_size);
 
   // Runs this test case and returns the test result through |completion_cb|.
   void Run(const CompletionCB& completion_cb);
 
  private:
   struct TestStep {
-    // |this| object doesn't take the ownership of |data|, which should be valid
-    // throughout the lifetime of |this| object.
+    // |this| object doesn't take the ownership of |data| or |data2|, which
+    // should be valid throughout the lifetime of |this| object.
     TestStep(StepType type,
              Status status,
-             const uint8_t* data,
-             uint32_t data_size)
-        : type(type), status(status), data(data), data_size(data_size) {}
+             const uint8_t* data = nullptr,
+             uint32_t data_size = 0,
+             const uint8_t* data2 = nullptr,
+             uint32_t data2_size = 0)
+        : type(type),
+          status(status),
+          data(data),
+          data_size(data_size),
+          data2(data2),
+          data2_size(data2_size) {}
 
     StepType type;
 
@@ -97,6 +112,10 @@ class FileIOTest : public cdm::FileIOClient {
     // Data to write in ACTION_WRITE, or read data in RESULT_READ.
     const uint8_t* data;
     uint32_t data_size;
+
+    // Alternate read data in RESULT_READ, if |data2| != nullptr.
+    const uint8_t* data2;
+    uint32_t data2_size;
   };
 
   // Returns whether |test_step| is a RESULT_* step.