Convert utility process extension Unpacker IPC to mojo

chrome/browser/extensions/zipfile_installer.h

 // 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.
 
 #ifndef CHROME_BROWSER_EXTENSIONS_ZIPFILE_INSTALLER_H_
 #define CHROME_BROWSER_EXTENSIONS_ZIPFILE_INSTALLER_H_
 
 #include <memory>
 #include <string>
 
-#include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
-#include "content/public/browser/utility_process_host_client.h"
+#include "content/public/browser/utility_process_mojo_client.h"
+#include "extensions/common/extension_unpacker.mojom.h"

[Devlin, 2017/02/14 17:24:59]

Could this just be a forward declaration?

[Noel Gordon, 2017/02/15 17:28:08]

Possible, but not something we've conventionally done when converting utility
process IPC to mojo ... since mojo tends to be very namespace nested.  Once we
have compilers with nested namespace support, it'll be a lot easier to do then.

Maybe we should change the mojo code generator to create a fwd declaration .h or
something, dunno.

[Devlin, 2017/02/17 15:53:56]

Sorry, I don't follow - what's wrong with

namespace extensions {
namespace mojom {
class ExtensionUnpacker;
}

...

}  // namespace extensions
?

We have nested namespace forward declarations in a lot of places in the code...

[Noel Gordon, 2017/02/27 11:47:50]

OK done.

 
 class ExtensionService;
 
-namespace IPC {
-class Message;
-}
-
 namespace extensions {
 
-// ZipFileInstaller unzips an extension that is zipped up via a utility process.
-// The contents are then loaded via UnpackedInstaller.
-class ZipFileInstaller : public content::UtilityProcessHostClient {
+// ZipFileInstaller unzips an extension in a utility process. On success, the
+// extension content is loaded by an extensions::UnpackedInstaller.
+class ZipFileInstaller : public base::RefCountedThreadSafe<ZipFileInstaller> {
  public:
-  static scoped_refptr<ZipFileInstaller> Create(
-      ExtensionService* extension_service);
+  static scoped_refptr<ZipFileInstaller> Create(ExtensionService* service);
 
-  void LoadFromZipFile(const base::FilePath& path);
+  void LoadFromZipFile(const base::FilePath& zip_file);
 
   void set_be_noisy_on_failure(bool value) { be_noisy_on_failure_ = value; }
 
-  // UtilityProcessHostClient
-  bool OnMessageReceived(const IPC::Message& message) override;
+ private:
+  friend class base::RefCountedThreadSafe<ZipFileInstaller>;
 
- private:
-  explicit ZipFileInstaller(ExtensionService* extension_service);
-  ~ZipFileInstaller() override;
+  explicit ZipFileInstaller(ExtensionService* service);
+  ~ZipFileInstaller();
 
-  void PrepareTempDir();
-  void StartWorkOnIOThread(const base::FilePath& temp_dir);
-  void ReportSuccessOnUIThread(const base::FilePath& unzipped_path);
+  void PrepareTempDir(const base::FilePath& zip_file);
+  void UnzipOnUIThread(const base::FilePath& temp_dir);
+  void UnzipDone(bool success);
   void ReportErrorOnUIThread(const std::string& error);
 
-  void OnUnzipSucceeded(const base::FilePath& unzipped_path);
-  void OnUnzipFailed(const std::string& error);
-
   bool be_noisy_on_failure_;
   base::WeakPtr<ExtensionService> extension_service_weak_;
-  base::FilePath zip_path_;
+  base::FilePath zip_file_;
+  base::FilePath temp_dir_;

[Devlin, 2017/02/14 17:24:59]

Add member comments

[Noel Gordon, 2017/02/15 17:28:08]

When adding mojo to other folks code that we are not familiar with, we often
don't know what members do, so any added comments about them from us are
~useless :)

I added comments for the members vars I do know something about.

+
+  std::unique_ptr<
+      content::UtilityProcessMojoClient<extensions::mojom::ExtensionUnpacker>>
+      utility_process_mojo_client_;
 
   DISALLOW_COPY_AND_ASSIGN(ZipFileInstaller);
 };
 
 }  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_ZIPFILE_INSTALLER_H_