TranslateManager is no longer a singleton

chrome/browser/translate/translate_manager.cc

Index: chrome/browser/translate/translate_manager.cc
diff --git a/chrome/browser/translate/translate_manager.cc b/chrome/browser/translate/translate_manager.cc
index 5a83624db83926e93fe3f4e1de7c8bce0bcb753b..7c600cad84ac06f585f9ce4eb9bd240d9aca9c7a 100644
--- a/chrome/browser/translate/translate_manager.cc
+++ b/chrome/browser/translate/translate_manager.cc
@@ -6,7 +6,6 @@
 
 #include "base/bind.h"
 #include "base/command_line.h"
-#include "base/memory/singleton.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
@@ -90,13 +89,7 @@ void NotifyTranslateError(const TranslateErrorDetails& details) {
 
 }  // namespace
 
-TranslateManager::~TranslateManager() {
-}
-
-// static
-TranslateManager* TranslateManager::GetInstance() {
-  return Singleton<TranslateManager>::get();
-}
+TranslateManager::~TranslateManager() {}
 
 // static
 bool TranslateManager::IsTranslatableURL(const GURL& url) {
@@ -124,6 +117,8 @@ void TranslateManager::Observe(int type,
     case content::NOTIFICATION_NAV_ENTRY_COMMITTED: {
       NavigationController* controller =
           content::Source<NavigationController>(source).ptr();
+      DCHECK_EQ(&translate_tab_helper_->GetWebContents()->GetController(),
+                controller);
       content::LoadCommittedDetails* load_details =
           content::Details<content::LoadCommittedDetails>(details).ptr();
       NavigationEntry* entry = controller->GetActiveEntry();
@@ -132,11 +127,6 @@ void TranslateManager::Observe(int type,
         return;
       }
 
-      TranslateTabHelper* translate_tab_helper =
-          TranslateTabHelper::FromWebContents(controller->GetWebContents());
-      if (!translate_tab_helper)
-        return;
-
       // If the navigation happened while offline don't show the translate
       // bar since there will be nothing to translate.
       if (load_details->http_status_code == 0 ||
@@ -145,7 +135,7 @@ void TranslateManager::Observe(int type,
       }
 
       if (!load_details->is_main_frame &&
-          translate_tab_helper->GetLanguageState().translation_declined()) {
+          translate_tab_helper_->GetLanguageState().translation_declined()) {
         // Some sites (such as Google map) may trigger sub-frame navigations
         // when the user interacts with the page.  We don't want to show a new
         // infobar if the user already dismissed one in that case.
@@ -159,19 +149,19 @@ void TranslateManager::Observe(int type,
       // When doing a page reload, TAB_LANGUAGE_DETERMINED is not sent,
       // so the translation needs to be explicitly initiated, but only when the
       // page needs translation.
-      if (!translate_tab_helper->GetLanguageState().page_needs_translation())
+      if (!translate_tab_helper_->GetLanguageState().page_needs_translation())
         return;
       // Note that we delay it as the TranslateManager gets this notification
       // before the WebContents and the WebContents processing might remove the
       // current infobars.  Since InitTranslation might add an infobar, it must
       // be done after that.
-      base::MessageLoop::current()->PostTask(FROM_HERE,
+      base::MessageLoop::current()->PostTask(
+          FROM_HERE,
           base::Bind(
               &TranslateManager::InitiateTranslationPosted,
               weak_method_factory_.GetWeakPtr(),
-              controller->GetWebContents()->GetRenderProcessHost()->GetID(),
-              controller->GetWebContents()->GetRenderViewHost()->GetRoutingID(),
-              translate_tab_helper->GetLanguageState().original_language(), 0));
+              translate_tab_helper_->GetLanguageState().original_language(),
+              0));
       break;
     }
     case chrome::NOTIFICATION_TAB_LANGUAGE_DETERMINED: {
@@ -179,21 +169,17 @@ void TranslateManager::Observe(int type,
           content::Details<const LanguageDetectionDetails>(details).ptr();
 
       WebContents* tab = content::Source<WebContents>(source).ptr();
+      DCHECK_EQ(translate_tab_helper_->GetWebContents(), tab);
 
       // We may get this notifications multiple times.  Make sure to translate
       // only once.
-      TranslateTabHelper* translate_tab_helper =
-          TranslateTabHelper::FromWebContents(tab);
-      if (!translate_tab_helper)
-        return;
-
-      LanguageState& language_state = translate_tab_helper->GetLanguageState();
+      LanguageState& language_state = translate_tab_helper_->GetLanguageState();
       if (language_state.page_needs_translation() &&
           !language_state.translation_pending() &&
           !language_state.translation_declined() &&
           !language_state.IsPageTranslated()) {
         std::string language = lang_det_details->adopted_language;
-        InitiateTranslation(tab, language);
+        InitiateTranslation(language);
       }
       break;
     }
@@ -201,10 +187,9 @@ void TranslateManager::Observe(int type,
       // Only add translate infobar if it doesn't exist; if it already exists,
       // just update the state, the actual infobar would have received the same
       //  notification and update the visual display accordingly.
-      WebContents* tab = content::Source<WebContents>(source).ptr();
       PageTranslatedDetails* page_translated_details =
           content::Details<PageTranslatedDetails>(details).ptr();
-      PageTranslated(tab, page_translated_details);
+      PageTranslated(page_translated_details);
       break;
     }
     default:
@@ -221,25 +206,26 @@ TranslateManager::RegisterTranslateErrorCallback(
   return g_callback_list_->Add(callback);
 }
 
-TranslateManager::TranslateManager()
-  : max_reload_check_attempts_(kMaxTranslateLoadCheckAttempts),
-    weak_method_factory_(this) {
-  notification_registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
-                              content::NotificationService::AllSources());
+TranslateManager::TranslateManager(TranslateTabHelper* helper)
+    : max_reload_check_attempts_(kMaxTranslateLoadCheckAttempts),
+      translate_tab_helper_(helper),
+      weak_method_factory_(this) {
+
+  WebContents* web_contents = translate_tab_helper_->GetWebContents();
+
+  notification_registrar_.Add(
+      this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+      content::Source<NavigationController>(&web_contents->GetController()));
   notification_registrar_.Add(this,
                               chrome::NOTIFICATION_TAB_LANGUAGE_DETERMINED,
-                              content::NotificationService::AllSources());
-  notification_registrar_.Add(this, chrome::NOTIFICATION_PAGE_TRANSLATED,
-                              content::NotificationService::AllSources());
+                              content::Source<WebContents>(web_contents));
+  notification_registrar_.Add(this,
+                              chrome::NOTIFICATION_PAGE_TRANSLATED,
+                              content::Source<WebContents>(web_contents));
 }
 
-void TranslateManager::InitiateTranslation(WebContents* web_contents,
-                                           const std::string& page_lang) {
-  TranslateTabHelper* translate_tab_helper =
-      TranslateTabHelper::FromWebContents(web_contents);
-  if (!translate_tab_helper)
-    return;
-
+void TranslateManager::InitiateTranslation(const std::string& page_lang) {
+  WebContents* web_contents = translate_tab_helper_->GetWebContents();
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
   Profile* original_profile = profile->GetOriginalProfile();
@@ -329,18 +315,18 @@ void TranslateManager::InitiateTranslation(WebContents* web_contents,
     if (!auto_target_lang.empty()) {
       TranslateBrowserMetrics::ReportInitiationStatus(
           TranslateBrowserMetrics::INITIATION_STATUS_AUTO_BY_CONFIG);
-      TranslatePage(web_contents, language_code, auto_target_lang);
+      TranslatePage(language_code, auto_target_lang);
       return;
     }
   }
 
-  LanguageState& language_state = translate_tab_helper->GetLanguageState();
+  LanguageState& language_state = translate_tab_helper_->GetLanguageState();
   std::string auto_translate_to = language_state.AutoTranslateTo();
   if (!auto_translate_to.empty()) {
     // This page was navigated through a click from a translated page.
     TranslateBrowserMetrics::ReportInitiationStatus(
         TranslateBrowserMetrics::INITIATION_STATUS_AUTO_BY_LINK);
-    TranslatePage(web_contents, language_code, auto_translate_to);
+    TranslatePage(language_code, auto_translate_to);
     return;
   }
 
@@ -348,49 +334,35 @@ void TranslateManager::InitiateTranslation(WebContents* web_contents,
       TranslateBrowserMetrics::INITIATION_STATUS_SHOW_INFOBAR);
 
   // Prompts the user if he/she wants the page translated.
-  translate_tab_helper->ShowTranslateUI(TranslateTabHelper::BEFORE_TRANSLATE,
-                                        web_contents,
-                                        language_code,
-                                        target_lang,
-                                        TranslateErrors::NONE);
+  translate_tab_helper_->ShowTranslateUI(TranslateTabHelper::BEFORE_TRANSLATE,
+                                         language_code,
+                                         target_lang,
+                                         TranslateErrors::NONE);
 }
 
-void TranslateManager::InitiateTranslationPosted(int process_id,
-                                                 int render_id,
-                                                 const std::string& page_lang,
+void TranslateManager::InitiateTranslationPosted(const std::string& page_lang,
                                                  int attempt) {
-  // The tab might have been closed.
-  WebContents* web_contents =
-      tab_util::GetWebContentsByID(process_id, render_id);
-  if (!web_contents)
-    return;
-
-  TranslateTabHelper* translate_tab_helper =
-      TranslateTabHelper::FromWebContents(web_contents);
-  if (translate_tab_helper->GetLanguageState().translation_pending())
-    return;
-
   // During a reload we need web content to be available before the
   // translate script is executed. Otherwise we will run the translate script on
   // an empty DOM which will fail. Therefore we wait a bit to see if the page
   // has finished.
+  WebContents* web_contents = translate_tab_helper_->GetWebContents();
   if ((web_contents->IsLoading()) && attempt < kMaxTranslateLoadCheckAttempts) {
     int backoff = attempt * max_reload_check_attempts_;
     base::MessageLoop::current()->PostDelayedTask(
         FROM_HERE, base::Bind(&TranslateManager::InitiateTranslationPosted,
-                              weak_method_factory_.GetWeakPtr(), process_id,
-                              render_id, page_lang, ++attempt),
+                              weak_method_factory_.GetWeakPtr(),
+                              page_lang, ++attempt),
         base::TimeDelta::FromMilliseconds(backoff));
     return;
   }
 
-  InitiateTranslation(web_contents,
-                      TranslateDownloadManager::GetLanguageCode(page_lang));
+  InitiateTranslation(TranslateDownloadManager::GetLanguageCode(page_lang));
 }
 
-void TranslateManager::TranslatePage(WebContents* web_contents,
-                                     const std::string& original_source_lang,
+void TranslateManager::TranslatePage(const std::string& original_source_lang,
                                      const std::string& target_lang) {
+  WebContents* web_contents = translate_tab_helper_->GetWebContents();
   NavigationEntry* entry = web_contents->GetController().GetActiveEntry();
   if (!entry) {
     NOTREACHED();
@@ -405,39 +377,34 @@ void TranslateManager::TranslatePage(WebContents* web_contents,
   if (!TranslateDownloadManager::IsSupportedLanguage(source_lang))
     source_lang = std::string(translate::kUnknownLanguageCode);
 
-  TranslateTabHelper* translate_tab_helper =
-      TranslateTabHelper::FromWebContents(web_contents);
-  DCHECK(translate_tab_helper);
-  translate_tab_helper->ShowTranslateUI(TranslateTabHelper::TRANSLATING,
-                                        web_contents,
-                                        source_lang,
-                                        target_lang,
-                                        TranslateErrors::NONE);
+  translate_tab_helper_->ShowTranslateUI(TranslateTabHelper::TRANSLATING,
+                                         source_lang,
+                                         target_lang,
+                                         TranslateErrors::NONE);
 
   TranslateScript* script = TranslateDownloadManager::GetInstance()->script();
   DCHECK(script != NULL);
 
   const std::string& script_data = script->data();
   if (!script_data.empty()) {
-    DoTranslatePage(web_contents, script_data, source_lang, target_lang);
+    DoTranslatePage(script_data, source_lang, target_lang);
     return;
   }
 
   // The script is not available yet.  Queue that request and query for the
   // script.  Once it is downloaded we'll do the translate.
-  content::RenderViewHost* rvh = web_contents->GetRenderViewHost();
-  PendingRequest request;
-  request.render_process_id = rvh->GetProcess()->GetID();
-  request.render_view_id = rvh->GetRoutingID();
-  request.page_id = entry->GetPageID();
-  request.source_lang = source_lang;
-  request.target_lang = target_lang;
-
-  script->Request(base::Bind(&TranslateManager::OnTranslateScriptFetchComplete,
-                             weak_method_factory_.GetWeakPtr(), request));
+  TranslateScript::RequestCallback callback =
+      base::Bind(&TranslateManager::OnTranslateScriptFetchComplete,
+                 weak_method_factory_.GetWeakPtr(),
+                 entry->GetPageID(),
+                 source_lang,
+                 target_lang);
+
+  script->Request(callback);
 }
 
-void TranslateManager::RevertTranslation(WebContents* web_contents) {
+void TranslateManager::RevertTranslation() {
+  WebContents* web_contents = translate_tab_helper_->GetWebContents();
   NavigationEntry* entry = web_contents->GetController().GetActiveEntry();
   if (!entry) {
     NOTREACHED();
@@ -446,15 +413,14 @@ void TranslateManager::RevertTranslation(WebContents* web_contents) {
   web_contents->GetRenderViewHost()->Send(new ChromeViewMsg_RevertTranslation(
       web_contents->GetRenderViewHost()->GetRoutingID(), entry->GetPageID()));
 
-  TranslateTabHelper* translate_tab_helper =
-      TranslateTabHelper::FromWebContents(web_contents);
-  translate_tab_helper->GetLanguageState().SetCurrentLanguage(
-      translate_tab_helper->GetLanguageState().original_language());
+  translate_tab_helper_->GetLanguageState().SetCurrentLanguage(
+      translate_tab_helper_->GetLanguageState().original_language());
 }
 
-void TranslateManager::ReportLanguageDetectionError(WebContents* web_contents) {
+void TranslateManager::ReportLanguageDetectionError() {
   TranslateBrowserMetrics::ReportLanguageDetectionError();
   // We'll open the URL in a new tab so that the user can tell us more.
+  WebContents* web_contents = translate_tab_helper_->GetWebContents();
   Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
   if (!browser) {
     NOTREACHED();
@@ -469,12 +435,10 @@ void TranslateManager::ReportLanguageDetectionError(WebContents* web_contents) {
       kUrlQueryName,
       page_url.spec());
 
-  TranslateTabHelper* translate_tab_helper =
-      TranslateTabHelper::FromWebContents(web_contents);
   report_error_url = net::AppendQueryParameter(
       report_error_url,
       kSourceLanguageQueryName,
-      translate_tab_helper->GetLanguageState().original_language());
+      translate_tab_helper_->GetLanguageState().original_language());
 
   report_error_url = TranslateURLUtil::AddHostLocaleToUrl(report_error_url);
   report_error_url = TranslateURLUtil::AddApiKeyToUrl(report_error_url);
@@ -483,29 +447,23 @@ void TranslateManager::ReportLanguageDetectionError(WebContents* web_contents) {
                                 content::PAGE_TRANSITION_AUTO_BOOKMARK);
 }
 
-void TranslateManager::DoTranslatePage(WebContents* web_contents,
-                                       const std::string& translate_script,
+void TranslateManager::DoTranslatePage(const std::string& translate_script,
                                        const std::string& source_lang,
                                        const std::string& target_lang) {
+  WebContents* web_contents = translate_tab_helper_->GetWebContents();
   NavigationEntry* entry = web_contents->GetController().GetActiveEntry();
   if (!entry) {
     NOTREACHED();
     return;
   }
 
-  TranslateTabHelper* translate_tab_helper =
-      TranslateTabHelper::FromWebContents(web_contents);
-  if (!translate_tab_helper)

[blundell, 2014/02/17 15:26:28]

I think you should keep this check, checking against web_contents at the top of
the method.

[droger, 2014/02/17 16:52:48]

Done.

-    return;
-
-  translate_tab_helper->GetLanguageState().set_translation_pending(true);
+  translate_tab_helper_->GetLanguageState().set_translation_pending(true);
   web_contents->GetRenderViewHost()->Send(new ChromeViewMsg_TranslatePage(
       web_contents->GetRenderViewHost()->GetRoutingID(), entry->GetPageID(),
       translate_script, source_lang, target_lang));
 }
 
-void TranslateManager::PageTranslated(WebContents* web_contents,
-                                      PageTranslatedDetails* details) {
+void TranslateManager::PageTranslated(PageTranslatedDetails* details) {
   if ((details->error_type == TranslateErrors::NONE) &&
       details->source_language != translate::kUnknownLanguageCode &&
       !TranslateDownloadManager::IsSupportedLanguage(
@@ -513,15 +471,12 @@ void TranslateManager::PageTranslated(WebContents* web_contents,
     details->error_type = TranslateErrors::UNSUPPORTED_LANGUAGE;
   }
 
-  TranslateTabHelper* translate_tab_helper =
-      TranslateTabHelper::FromWebContents(web_contents);
-  DCHECK(translate_tab_helper);

[blundell, 2014/02/17 15:26:28]

You can change this to DCHECK(translate_tab_helper_->GetWebContents())

[droger, 2014/02/17 16:52:48]

Done.

-  translate_tab_helper->ShowTranslateUI(TranslateTabHelper::AFTER_TRANSLATE,
-                                        web_contents,
-                                        details->source_language,
-                                        details->target_language,
-                                        details->error_type);
+  translate_tab_helper_->ShowTranslateUI(TranslateTabHelper::AFTER_TRANSLATE,
+                                         details->source_language,
+                                         details->target_language,
+                                         details->error_type);
 
+  WebContents* web_contents = translate_tab_helper_->GetWebContents();
   if (details->error_type != TranslateErrors::NONE &&
       !web_contents->GetBrowserContext()->IsOffTheRecord()) {
     TranslateErrorDetails error_details;
@@ -532,17 +487,15 @@ void TranslateManager::PageTranslated(WebContents* web_contents,
   }
 }
 
-void TranslateManager::OnTranslateScriptFetchComplete(PendingRequest request,
-                                                      bool success,
-                                                      const std::string& data) {
-  WebContents* web_contents = tab_util::GetWebContentsByID(
-      request.render_process_id, request.render_view_id);
-  if (!web_contents) {
-    // The tab went away while we were retrieving the script.
-    return;
-  }
+void TranslateManager::OnTranslateScriptFetchComplete(
+    int page_id,
+    const std::string& source_lang,
+    const std::string& target_lang,
+    bool success,
+    const std::string& data) {
+  WebContents* web_contents = translate_tab_helper_->GetWebContents();

[blundell, 2014/02/17 15:26:28]

This can return NULL, so you need to keep the check that was there before.

[droger, 2014/02/17 16:52:48]

I changed TranslateTabHelper so that in theory this can never be NULL
(translate_manager_ is deleted when OnWebContentsDestroyed is called).
I nevertheless added the check as you suggested, and we'll maybe remove it
later.

   NavigationEntry* entry = web_contents->GetController().GetActiveEntry();
-  if (!entry || entry->GetPageID() != request.page_id) {
+  if (!entry || entry->GetPageID() != page_id) {
     // We navigated away from the page the translation was triggered on.
     return;
   }
@@ -552,17 +505,12 @@ void TranslateManager::OnTranslateScriptFetchComplete(PendingRequest request,
     TranslateScript* translate_script =
         TranslateDownloadManager::GetInstance()->script();
     DCHECK(translate_script);
-    DoTranslatePage(
-        web_contents, data, request.source_lang, request.target_lang);
+    DoTranslatePage(data, source_lang, target_lang);
   } else {
-    TranslateTabHelper* translate_tab_helper =
-        TranslateTabHelper::FromWebContents(web_contents);
-    DCHECK(translate_tab_helper);
-    translate_tab_helper->ShowTranslateUI(TranslateTabHelper::TRANSLATE_ERROR,
-                                          web_contents,
-                                          request.source_lang,
-                                          request.target_lang,
-                                          TranslateErrors::NETWORK);
+    translate_tab_helper_->ShowTranslateUI(TranslateTabHelper::TRANSLATE_ERROR,
+                                           source_lang,
+                                           target_lang,
+                                           TranslateErrors::NETWORK);
     if (!web_contents->GetBrowserContext()->IsOffTheRecord()) {
       TranslateErrorDetails error_details;
       error_details.time = base::Time::Now();