[Wait to be closed] [Translate] Migrate IPCs to Mojo interfaces.

components/translate/content/browser/content_translate_driver_impl.cc

Index: components/translate/content/browser/content_translate_driver_impl.cc
diff --git a/components/translate/content/browser/content_translate_driver.cc b/components/translate/content/browser/content_translate_driver_impl.cc
similarity index 63%
rename from components/translate/content/browser/content_translate_driver.cc
rename to components/translate/content/browser/content_translate_driver_impl.cc
index 24376c16d711385043017b4f44e6a4e94396375c..0aca86adac3e05693996f3db9d7a24b620dee457 100644
--- a/components/translate/content/browser/content_translate_driver.cc
+++ b/components/translate/content/browser/content_translate_driver_impl.cc
@@ -2,14 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/translate/content/browser/content_translate_driver.h"
+#include "components/translate/content/browser/content_translate_driver_impl.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
-#include "components/translate/content/common/translate_messages.h"
 #include "components/translate/core/browser/translate_download_manager.h"
 #include "components/translate/core/browser/translate_manager.h"
 #include "content/public/browser/browser_context.h"
@@ -33,28 +32,35 @@ const int kMaxTranslateLoadCheckAttempts = 20;
 
 namespace translate {
 
-ContentTranslateDriver::ContentTranslateDriver(
+ContentTranslateDriverImpl::ContentTranslateDriverImpl(
     content::NavigationController* nav_controller)
     : content::WebContentsObserver(nav_controller->GetWebContents()),
       navigation_controller_(nav_controller),
       translate_manager_(NULL),
       max_reload_check_attempts_(kMaxTranslateLoadCheckAttempts),
+      next_page_seq_no_(0),
       weak_pointer_factory_(this) {
   DCHECK(navigation_controller_);
 }
 
-ContentTranslateDriver::~ContentTranslateDriver() {}
+ContentTranslateDriverImpl::~ContentTranslateDriverImpl() {}
 
-void ContentTranslateDriver::AddObserver(Observer* observer) {
+void ContentTranslateDriverImpl::BindRequest(
+    mojom::ContentTranslateDriverRequest request) {
+  bindings_.AddBinding(this, std::move(request));
+}
+
+void ContentTranslateDriverImpl::AddObserver(Observer* observer) {
   observer_list_.AddObserver(observer);
 }
 
-void ContentTranslateDriver::RemoveObserver(Observer* observer) {
+void ContentTranslateDriverImpl::RemoveObserver(Observer* observer) {
   observer_list_.RemoveObserver(observer);
 }
 
-void ContentTranslateDriver::InitiateTranslation(const std::string& page_lang,
-                                                 int attempt) {
+void ContentTranslateDriverImpl::InitiateTranslation(
+    const std::string& page_lang,
+    int attempt) {
   if (translate_manager_->GetLanguageState().translation_pending())
     return;
 
@@ -66,7 +72,7 @@ void ContentTranslateDriver::InitiateTranslation(const std::string& page_lang,
     int backoff = attempt * kMaxTranslateLoadCheckAttempts;
     base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
         FROM_HERE,
-        base::Bind(&ContentTranslateDriver::InitiateTranslation,
+        base::Bind(&ContentTranslateDriverImpl::InitiateTranslation,
                    weak_pointer_factory_.GetWeakPtr(), page_lang, attempt + 1),
         base::TimeDelta::FromMilliseconds(backoff));
     return;
@@ -78,7 +84,7 @@ void ContentTranslateDriver::InitiateTranslation(const std::string& page_lang,
 
 // TranslateDriver methods
 
-bool ContentTranslateDriver::IsLinkNavigation() {
+bool ContentTranslateDriverImpl::IsLinkNavigation() {
   return navigation_controller_ &&
          navigation_controller_->GetLastCommittedEntry() &&
          ui::PageTransitionCoreTypeIs(
@@ -87,67 +93,70 @@ bool ContentTranslateDriver::IsLinkNavigation() {
              ui::PAGE_TRANSITION_LINK);
 }
 
-void ContentTranslateDriver::OnTranslateEnabledChanged() {
+void ContentTranslateDriverImpl::OnTranslateEnabledChanged() {
   content::WebContents* web_contents = navigation_controller_->GetWebContents();
-  FOR_EACH_OBSERVER(
-      Observer, observer_list_, OnTranslateEnabledChanged(web_contents));
+  FOR_EACH_OBSERVER(Observer, observer_list_,
+                    OnTranslateEnabledChanged(web_contents));
 }
 
-void ContentTranslateDriver::OnIsPageTranslatedChanged() {
-    content::WebContents* web_contents =
-        navigation_controller_->GetWebContents();
-    FOR_EACH_OBSERVER(
-        Observer, observer_list_, OnIsPageTranslatedChanged(web_contents));
+void ContentTranslateDriverImpl::OnIsPageTranslatedChanged() {
+  content::WebContents* web_contents = navigation_controller_->GetWebContents();
+  FOR_EACH_OBSERVER(Observer, observer_list_,
+                    OnIsPageTranslatedChanged(web_contents));
 }
 
-void ContentTranslateDriver::TranslatePage(int page_seq_no,
-                                           const std::string& translate_script,
-                                           const std::string& source_lang,
-                                           const std::string& target_lang) {
-  content::WebContents* web_contents = navigation_controller_->GetWebContents();
-  web_contents->GetMainFrame()->Send(new ChromeFrameMsg_TranslatePage(
-      web_contents->GetMainFrame()->GetRoutingID(), page_seq_no,
-      translate_script, source_lang, target_lang));
+void ContentTranslateDriverImpl::TranslatePage(
+    int page_seq_no,
+    const std::string& translate_script,
+    const std::string& source_lang,
+    const std::string& target_lang) {
+  auto it = mojo_pages_.find(page_seq_no);
+  if (it == mojo_pages_.end())
+    return;  // This page has navigated away.
+
+  it->second->Translate(
+      translate_script, source_lang, target_lang,
+      base::Bind(&ContentTranslateDriverImpl::OnPageTranslated,
+                 base::Unretained(this)));
 }
 
-void ContentTranslateDriver::RevertTranslation(int page_seq_no) {
-  content::WebContents* web_contents = navigation_controller_->GetWebContents();
-  web_contents->GetMainFrame()->Send(new ChromeFrameMsg_RevertTranslation(
-      web_contents->GetMainFrame()->GetRoutingID(), page_seq_no));
+void ContentTranslateDriverImpl::RevertTranslation(int page_seq_no) {
+  auto it = mojo_pages_.find(page_seq_no);
+  if (it == mojo_pages_.end())
+    return;  // This page has navigated away.
+
+  it->second->RevertTranslation();
 }
 
-bool ContentTranslateDriver::IsOffTheRecord() {
+bool ContentTranslateDriverImpl::IsOffTheRecord() {
   return navigation_controller_->GetBrowserContext()->IsOffTheRecord();
 }
 
-const std::string& ContentTranslateDriver::GetContentsMimeType() {
+const std::string& ContentTranslateDriverImpl::GetContentsMimeType() {
   return navigation_controller_->GetWebContents()->GetContentsMimeType();
 }
 
-const GURL& ContentTranslateDriver::GetLastCommittedURL() {
+const GURL& ContentTranslateDriverImpl::GetLastCommittedURL() {
   return navigation_controller_->GetWebContents()->GetLastCommittedURL();
 }
 
-const GURL& ContentTranslateDriver::GetVisibleURL() {
+const GURL& ContentTranslateDriverImpl::GetVisibleURL() {
   return navigation_controller_->GetWebContents()->GetVisibleURL();
 }
 
-bool ContentTranslateDriver::HasCurrentPage() {
+bool ContentTranslateDriverImpl::HasCurrentPage() {
   return (navigation_controller_->GetLastCommittedEntry() != NULL);
 }
 
-void ContentTranslateDriver::OpenUrlInNewTab(const GURL& url) {
-  content::OpenURLParams params(url,
-                                content::Referrer(),
-                                NEW_FOREGROUND_TAB,
-                                ui::PAGE_TRANSITION_LINK,
-                                false);
+void ContentTranslateDriverImpl::OpenUrlInNewTab(const GURL& url) {
+  content::OpenURLParams params(url, content::Referrer(), NEW_FOREGROUND_TAB,
+                                ui::PAGE_TRANSITION_LINK, false);
   navigation_controller_->GetWebContents()->OpenURL(params);
 }
 
 // content::WebContentsObserver methods
 
-void ContentTranslateDriver::NavigationEntryCommitted(
+void ContentTranslateDriverImpl::NavigationEntryCommitted(
     const content::LoadCommittedDetails& load_details) {
   // Check whether this is a reload: When doing a page reload, the
   // TranslateLanguageDetermined IPC is not sent so the translation needs to be
@@ -191,13 +200,13 @@ void ContentTranslateDriver::NavigationEntryCommitted(
   // an infobar, it must be done after that.
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
-      base::Bind(&ContentTranslateDriver::InitiateTranslation,
+      base::Bind(&ContentTranslateDriverImpl::InitiateTranslation,
                  weak_pointer_factory_.GetWeakPtr(),
                  translate_manager_->GetLanguageState().original_language(),
                  0));
 }
 
-void ContentTranslateDriver::DidNavigateAnyFrame(
+void ContentTranslateDriverImpl::DidNavigateAnyFrame(
     content::RenderFrameHost* render_frame_host,
     const content::LoadCommittedDetails& details,
     const content::FrameNavigateParams& params) {
@@ -210,29 +219,36 @@ void ContentTranslateDriver::DidNavigateAnyFrame(
       details.is_in_page, details.is_main_frame, reload);
 }
 
-bool ContentTranslateDriver::OnMessageReceived(
-    const IPC::Message& message,
-    content::RenderFrameHost* render_frame_host) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(ContentTranslateDriver, message)
-    IPC_MESSAGE_HANDLER(ChromeFrameHostMsg_TranslateAssignedSequenceNumber,
-                        OnTranslateAssignedSequenceNumber)
-    IPC_MESSAGE_HANDLER(ChromeFrameHostMsg_TranslateLanguageDetermined,
-                        OnLanguageDetermined)
-    IPC_MESSAGE_HANDLER(ChromeFrameHostMsg_PageTranslated, OnPageTranslated)
-  IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
+void ContentTranslateDriverImpl::OnPageTranslated(

[groby-ooo-7-16, 2016/07/11 22:34:43]

Is there a reason this moved up here? (Keeping it in the same position at the
end of the file would make the diff slightly clearer)

[leonhsl(Using Gerrit), 2016/07/12 02:48:17]

Ah, I don't remember why it's moved here. I'll move it back.

[leonhsl(Using Gerrit), 2016/07/13 09:56:18]

Done.

+    bool cancelled,
+    mojo::String original_lang,
+    mojo::String translated_lang,
+    TranslateErrors::Type error_type) {
+  if (cancelled)
+    return;
+
+  translate_manager_->PageTranslated(original_lang, translated_lang,
+                                     error_type);
+  FOR_EACH_OBSERVER(
+      Observer, observer_list_,
+      OnPageTranslated(original_lang, translated_lang, error_type));
 }
 
-void ContentTranslateDriver::OnTranslateAssignedSequenceNumber(
-    int page_seq_no) {
-  translate_manager_->set_current_seq_no(page_seq_no);
+void ContentTranslateDriverImpl::OnPageAway(int page_seq_no) {

[groby-ooo-7-16, 2016/07/11 22:34:43]

Do we actually still need the concept of page sequence numbers? IIUC, we used
them as pseudo-cross-process handles for translate requests. Now that we have
mojom::PagePtr, can't we just hand that to the TranslateManager?

[leonhsl(Using Gerrit), 2016/07/12 02:48:17]

I think TranslateManager also serves IOSTranslateDriver, which has no mojo
concept and still has some complicated logic based on the page sequence numbers.
So I did not touch inside TranslateManager to eliminate the page sequence
numbers.

+  mojo_pages_.erase(page_seq_no);
 }
 
-void ContentTranslateDriver::OnLanguageDetermined(
+// mojom::ContentTranslateDriver implementation.
+void ContentTranslateDriverImpl::NewPage(

[groby-ooo-7-16, 2016/07/11 22:34:43]

I take it all functions modifying/reading |mojo_pages_| are on the same thread?

[leonhsl(Using Gerrit), 2016/07/12 02:48:17]

Yeah, mojo is thread-hostile, all the mojo calls and connection error handlers
run on the same thread on which the mojo interface has been registered.
ContentTranslateDriver case is the UI/main thread.

+    mojom::PagePtr page,
     const LanguageDetectionDetails& details,
     bool page_needs_translation) {
+  mojo_pages_[++next_page_seq_no_] = std::move(page);
+  mojo_pages_[next_page_seq_no_].set_connection_error_handler(

[groby-ooo-7-16, 2016/07/11 22:34:43]

I'm new to this mojo thing, so possibly odd question: This means, in a nutshell,
that |mojo_pages_| holds persistent connections to  mojom::Page objects, and we
clear it out when the connection goes away?

[leonhsl(Using Gerrit), 2016/07/12 02:48:17]

Yeah it's right. TranslateHelper will close the connection once page navigated
away, then our connection error handler OnPageAway() here will clear it from
|mojo_pages_|.

+      base::Bind(&ContentTranslateDriverImpl::OnPageAway,
+                 base::Unretained(this), next_page_seq_no_));

[groby-ooo-7-16, 2016/07/11 22:34:43]

Are we guaranteed that |this| will be alive until all error handlers have been
invoked/abandoned?

[leonhsl(Using Gerrit), 2016/07/12 02:48:17]

Yeah this is guaranteed. Because destroying |this| will destroy |mojo_pages_|,
which could not invoke error handler after it's destroyed.

+  translate_manager_->set_current_seq_no(next_page_seq_no_);
+
   translate_manager_->GetLanguageState().LanguageDetermined(
       details.adopted_language, page_needs_translation);
 
@@ -242,16 +258,4 @@ void ContentTranslateDriver::OnLanguageDetermined(
   FOR_EACH_OBSERVER(Observer, observer_list_, OnLanguageDetermined(details));
 }
 
-void ContentTranslateDriver::OnPageTranslated(
-    const std::string& original_lang,
-    const std::string& translated_lang,
-    TranslateErrors::Type error_type) {
-  translate_manager_->PageTranslated(
-      original_lang, translated_lang, error_type);
-  FOR_EACH_OBSERVER(
-      Observer,
-      observer_list_,
-      OnPageTranslated(original_lang, translated_lang, error_type));
-}
-
 }  // namespace translate