cros: WallpaperControllerTest should use ScopedTempDir

The flakiness is not related to RunUntilIdle(), but is because
ScopedTempDir is not used for custom or online wallpapers: the tests
do lots of operations on the directories, so we should make sure they
are unique to avoid affecting each other.

Bug: 823760, 823762
Test: ash_unittests (Run all tests together)
Change-Id: I9cf6a70a4cfc146c8a0b91ed4cf2860dbc72394f
Reviewed-on: https://chromium-review.googlesource.com/1252981
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595308}

ash/wallpaper/wallpaper_controller_unittest.cc

@@ -87,13 +87,6 @@ const std::string file_name_2 = GetDummyFileName(account_id_2);
 const std::string kDummyUrl = "https://best_wallpaper/1";
 const std::string kDummyUrl2 = "https://best_wallpaper/2";
 
-const base::FilePath user_data_dir =
-    base::FilePath(FILE_PATH_LITERAL("user_data"));
-const base::FilePath wallpapers_dir =
-    base::FilePath(FILE_PATH_LITERAL("chrome_os_wallpapers"));
-const base::FilePath custom_wallpapers_dir =
-    base::FilePath(FILE_PATH_LITERAL("chrome_os_custom_wallpapers"));
-
 // Creates an image of size |size|.
 gfx::ImageSkia CreateImage(int width, int height, SkColor color) {
   SkBitmap bitmap;
@@ -190,12 +183,6 @@ void WaitUntilCustomWallpapersDeleted(const AccountId& account_id) {
   }
 }
 
-void DeleteWallpaperDirectories() {
-  base::DeleteFile(user_data_dir, true /*recursive=*/);
-  base::DeleteFile(wallpapers_dir, true /*recursive=*/);
-  base::DeleteFile(custom_wallpapers_dir, true /*recursive=*/);
-}
-
 // Monitors if any task is processed by the message loop.
 class TaskObserver : public base::MessageLoop::TaskObserver {
  public:
@@ -296,16 +283,13 @@ class WallpaperControllerTest : public AshTestBase {
         ->ResetWidgetsForTesting();
     controller_ = Shell::Get()->wallpaper_controller();
     controller_->set_wallpaper_reload_no_delay_for_test();
-    controller_->InitializePathsForTesting(user_data_dir, wallpapers_dir,
-                                           custom_wallpapers_dir);
-  }
 
-  void TearDown() override {
-    base::PostTaskWithTraits(FROM_HERE,
-                             {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
-                              base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
-                             base::Bind(&DeleteWallpaperDirectories));
-    AshTestBase::TearDown();
+    ASSERT_TRUE(user_data_dir_.CreateUniqueTempDir());
+    ASSERT_TRUE(online_wallpaper_dir_.CreateUniqueTempDir());
+    ASSERT_TRUE(custom_wallpaper_dir_.CreateUniqueTempDir());
+    controller_->InitializePathsForTesting(user_data_dir_.GetPath(),
+                                           online_wallpaper_dir_.GetPath(),
+                                           custom_wallpaper_dir_.GetPath());
   }
 
   WallpaperView* wallpaper_view() {
@@ -446,34 +430,35 @@ class WallpaperControllerTest : public AshTestBase {
   // want to test loading of default wallpapers.
   void CreateDefaultWallpapers() {
     base::ScopedAllowBlockingForTesting allow_blocking;
-    wallpaper_dir_.reset(new base::ScopedTempDir);
-    ASSERT_TRUE(wallpaper_dir_->CreateUniqueTempDir());
+    ASSERT_TRUE(default_wallpaper_dir_.CreateUniqueTempDir());
+    const base::FilePath default_wallpaper_path =
+        default_wallpaper_dir_.GetPath();
 
     base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-    const base::FilePath small_file = wallpaper_dir_->GetPath().Append(
-        FILE_PATH_LITERAL(kDefaultSmallWallpaperName));
+    const base::FilePath small_file =
+        default_wallpaper_path.Append(kDefaultSmallWallpaperName);
     command_line->AppendSwitchASCII(chromeos::switches::kDefaultWallpaperSmall,
                                     small_file.value());
-    const base::FilePath large_file = wallpaper_dir_->GetPath().Append(
-        FILE_PATH_LITERAL(kDefaultLargeWallpaperName));
+    const base::FilePath large_file =
+        default_wallpaper_path.Append(kDefaultLargeWallpaperName);
     command_line->AppendSwitchASCII(chromeos::switches::kDefaultWallpaperLarge,
                                     large_file.value());
 
-    const base::FilePath guest_small_file = wallpaper_dir_->GetPath().Append(
-        FILE_PATH_LITERAL(kGuestSmallWallpaperName));
+    const base::FilePath guest_small_file =
+        default_wallpaper_path.Append(kGuestSmallWallpaperName);
     command_line->AppendSwitchASCII(chromeos::switches::kGuestWallpaperSmall,
                                     guest_small_file.value());
-    const base::FilePath guest_large_file = wallpaper_dir_->GetPath().Append(
-        FILE_PATH_LITERAL(kGuestLargeWallpaperName));
+    const base::FilePath guest_large_file =
+        default_wallpaper_path.Append(kGuestLargeWallpaperName);
     command_line->AppendSwitchASCII(chromeos::switches::kGuestWallpaperLarge,
                                     guest_large_file.value());
 
-    const base::FilePath child_small_file = wallpaper_dir_->GetPath().Append(
-        FILE_PATH_LITERAL(kChildSmallWallpaperName));
+    const base::FilePath child_small_file =
+        default_wallpaper_path.Append(kChildSmallWallpaperName);
     command_line->AppendSwitchASCII(chromeos::switches::kChildWallpaperSmall,
                                     child_small_file.value());
-    const base::FilePath child_large_file = wallpaper_dir_->GetPath().Append(
-        FILE_PATH_LITERAL(kChildLargeWallpaperName));
+    const base::FilePath child_large_file =
+        default_wallpaper_path.Append(kChildLargeWallpaperName);
     command_line->AppendSwitchASCII(chromeos::switches::kChildWallpaperLarge,
                                     child_large_file.value());
 
@@ -534,6 +519,10 @@ class WallpaperControllerTest : public AshTestBase {
 
   int GetWallpaperCount() { return controller_->wallpaper_count_for_testing_; }
 
+  const std::vector<base::FilePath>& GetDecodeFilePaths() {
+    return controller_->decode_requests_for_testing_;
+  }
+
   void SetBypassDecode() { controller_->bypass_decode_for_testing_ = true; }
 
   void ClearWallpaperCount() { controller_->wallpaper_count_for_testing_ = 0; }
@@ -542,22 +531,12 @@ class WallpaperControllerTest : public AshTestBase {
     controller_->decode_requests_for_testing_.clear();
   }
 
-  bool CompareDecodeFilePaths(const std::vector<base::FilePath> expected) {
-    if (controller_->decode_requests_for_testing_.size() != expected.size())
-      return false;
-
-    for (size_t i = 0; i < expected.size(); ++i) {
-      if (controller_->decode_requests_for_testing_[i] != expected[i])
-        return false;
-    }
-    return true;
-  }
-
   WallpaperController* controller_;  // Not owned.
 
-  // Directory created by |CreateDefaultWallpapers| to store default wallpaper
-  // images.
-  std::unique_ptr<base::ScopedTempDir> wallpaper_dir_;
+  base::ScopedTempDir user_data_dir_;
+  base::ScopedTempDir online_wallpaper_dir_;
+  base::ScopedTempDir custom_wallpaper_dir_;
+  base::ScopedTempDir default_wallpaper_dir_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(WallpaperControllerTest);
@@ -1234,8 +1213,9 @@ TEST_F(WallpaperControllerTest, SetDefaultWallpaperForRegularAccount) {
   RunAllTasksUntilIdle();
   EXPECT_EQ(1, GetWallpaperCount());
   EXPECT_EQ(controller_->GetWallpaperType(), DEFAULT);
-  EXPECT_TRUE(CompareDecodeFilePaths(
-      {wallpaper_dir_->GetPath().Append(kDefaultLargeWallpaperName)}));
+  ASSERT_EQ(1u, GetDecodeFilePaths().size());
+  EXPECT_EQ(default_wallpaper_dir_.GetPath().Append(kDefaultLargeWallpaperName),
+            GetDecodeFilePaths()[0]);
 
   EXPECT_TRUE(controller_->GetUserWallpaperInfo(account_id_1, &wallpaper_info,
                                                 false /*is_ephemeral=*/));
@@ -1254,8 +1234,9 @@ TEST_F(WallpaperControllerTest, SetDefaultWallpaperForRegularAccount) {
   RunAllTasksUntilIdle();
   EXPECT_EQ(1, GetWallpaperCount());
   EXPECT_EQ(controller_->GetWallpaperType(), DEFAULT);
-  EXPECT_TRUE(CompareDecodeFilePaths(
-      {wallpaper_dir_->GetPath().Append(kDefaultSmallWallpaperName)}));
+  ASSERT_EQ(1u, GetDecodeFilePaths().size());
+  EXPECT_EQ(default_wallpaper_dir_.GetPath().Append(kDefaultSmallWallpaperName),
+            GetDecodeFilePaths()[0]);
 
   EXPECT_TRUE(controller_->GetUserWallpaperInfo(account_id_1, &wallpaper_info,
                                                 false /*is_ephemeral=*/));
@@ -1274,8 +1255,9 @@ TEST_F(WallpaperControllerTest, SetDefaultWallpaperForRegularAccount) {
   RunAllTasksUntilIdle();
   EXPECT_EQ(1, GetWallpaperCount());
   EXPECT_EQ(controller_->GetWallpaperType(), DEFAULT);
-  EXPECT_TRUE(CompareDecodeFilePaths(
-      {wallpaper_dir_->GetPath().Append(kDefaultSmallWallpaperName)}));
+  ASSERT_EQ(1u, GetDecodeFilePaths().size());
+  EXPECT_EQ(default_wallpaper_dir_.GetPath().Append(kDefaultSmallWallpaperName),
+            GetDecodeFilePaths()[0]);
 
   EXPECT_TRUE(controller_->GetUserWallpaperInfo(account_id_1, &wallpaper_info,
                                                 false /*is_ephemeral=*/));
@@ -1304,8 +1286,9 @@ TEST_F(WallpaperControllerTest, SetDefaultWallpaperForChildAccount) {
   RunAllTasksUntilIdle();
   EXPECT_EQ(1, GetWallpaperCount());
   EXPECT_EQ(controller_->GetWallpaperType(), DEFAULT);
-  EXPECT_TRUE(CompareDecodeFilePaths(
-      {wallpaper_dir_->GetPath().Append(kChildLargeWallpaperName)}));
+  ASSERT_EQ(1u, GetDecodeFilePaths().size());
+  EXPECT_EQ(default_wallpaper_dir_.GetPath().Append(kChildLargeWallpaperName),
+            GetDecodeFilePaths()[0]);
 
   // Verify the small child wallpaper is set successfully with the correct file
   // path.
@@ -1319,8 +1302,9 @@ TEST_F(WallpaperControllerTest, SetDefaultWallpaperForChildAccount) {
   RunAllTasksUntilIdle();
   EXPECT_EQ(1, GetWallpaperCount());
   EXPECT_EQ(controller_->GetWallpaperType(), DEFAULT);
-  EXPECT_TRUE(CompareDecodeFilePaths(
-      {wallpaper_dir_->GetPath().Append(kChildSmallWallpaperName)}));
+  ASSERT_EQ(1u, GetDecodeFilePaths().size());
+  EXPECT_EQ(default_wallpaper_dir_.GetPath().Append(kChildSmallWallpaperName),
+            GetDecodeFilePaths()[0]);
 }
 
 TEST_F(WallpaperControllerTest, SetDefaultWallpaperForGuestSession) {
@@ -1353,8 +1337,9 @@ TEST_F(WallpaperControllerTest, SetDefaultWallpaperForGuestSession) {
   EXPECT_TRUE(controller_->GetUserWallpaperInfo(account_id_1, &wallpaper_info,
                                                 false /*is_ephemeral=*/));
   EXPECT_EQ(wallpaper_info, default_wallpaper_info);
-  EXPECT_TRUE(CompareDecodeFilePaths(
-      {wallpaper_dir_->GetPath().Append(kGuestLargeWallpaperName)}));
+  ASSERT_EQ(1u, GetDecodeFilePaths().size());
+  EXPECT_EQ(default_wallpaper_dir_.GetPath().Append(kGuestLargeWallpaperName),
+            GetDecodeFilePaths()[0]);
 
   UpdateDisplay("800x600");
   RunAllTasksUntilIdle();
@@ -1364,8 +1349,9 @@ TEST_F(WallpaperControllerTest, SetDefaultWallpaperForGuestSession) {
   RunAllTasksUntilIdle();
   EXPECT_EQ(1, GetWallpaperCount());
   EXPECT_EQ(controller_->GetWallpaperType(), DEFAULT);
-  EXPECT_TRUE(CompareDecodeFilePaths(
-      {wallpaper_dir_->GetPath().Append(kGuestSmallWallpaperName)}));
+  ASSERT_EQ(1u, GetDecodeFilePaths().size());
+  EXPECT_EQ(default_wallpaper_dir_.GetPath().Append(kGuestSmallWallpaperName),
+            GetDecodeFilePaths()[0]);
 }
 
 TEST_F(WallpaperControllerTest, IgnoreWallpaperRequestInKioskMode) {
@@ -1532,12 +1518,16 @@ TEST_F(WallpaperControllerTest, VerifyWallpaperCache) {
 // on the desktop resolution.
 TEST_F(WallpaperControllerTest, ShowCustomWallpaperWithCorrectResolution) {
   CreateDefaultWallpapers();
-  base::FilePath small_wallpaper_path =
+  const base::FilePath small_custom_wallpaper_path =
       GetCustomWallpaperPath(WallpaperController::kSmallWallpaperSubDir,
                              wallpaper_files_id_1, file_name_1);
-  base::FilePath large_wallpaper_path =
+  const base::FilePath large_custom_wallpaper_path =
       GetCustomWallpaperPath(WallpaperController::kLargeWallpaperSubDir,
                              wallpaper_files_id_1, file_name_1);
+  const base::FilePath small_default_wallpaper_path =
+      default_wallpaper_dir_.GetPath().Append(kDefaultSmallWallpaperName);
+  const base::FilePath large_default_wallpaper_path =
+      default_wallpaper_dir_.GetPath().Append(kDefaultLargeWallpaperName);
 
   CreateAndSaveWallpapers(account_id_1);
   controller_->ShowUserWallpaper(InitializeUser(account_id_1));
@@ -1547,9 +1537,9 @@ TEST_F(WallpaperControllerTest, ShowCustomWallpaperWithCorrectResolution) {
   // is also expected. (Because unit tests don't support actual wallpaper
   // decoding, it falls back to the default wallpaper.)
   EXPECT_EQ(1, GetWallpaperCount());
-  EXPECT_TRUE(CompareDecodeFilePaths(
-      {small_wallpaper_path,
-       wallpaper_dir_->GetPath().Append(kDefaultSmallWallpaperName)}));
+  ASSERT_EQ(2u, GetDecodeFilePaths().size());
+  EXPECT_EQ(small_custom_wallpaper_path, GetDecodeFilePaths()[0]);
+  EXPECT_EQ(small_default_wallpaper_path, GetDecodeFilePaths()[1]);
 
   // Hook up another 800x600 display. This shouldn't trigger a reload.
   ClearWallpaperCount();
@@ -1557,7 +1547,7 @@ TEST_F(WallpaperControllerTest, ShowCustomWallpaperWithCorrectResolution) {
   UpdateDisplay("800x600,800x600");
   RunAllTasksUntilIdle();
   EXPECT_EQ(0, GetWallpaperCount());
-  EXPECT_TRUE(CompareDecodeFilePaths({}));
+  EXPECT_EQ(0u, GetDecodeFilePaths().size());
 
   // Detach the secondary display.
   UpdateDisplay("800x600");
@@ -1569,9 +1559,9 @@ TEST_F(WallpaperControllerTest, ShowCustomWallpaperWithCorrectResolution) {
   UpdateDisplay("800x600,2000x2000");
   RunAllTasksUntilIdle();
   EXPECT_EQ(1, GetWallpaperCount());
-  EXPECT_TRUE(CompareDecodeFilePaths(
-      {large_wallpaper_path,
-       wallpaper_dir_->GetPath().Append(kDefaultLargeWallpaperName)}));
+  ASSERT_EQ(2u, GetDecodeFilePaths().size());
+  EXPECT_EQ(large_custom_wallpaper_path, GetDecodeFilePaths()[0]);
+  EXPECT_EQ(large_default_wallpaper_path, GetDecodeFilePaths()[1]);
 
   // Detach the secondary display.
   UpdateDisplay("800x600");
@@ -1583,9 +1573,9 @@ TEST_F(WallpaperControllerTest, ShowCustomWallpaperWithCorrectResolution) {
   UpdateDisplay("800x600,2000x2000");
   RunAllTasksUntilIdle();
   EXPECT_EQ(1, GetWallpaperCount());
-  EXPECT_TRUE(CompareDecodeFilePaths(
-      {large_wallpaper_path,
-       wallpaper_dir_->GetPath().Append(kDefaultLargeWallpaperName)}));
+  ASSERT_EQ(2u, GetDecodeFilePaths().size());
+  EXPECT_EQ(large_custom_wallpaper_path, GetDecodeFilePaths()[0]);
+  EXPECT_EQ(large_default_wallpaper_path, GetDecodeFilePaths()[1]);
 }
 
 // After the display is rotated, the sign in wallpaper should be kept. Test for
@@ -1601,8 +1591,9 @@ TEST_F(WallpaperControllerTest, SigninWallpaperIsKeptAfterRotation) {
   // is expected.
   EXPECT_EQ(1, GetWallpaperCount());
   EXPECT_EQ(controller_->GetWallpaperType(), DEFAULT);
-  EXPECT_TRUE(CompareDecodeFilePaths(
-      {wallpaper_dir_->GetPath().Append(kDefaultSmallWallpaperName)}));
+  ASSERT_EQ(1u, GetDecodeFilePaths().size());
+  EXPECT_EQ(default_wallpaper_dir_.GetPath().Append(kDefaultSmallWallpaperName),
+            GetDecodeFilePaths()[0]);
 
   ClearWallpaperCount();
   ClearDecodeFilePaths();
@@ -1612,8 +1603,9 @@ TEST_F(WallpaperControllerTest, SigninWallpaperIsKeptAfterRotation) {
   RunAllTasksUntilIdle();
   EXPECT_EQ(1, GetWallpaperCount());
   EXPECT_EQ(controller_->GetWallpaperType(), DEFAULT);
-  EXPECT_TRUE(CompareDecodeFilePaths(
-      {wallpaper_dir_->GetPath().Append(kDefaultSmallWallpaperName)}));
+  ASSERT_EQ(1u, GetDecodeFilePaths().size());
+  EXPECT_EQ(default_wallpaper_dir_.GetPath().Append(kDefaultSmallWallpaperName),
+            GetDecodeFilePaths()[0]);
 }
 
 // Display size change should trigger reload for both user wallpaper and preview