[ios] Simplify handling of ScopedTempDir with TestChromeBrowserState

Analogous to crrev.com/c/2332703 for iOS: the recently-introduced API
in base/test/test_file_util.h allows temp dirs to be created without
explicit lifetime control. Tests can assume the directory will be around
during the execution of the test, and it will get automatically deleted
afterwards.

Bug: 546640
Change-Id: I9c8dc59b8ae3963f0be692bfe302cdc8d01d9a83
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2335194Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#794968}

ios/chrome/browser/app_launcher/app_launcher_tab_helper_unittest.mm

@@ -8,6 +8,7 @@
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
+#include "base/files/scoped_temp_dir.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/task_environment.h"
 #include "base/time/default_clock.h"

ios/chrome/browser/browser_state/test_chrome_browser_state.h

@@ -10,7 +10,6 @@
 #include <vector>
 
 #include "base/files/file_path.h"
-#include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/sequenced_task_runner.h"
 #include "components/keyed_service/ios/browser_state_keyed_service_factory.h"
@@ -144,11 +143,6 @@ class TestChromeBrowserState : public ChromeBrowserState {
   // and off-the-record TestChromeBrowserState has been created.
   void Init();
 
-  // We use a temporary directory to store testing browser state data.
-  // This must be declared before anything that may make use of the
-  // directory so as to ensure files are closed before cleanup.
-  base::ScopedTempDir temp_dir_;
-
   // The path to this browser state.
   base::FilePath state_path_;
 

ios/chrome/browser/browser_state/test_chrome_browser_state.mm

@@ -16,6 +16,7 @@
 #include "base/run_loop.h"
 #include "base/task/post_task.h"
 #include "base/task/thread_pool.h"
+#include "base/test/test_file_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/common/bookmark_constants.h"
@@ -86,33 +87,6 @@ std::unique_ptr<KeyedService> BuildWebDataService(web::BrowserState* context) {
       base::DoNothing());
 }
 
-base::FilePath CreateTempBrowserStateDir(base::ScopedTempDir* temp_dir) {
-  DCHECK(temp_dir);
-  if (!temp_dir->CreateUniqueTempDir()) {
-    // Fallback logic in case we fail to create unique temporary directory.
-    LOG(ERROR) << "Failed to create unique temporary directory.";
-    base::FilePath system_tmp_dir;
-    bool success = base::PathService::Get(base::DIR_TEMP, &system_tmp_dir);
-
-    // We're severely screwed if we can't get the system temporary
-    // directory. Die now to avoid writing to the filesystem root
-    // or other bad places.
-    CHECK(success);
-
-    base::FilePath fallback_dir(
-        system_tmp_dir.Append(FILE_PATH_LITERAL("TestChromeBrowserStatePath")));
-    base::DeletePathRecursively(fallback_dir);
-    base::CreateDirectory(fallback_dir);
-    if (!temp_dir->Set(fallback_dir)) {
-      // That shouldn't happen, but if it does, try to recover.
-      LOG(ERROR) << "Failed to use a fallback temporary directory.";
-
-      // We're screwed if this fails, see CHECK above.
-      CHECK(temp_dir->Set(system_tmp_dir));
-    }
-  }
-  return temp_dir->GetPath();
-}
 }  // namespace
 
 TestChromeBrowserState::TestChromeBrowserState(
@@ -160,12 +134,6 @@ TestChromeBrowserState::~TestChromeBrowserState() {
 
   BrowserStateDependencyManager::GetInstance()->DestroyBrowserStateServices(
       this);
-  // The destructor of temp_dir_ will perform IO, so it needs to be deleted
-  // here while |allow_bocking| is still in scope. Keeps the same logic as
-  // ScopedTempDir::~ScopedTempDir().
-  if (temp_dir_.IsValid()) {
-    ignore_result(temp_dir_.Delete());
-  }
 }
 
 void TestChromeBrowserState::Init() {
@@ -178,7 +146,7 @@ void TestChromeBrowserState::Init() {
          web::WebThread::CurrentlyOn(web::WebThread::UI));
 
   if (state_path_.empty())
-    state_path_ = CreateTempBrowserStateDir(&temp_dir_);
+    state_path_ = base::CreateUniqueTempDirectoryScopedToTest();
 
   if (IsOffTheRecord())
     state_path_ = state_path_.Append(FILE_PATH_LITERAL("OTR"));

ios/chrome/browser/history/history_tab_helper_unittest.mm

@@ -6,7 +6,6 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
-#include "base/files/scoped_temp_dir.h"
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/bind_test_util.h"
@@ -31,10 +30,6 @@ class HistoryTabHelperTest : public PlatformTest {
  public:
   void SetUp() override {
     TestChromeBrowserState::Builder test_cbs_builder;
-
-    ASSERT_TRUE(state_dir_.CreateUniqueTempDir());
-    test_cbs_builder.SetPath(state_dir_.GetPath());
-
     chrome_browser_state_ = test_cbs_builder.Build();
     ASSERT_TRUE(chrome_browser_state_->CreateHistoryService());
 
@@ -72,11 +67,6 @@ class HistoryTabHelperTest : public PlatformTest {
   }
 
  protected:
-  // A state directory that outlives |task_environment_| is needed because
-  // CreateHistoryService/CreateBookmarkModel use the directory to host
-  // databases. See https://crbug.com/546640 for more details.
-  base::ScopedTempDir state_dir_;
-
   base::test::TaskEnvironment task_environment_;
   std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
   web::TestWebState web_state_;

ios/chrome/browser/ui/popup_menu/popup_menu_mediator_unittest.mm

@@ -4,6 +4,7 @@
 
 #import "ios/chrome/browser/ui/popup_menu/popup_menu_mediator.h"
 
+#include "base/files/scoped_temp_dir.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/time/default_clock.h"