Convert profile import IPCs to Mojo

chrome/utility/profile_import_handler.cc

 // Copyright (c) 2012 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/profile_import_handler.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
 #include "base/single_thread_task_runner.h"

[sky, 2016/11/29 23:56:28]

Remove any includes that are no longer applicable.

[tibell, 2016/11/30 03:36:47]

Still used below by the call to task_runner().

 #include "base/threading/thread.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
-#include "chrome/common/importer/profile_import_process_messages.h"
 #include "chrome/utility/importer/external_process_importer_bridge.h"
 #include "chrome/utility/importer/importer.h"
 #include "chrome/utility/importer/importer_creator.h"
 #include "content/public/utility/utility_thread.h"
+#include "mojo/public/cpp/bindings/strong_binding.h"
 
-namespace {
-
-bool Send(IPC::Message* message) {
-  return content::UtilityThread::Get()->Send(message);
+// static

[sky, 2016/11/29 23:56:28]

Declaration and definition order should match (see style guide).

[tibell, 2016/11/30 03:36:47]

Done.

[sky, 2016/11/30 16:19:07]

Did you miss this one? This function should be after the destructor.

[tibell, 2016/11/30 23:56:26]

Done.

+void ProfileImportHandler::Create(
+    mojo::InterfaceRequest<chrome::mojom::ProfileImport> request) {
+  mojo::MakeStrongBinding(base::MakeUnique<ProfileImportHandler>(),
+                          std::move(request));
 }
 
-}  // namespace
-
 ProfileImportHandler::ProfileImportHandler() : items_to_import_(0) {}
 
 ProfileImportHandler::~ProfileImportHandler() {}
 
-bool ProfileImportHandler::OnMessageReceived(const IPC::Message& message) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(ProfileImportHandler, message)
-    IPC_MESSAGE_HANDLER(ProfileImportProcessMsg_StartImport, OnImportStart)
-    IPC_MESSAGE_HANDLER(ProfileImportProcessMsg_CancelImport, OnImportCancel)
-    IPC_MESSAGE_HANDLER(ProfileImportProcessMsg_ReportImportItemFinished,
-                        OnImportItemFinished)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
-}
-
-void ProfileImportHandler::OnImportStart(
+void ProfileImportHandler::StartImport(
     const importer::SourceProfile& source_profile,
     uint16_t items,
-    const base::DictionaryValue& localized_strings) {
+    const base::DictionaryValue& localized_strings,
+    chrome::mojom::ProfileImportObserverPtr observer) {
   content::UtilityThread::Get()->EnsureBlinkInitialized();
-  bridge_ = new ExternalProcessImporterBridge(
-      localized_strings,
-      content::UtilityThread::Get(),
-      base::ThreadTaskRunnerHandle::Get().get());
   importer_ = importer::CreateImporterByType(source_profile.importer_type);
   if (!importer_.get()) {
-    Send(new ProfileImportProcessHostMsg_Import_Finished(
-        false, "Importer could not be created."));
+    observer->OnImportFinished(false, "Importer could not be created.");
     return;
   }
 
   items_to_import_ = items;
 
   // Create worker thread in which importer runs.
   import_thread_.reset(new base::Thread("import_thread"));
 #if defined(OS_WIN)
   import_thread_->init_com_with_mta(false);
 #endif
   if (!import_thread_->Start()) {
     NOTREACHED();
     ImporterCleanup();
   }
+  // Move the InterfacePtr to the import thread, as remaining messages will be
+  // sent from there:
+  observer.Bind(observer.PassInterface(), import_thread_->task_runner());

[sky, 2016/11/29 23:56:28]

The description of Bind indicates the runner "must belong to the same thread".
That doesn't seem to be the case here. I'm not familiar with InterfacePtr to
know if this is problematic. +yzshen

[tibell, 2016/11/30 03:36:47]

Talked this through with sammc@ and we decided that a ThreadSafeInterfacePtr is
the right choice here (we could try to post tasks around manually to achieve
e.g. deletion on the right thread, but that would basically just do what
ThreadSafeInterfacePtr does.)

[yzshen1, 2016/12/01 18:15:48]

Sorry for late reply. Just realized that I missed this.

Correct. The task runner must be on the same thread as where Bind is called. It
is used for task scheduling on the same thread.

+  bridge_ =
+      new ExternalProcessImporterBridge(localized_strings, std::move(observer));
   import_thread_->task_runner()->PostTask(
       FROM_HERE, base::Bind(&Importer::StartImport, importer_,
                             source_profile, items, base::RetainedRef(bridge_)));
 }
 
-void ProfileImportHandler::OnImportCancel() {
+void ProfileImportHandler::CancelImport() {
   ImporterCleanup();
 }
 
-void ProfileImportHandler::OnImportItemFinished(uint16_t item) {
+void ProfileImportHandler::ReportImportItemFinished(importer::ImportItem item) {
   items_to_import_ ^= item;  // Remove finished item from mask.
-  // If we've finished with all items, notify the browser process.
   if (items_to_import_ == 0) {
-    Send(new ProfileImportProcessHostMsg_Import_Finished(true, std::string()));
     ImporterCleanup();
   }
 }
 
 void ProfileImportHandler::ImporterCleanup() {
   importer_->Cancel();
   importer_ = NULL;
   bridge_ = NULL;
   import_thread_.reset();
   content::UtilityThread::Get()->ReleaseProcessIfNeeded();
 }