//chrome/browser/chromeos: Change ScopedTempDir::path() to GetPath()

chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Index: chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
diff --git a/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc b/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
index abc7a0fbf9372e038e147ae5df30d6bc71b79a00..6caf07b69503cb6a9a24be5b3f6991df8188a025 100644
--- a/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
+++ b/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
@@ -184,11 +184,19 @@ class TestVolume {
 
   bool CreateRootDirectory(const Profile* profile) {
     const base::FilePath path = profile->GetPath().Append(name_);
-    return root_.path() == path || root_.Set(path);
+    // Ensure that |path| is taken by |root_|. Cannot use |root_.GetPath()| to

[vabr (Chromium), 2016/09/12 19:40:37]

I'm aware that this is a bit hacky.

I was tempted to leave out the check involving |current_root_path|, but I was
not sure whether CreateRootDirectory needs to handle the case when |root_|
already owns a different directory.

[achuithb, 2016/09/13 20:55:19]

I'm really not sure either. Is it possible to pull this out into a separate CL?

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

Sure: https://codereview.chromium.org/2345473002
I realised that CreateRootDirectory is potentially called repeatedly from
FakeTestVolume, so I made it handle repeated calls. But there is no need to
assume that root_ was initialised from anywhere else, so I added a Boolean
member variable to track initialisation done by CreateRootDirectory, to make the
code clearer.

+    // check if it already is so, because GetPath checks that the current path
+    // is not empty.
+    base::FilePath current_root_path = root_.Take();

[achuithb, 2016/09/13 20:55:19]

const

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

Now obsolete after the above described changes (made in
https://codereview.chromium.org/2345473002).

+    if (!current_root_path.empty() && current_root_path != path) {
+      CHECK(root_.Set(current_root_path));
+      return false;
+    }
+    return root_.Set(path);
   }
 
   const std::string& name() { return name_; }
-  const base::FilePath root_path() { return root_.path(); }
+  const base::FilePath root_path() { return root_.GetPath(); }

[achuithb, 2016/09/13 20:55:19]

Get rid of this const return type, but make the function const. Please also make
name() a const function.

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

Looking at the callsites, they never move away the result, and often require a
const reference, so returning by value (const or not) would do one more copy
than needed. So I made this a const ref instead in
https://codereview.chromium.org/2345473002.

(On the topic of returning const values: sometimes returning a const value has
been done on purpose to prevent assigning to it, see
http://crbug.com/393155#c4.)

 
  private:
   std::string name_;