Avoid touching ScopedFileOpener::Runner's ref count before it's fully constructed

ScopedFileOpener::Runner is a ref counted object, and its first reference
used to be made in its constructor through base::Bind.
The reference is passed to ProvidedFileSystem::OpenFile, and released
when the callback object is destroyed.

However, if OpenFile failed, the reference is released before the
Runner construction has done. Then `new Runner` returns a stale pointer,
instead of newly created object.

This CL adds a static consntructor and moves the implicit ref count
manipulation out of the real constructor.

Bug: 866456
Change-Id: I92d68b3383d8fecd900be080cc23551cddb5f12b
Reviewed-on: https://chromium-review.googlesource.com/1156325Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579359}

chrome/browser/chromeos/file_system_provider/scoped_file_opener.cc

@@ -17,18 +17,16 @@ using OpenFileCallback = ProvidedFileSystemInterface::OpenFileCallback;
 class ScopedFileOpener::Runner
     : public base::RefCounted<ScopedFileOpener::Runner> {
  public:
-  Runner(ProvidedFileSystemInterface* file_system,
-         const base::FilePath& file_path,
-         OpenFileMode mode,
-         OpenFileCallback callback)
-      : file_system_(file_system->GetWeakPtr()),
-        open_callback_(std::move(callback)),
-        aborting_requested_(false),
-        open_completed_(false),
-        file_handle_(0) {
-    abort_callback_ = file_system_->OpenFile(
+  static scoped_refptr<Runner> Create(ProvidedFileSystemInterface* file_system,
+                                      const base::FilePath& file_path,
+                                      OpenFileMode mode,
+                                      OpenFileCallback callback) {
+    auto runner =
+        base::WrapRefCounted(new Runner(file_system, std::move(callback)));
+    runner->abort_callback_ = file_system->OpenFile(
         file_path, mode,
-        base::Bind(&ScopedFileOpener::Runner::OnOpenFileCompleted, this));
+        base::Bind(&ScopedFileOpener::Runner::OnOpenFileCompleted, runner));
+    return runner;
   }
 
   // Aborts pending open operation, or closes a file if it's already opened.
@@ -59,6 +57,13 @@ class ScopedFileOpener::Runner
  private:
   friend class base::RefCounted<ScopedFileOpener::Runner>;
 
+  Runner(ProvidedFileSystemInterface* file_system, OpenFileCallback callback)
+      : file_system_(file_system->GetWeakPtr()),
+        open_callback_(std::move(callback)),
+        aborting_requested_(false),
+        open_completed_(false),
+        file_handle_(0) {}
+
   ~Runner() {}
 
   // Called when opening is completed with either a success or an error.
@@ -127,7 +132,8 @@ ScopedFileOpener::ScopedFileOpener(ProvidedFileSystemInterface* file_system,
                                    const base::FilePath& file_path,
                                    OpenFileMode mode,
                                    OpenFileCallback callback)
-    : runner_(new Runner(file_system, file_path, mode, std::move(callback))) {}
+    : runner_(
+          Runner::Create(file_system, file_path, mode, std::move(callback))) {}
 
 ScopedFileOpener::~ScopedFileOpener() {
   runner_->AbortOrClose();