Convert utility process ImageWriter IPC to mojo

chrome/utility/image_writer/image_writer_handler.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/utility/image_writer/image_writer_handler.h"
 
+#include "base/bind.h"
 #include "base/files/file_path.h"
-#include "chrome/common/extensions/chrome_utility_extensions_messages.h"
+#include "base/optional.h"
+#include "chrome/common/extensions/removable_storage_writer.mojom.h"
 #include "chrome/utility/image_writer/error_messages.h"
-#include "content/public/utility/utility_thread.h"
+
+namespace {
+
+bool IsTestDevice(const base::FilePath& device) {
+  return device.AsUTF8Unsafe() ==
+         extensions::mojom::RemovableStorageWriter::kTestDevice;
+}
+
+base::FilePath MakeTestDevicePath(const base::FilePath& image) {
+  return image.ReplaceExtension(FILE_PATH_LITERAL("out"));
+}
+
+}  // namespace
 
 namespace image_writer {
 
-ImageWriterHandler::ImageWriterHandler() {}
-ImageWriterHandler::~ImageWriterHandler() {}
+ImageWriterHandler::ImageWriterHandler() = default;
 
-void ImageWriterHandler::SendSucceeded() {
-  Send(new ChromeUtilityHostMsg_ImageWriter_Succeeded());
-  content::UtilityThread::Get()->ReleaseProcessIfNeeded();
-}
+ImageWriterHandler::~ImageWriterHandler() = default;
 
-void ImageWriterHandler::SendCancelled() {
-  Send(new ChromeUtilityHostMsg_ImageWriter_Cancelled());
-  content::UtilityThread::Get()->ReleaseProcessIfNeeded();
-}
+void ImageWriterHandler::Write(
+    const base::FilePath& image,
+    const base::FilePath& device,
+    extensions::mojom::RemovableStorageWriterClientPtr client) {
+  client_ = std::move(client);
+  client_.set_connection_error_handler(
+      base::Bind(&ImageWriterHandler::Cancel, base::Unretained(this)));
 
-void ImageWriterHandler::SendFailed(const std::string& message) {
-  Send(new ChromeUtilityHostMsg_ImageWriter_Failed(message));
-  content::UtilityThread::Get()->ReleaseProcessIfNeeded();
-}
+  base::FilePath target_device = device;
+  const bool test_mode = IsTestDevice(device);
+  if (test_mode)
+    target_device = MakeTestDevicePath(image);
 
-void ImageWriterHandler::SendProgress(int64_t progress) {
-  Send(new ChromeUtilityHostMsg_ImageWriter_Progress(progress));
-}
-
-void ImageWriterHandler::Send(IPC::Message* msg) {
-  content::UtilityThread::Get()->Send(msg);
-}
-
-bool ImageWriterHandler::OnMessageReceived(const IPC::Message& message) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(ImageWriterHandler, message)
-  IPC_MESSAGE_HANDLER(ChromeUtilityMsg_ImageWriter_Write, OnWriteStart)
-  IPC_MESSAGE_HANDLER(ChromeUtilityMsg_ImageWriter_Verify, OnVerifyStart)
-  IPC_MESSAGE_HANDLER(ChromeUtilityMsg_ImageWriter_Cancel, OnCancel)
-  IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
-}
-
-void ImageWriterHandler::OnWriteStart(const base::FilePath& image,
-                                      const base::FilePath& device) {
+  // http://crbug.com/352442

[dcheng, 2017/02/08 09:31:18]

Nit: pointer to the bug is good, but a brief explanation of why this logic is
necessary would be useful as well

Also, http => https ;-)

[Noel Gordon, 2017/02/08 12:12:59]

Done.  I think the URL is the best pointer I can give to the details.  Also a
brief explanation of the bug is given in the ChangeLog, and that saves me
repeating myself twice :)

Comments are not code, and too often don't match the actual code, leading to
maintenance burden and other issues.  Better to rephrase them into actual code
if possible.  It's not always possible, so ensure the comment adds value. [1]
  
[1] https://lists.webkit.org/pipermail/webkit-dev/2011-January/015769.html

[dcheng, 2017/02/09 01:25:36]

Hmm... I personally don't like bug URLs as the only explanation, because you
often have to read through all the comments and try to reverse engineer what the
original bug was from the comments/diffs. As best as I can tell, we just want to
avoid creating a new ImageWriter if the image path and the target device are
identical, so we can keep any existing file handles open.

Given that the Mojo set up / ImageWriter setup looks largely shared, another
option is to just factor it out into a helper method. Then the comment won't be
duplicated =)

   if (!image_writer_.get() || image != image_writer_->GetImagePath() ||
-      device != image_writer_->GetDevicePath()) {
-    image_writer_.reset(new ImageWriter(this, image, device));
+      target_device != image_writer_->GetDevicePath()) {
+    image_writer_.reset(new ImageWriter(this, image, target_device));
   }
 
   if (image_writer_->IsRunning()) {
     SendFailed(error::kOperationAlreadyInProgress);
     return;
   }
 
+  if (test_mode) {
+    image_writer_->Write();
+    return;
+  }
+
   if (!image_writer_->IsValidDevice()) {
     SendFailed(error::kInvalidDevice);
     return;
   }
 
   image_writer_->UnmountVolumes(
       base::Bind(&ImageWriter::Write, image_writer_->AsWeakPtr()));
 }
 
-void ImageWriterHandler::OnVerifyStart(const base::FilePath& image,
-                                       const base::FilePath& device) {
+void ImageWriterHandler::Verify(
+    const base::FilePath& image,
+    const base::FilePath& device,
+    extensions::mojom::RemovableStorageWriterClientPtr client) {
+  client_ = std::move(client);
+  client_.set_connection_error_handler(
+      base::Bind(&ImageWriterHandler::Cancel, base::Unretained(this)));
+
+  base::FilePath target_device = device;
+  const bool test_mode = IsTestDevice(device);
+  if (test_mode)
+    target_device = MakeTestDevicePath(image);
+
+  // http://crbug.com/352442

[dcheng, 2017/02/08 09:31:18]

Ditto.

[Noel Gordon, 2017/02/08 12:12:59]

Done.

   if (!image_writer_.get() || image != image_writer_->GetImagePath() ||
-      device != image_writer_->GetDevicePath()) {
-    image_writer_.reset(new ImageWriter(this, image, device));
+      target_device != image_writer_->GetDevicePath()) {
+    image_writer_.reset(new ImageWriter(this, image, target_device));
   }
 
   if (image_writer_->IsRunning()) {
     SendFailed(error::kOperationAlreadyInProgress);
     return;
   }
 
+  if (test_mode) {
+    image_writer_->Verify();
+    return;
+  }
+
   if (!image_writer_->IsValidDevice()) {
     SendFailed(error::kInvalidDevice);
     return;
   }
 
   image_writer_->Verify();
 }
 
-void ImageWriterHandler::OnCancel() {
-  if (image_writer_.get()) {
+void ImageWriterHandler::SendProgress(int64_t progress) {
+  client_->Progress(progress);
+}
+
+void ImageWriterHandler::SendSucceeded() {
+  client_->Complete(base::nullopt);
+  client_.reset();
+}
+
+void ImageWriterHandler::SendFailed(const std::string& error) {
+  client_->Complete(error);
+  client_.reset();
+}
+
+void ImageWriterHandler::Cancel() {
+  if (image_writer_)
     image_writer_->Cancel();
-  } else {
-    SendCancelled();
-  }
+  client_.reset();
 }
 
 }  // namespace image_writer