Move the translate script callbacks from TranslateManager to TranslateScript

This CL refactors the way callbacks for TranslateScript are handled.
Previously, only one callback was allowed, and thus TranslateManager was
managing a global list of pending script request to be able to merge all these
requests into one callback.

As we want to move away from TranslateManager being a singleton, the management
of TranslateScript requests is moved to TranslateScript (which is also cleaner).
TranslateScript now supports multiple callbacks.

BUG=332736

Review URL: https://codereview.chromium.org/166553003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@251666 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 59 insertions, 67 deletions

diff --git a/chrome/browser/translate/translate_manager.cc b/chrome/browser/translate/translate_manager.cc
index f010dc9..cdb2727 100644
--- a/chrome/browser/translate/translate_manager.cc
+++ b/chrome/browser/translate/translate_manager.cc
@@ -432,13 +432,9 @@ void TranslateManager::TranslatePage(WebContents* web_contents,
   request.page_id = entry->GetPageID();
   request.source_lang = source_lang;
   request.target_lang = target_lang;
-  pending_requests_.push_back(request);
-
-  if (script->HasPendingRequest())
-    return;
 
   script->Request(base::Bind(&TranslateManager::OnTranslateScriptFetchComplete,
-                             base::Unretained(this)));
+                             weak_method_factory_.GetWeakPtr(), request));
 }
 
 void TranslateManager::RevertTranslation(WebContents* web_contents) {
@@ -536,52 +532,45 @@ void TranslateManager::PageTranslated(WebContents* web_contents,
   }
 }
 
-void TranslateManager::OnTranslateScriptFetchComplete(
-    bool success, const std::string& data) {
-  std::vector<PendingRequest>::const_iterator iter;
-  for (iter = pending_requests_.begin(); iter != pending_requests_.end();
-       ++iter) {
-    const PendingRequest& request = *iter;
-    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.
-      continue;
-    }
-    NavigationEntry* entry = web_contents->GetController().GetActiveEntry();
-    if (!entry || entry->GetPageID() != request.page_id) {
-      // We navigated away from the page the translation was triggered on.
-      continue;
-    }
+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;
+  }
+  NavigationEntry* entry = web_contents->GetController().GetActiveEntry();
+  if (!entry || entry->GetPageID() != request.page_id) {
+    // We navigated away from the page the translation was triggered on.
+    return;
+  }
 
-    if (success) {
-      // Translate the page.
-      TranslateScript* translate_script =
-          TranslateDownloadManager::GetInstance()->script();
-      DCHECK(translate_script);
-      DoTranslatePage(web_contents, translate_script->data(),
-                      request.source_lang, request.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);
-
-      if (!web_contents->GetBrowserContext()->IsOffTheRecord()) {
-        TranslateErrorDetails error_details;
-        error_details.time = base::Time::Now();
-        error_details.url = entry->GetURL();
-        error_details.error = TranslateErrors::NETWORK;
-        NotifyTranslateError(error_details);
-      }
+  if (success) {
+    // Translate the page.
+    TranslateScript* translate_script =
+        TranslateDownloadManager::GetInstance()->script();
+    DCHECK(translate_script);
+    DoTranslatePage(web_contents, translate_script->data(), request.source_lang,
+                    request.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);
+    if (!web_contents->GetBrowserContext()->IsOffTheRecord()) {
+      TranslateErrorDetails error_details;
+      error_details.time = base::Time::Now();
+      error_details.url = entry->GetURL();
+      error_details.error = TranslateErrors::NETWORK;
+      NotifyTranslateError(error_details);
     }
   }
-  pending_requests_.clear();
 }
 
 // static
diff --git a/chrome/browser/translate/translate_manager.h b/chrome/browser/translate/translate_manager.h
index aa79374..d1b29a6 100644
--- a/chrome/browser/translate/translate_manager.h
+++ b/chrome/browser/translate/translate_manager.h
@@ -136,7 +136,9 @@ class TranslateManager : public content::NotificationObserver {
   void PageTranslated(content::WebContents* web_contents,
                       PageTranslatedDetails* details);
 
-  void OnTranslateScriptFetchComplete(bool success, const std::string& data);
+  void OnTranslateScriptFetchComplete(PendingRequest request,
+                                      bool success,
+                                      const std::string& data);
 
   // Notifies to the observers when a language is detected.
   void NotifyLanguageDetection(const LanguageDetectionDetails& details);
@@ -149,11 +151,6 @@ class TranslateManager : public content::NotificationObserver {
   // Max number of attempts before checking if a page has been reloaded.
   int max_reload_check_attempts_;
 
-  // The list of pending translate requests.  Translate requests are queued when
-  // the translate script is not ready and has to be fetched from the translate
-  // server.
-  std::vector<PendingRequest> pending_requests_;
-
   // List of registered observers.
   ObserverList<Observer> observer_list_;
 
diff --git a/components/translate/core/browser/translate_script.cc b/components/translate/core/browser/translate_script.cc
index a83f499..68b5cff 100644
--- a/components/translate/core/browser/translate_script.cc
+++ b/components/translate/core/browser/translate_script.cc
@@ -51,14 +51,16 @@ TranslateScript::TranslateScript()
 TranslateScript::~TranslateScript() {
 }
 
-void TranslateScript::Request(const Callback& callback) {
+void TranslateScript::Request(const RequestCallback& callback) {
+  DCHECK(data_.empty()) << "Do not fetch the script if it is already fetched";
+  callback_list_.push_back(callback);
+
   if (fetcher_.get() != NULL) {
-    NOTREACHED();
+    // If there is already a request in progress, do nothing. |callback| will be
+    // run on completion.
     return;
   }
 
-  callback_ = callback;
-
   GURL translate_script_url;
   // Check if command-line contains an alternative URL for translate service.
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
@@ -145,5 +147,10 @@ void TranslateScript::OnScriptFetchComplete(
         expiration_delay_);
   }
 
-  callback_.Run(success, data);
+  for (RequestCallbackList::iterator it = callback_list_.begin();
+       it != callback_list_.end();
+       ++it) {
+    it->Run(success, data);
+  }
+  callback_list_.clear();
 }
diff --git a/components/translate/core/browser/translate_script.h b/components/translate/core/browser/translate_script.h
index 552daee..156d3a5 100644
--- a/components/translate/core/browser/translate_script.h
+++ b/components/translate/core/browser/translate_script.h
@@ -6,8 +6,9 @@
 #define COMPONENTS_TRANSLATE_CORE_BROWSER_TRANSLATE_SCRIPT_H_
 
 #include <string>
+#include <vector>
 
-#include "base/callback.h"
+#include "base/callback_forward.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -18,7 +19,7 @@ class TranslateURLFetcher;
 
 class TranslateScript {
  public:
-  typedef base::Callback<void(bool, const std::string&)> Callback;
+  typedef base::Callback<void(bool, const std::string&)> RequestCallback;
 
   static const int kFetcherId = 0;
 
@@ -40,10 +41,7 @@ class TranslateScript {
 
   // Fetches the JS translate script (the script that is injected in the page
   // to translate it).
-  void Request(const Callback& callback);
-
-  // Returns true if this has a pending request.
-  bool HasPendingRequest() const { return fetcher_.get() != NULL; }
+  void Request(const RequestCallback& callback);
 
  private:
   friend class TranslateScriptTest;
@@ -83,8 +81,9 @@ class TranslateScript {
   // server.
   base::TimeDelta expiration_delay_;
 
-  // The callback called when the server sends a response.
-  Callback callback_;
+  // The callbacks called when the server sends a response.
+  typedef std::vector<RequestCallback> RequestCallbackList;
+  RequestCallbackList callback_list_;
 
   base::WeakPtrFactory<TranslateScript> weak_method_factory_;