c/browser, c/common, components M-N: Change ScopedTempDir::path() to GetPath()

chrome/browser/media_galleries/fileapi/itunes_file_util_unittest.cc

Index: chrome/browser/media_galleries/fileapi/itunes_file_util_unittest.cc
diff --git a/chrome/browser/media_galleries/fileapi/itunes_file_util_unittest.cc b/chrome/browser/media_galleries/fileapi/itunes_file_util_unittest.cc
index 9f9323c57721757259e1f4a7eed92e696b2fbd20..256b56ebf83b5667ed4417971877ad06317d3a78 100644
--- a/chrome/browser/media_galleries/fileapi/itunes_file_util_unittest.cc
+++ b/chrome/browser/media_galleries/fileapi/itunes_file_util_unittest.cc
@@ -83,19 +83,22 @@ class TestITunesDataProvider : public ITunesDataProvider {
   }
 
   const base::FilePath& auto_add_path() const override {
-    return fake_auto_add_dir_.path();
+    return fake_auto_add_dir_path_;
   }
 
   void SetProvideAutoAddDir(bool provide_auto_add_dir) {
     if (provide_auto_add_dir) {
       if (!fake_auto_add_dir_.IsValid())
         ASSERT_TRUE(fake_auto_add_dir_.CreateUniqueTempDir());
+      fake_auto_add_dir_path_ = fake_auto_add_dir_.GetPath();
     } else {
       ASSERT_TRUE(fake_auto_add_dir_.Delete());
+      fake_auto_add_dir_path_.clear();
     }
   }
 
  private:
+  base::FilePath fake_auto_add_dir_path_;

[vabr (Chromium), 2016/09/14 16:22:17]

Before: ScopedTempDir needed to trust the code calling auto_add_path() that the
path is not used if empty.
Now: With adding fake_auto_add_dir_path_ auto_add_path() needs to trust its
callers that they won't use the path for file access if the path is empty.

In other words, there is still potential for misuse, but at least the scope is
now more limited.

[Reilly Grant (use Gerrit), 2016/09/14 18:06:19]

You could add an ASSERT_FALSE(fake_auto_add_dir_path_.empty()) to
auto_add_path().

[vabr (Chromium), 2016/09/14 20:14:39]

The trouble is that it can actually be empty:
ItunesMediaDirectoryContentsNoAutoAdd instructs the data provider not to have
the directory. The empty path is still taken from the provider in
itunes::ITunesFileUtil::ReadDirectorySync, but fortunately only to check if it
is empty, not to write files under it in the empty case.

I guess this kind of trust in the code is OK inside
chrome/browser/media_galleries/fileapi, where a single person is likely to
understand the impacted code. It is problematic in the case when
base::ScopedTempDir should be vending the empty path not knowing what the caller
is going to do with it.

   base::ScopedTempDir fake_auto_add_dir_;
 };
 
@@ -143,15 +146,12 @@ class ItunesFileUtilTest : public testing::Test {
 
   void SetUpDataProvider() {
     ASSERT_TRUE(fake_library_dir_.CreateUniqueTempDir());
-    ASSERT_EQ(
-        0,
-        base::WriteFile(
-            fake_library_dir_.path().AppendASCII(kITunesLibraryXML),
-            NULL,
-            0));
+    ASSERT_EQ(0, base::WriteFile(
+                     fake_library_dir_.GetPath().AppendASCII(kITunesLibraryXML),
+                     NULL, 0));
 
     itunes_data_provider_.reset(
-        new TestITunesDataProvider(fake_library_dir_.path()));
+        new TestITunesDataProvider(fake_library_dir_.GetPath()));
   }
 
   void SetUp() override {
@@ -176,7 +176,7 @@ class ItunesFileUtilTest : public testing::Test {
     media_path_filter_.reset(new MediaPathFilter());
     ScopedVector<storage::FileSystemBackend> additional_providers;
     additional_providers.push_back(new TestMediaFileSystemBackend(
-        profile_dir_.path(),
+        profile_dir_.GetPath(),
         new TestITunesFileUtil(media_path_filter_.get(),
                                itunes_data_provider_.get())));
 
@@ -185,8 +185,8 @@ class ItunesFileUtilTest : public testing::Test {
         base::ThreadTaskRunnerHandle::Get().get(),
         storage::ExternalMountPoints::CreateRefCounted().get(),
         storage_policy.get(), NULL, std::move(additional_providers),
-        std::vector<storage::URLRequestAutoMountHandler>(), profile_dir_.path(),
-        content::CreateAllowFileAccessOptions());
+        std::vector<storage::URLRequestAutoMountHandler>(),
+        profile_dir_.GetPath(), content::CreateAllowFileAccessOptions());
   }
 
  protected: