Refactoring: Create TranslateScript from TranslateManager

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 8a2d93a5d28662164ba4f995a2436ea8c69a88c7..267b4043872594d1be8d087ea57a32e46d8e22a3 100644
--- a/chrome/browser/translate/translate_manager.cc
+++ b/chrome/browser/translate/translate_manager.cc
@@ -8,7 +8,6 @@
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/memory/singleton.h"
-#include "base/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_split.h"
@@ -29,6 +28,7 @@
 #include "chrome/browser/translate/translate_infobar_delegate.h"
 #include "chrome/browser/translate/translate_language_list.h"
 #include "chrome/browser/translate/translate_prefs.h"
+#include "chrome/browser/translate/translate_script.h"
 #include "chrome/browser/translate/translate_tab_helper.h"
 #include "chrome/browser/translate/translate_url_util.h"
 #include "chrome/browser/ui/browser.h"
@@ -52,16 +52,10 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
-#include "google_apis/google_api_keys.h"
-#include "grit/browser_resources.h"
-#include "net/base/escape.h"
 #include "net/base/load_flags.h"
 #include "net/base/url_util.h"
 #include "net/http/http_status_code.h"
-#include "net/url_request/url_fetcher.h"
-#include "net/url_request/url_request_status.h"
 #include "ui/base/l10n/l10n_util.h"
-#include "ui/base/resource/resource_bundle.h"
 
 #ifdef FILE_MANAGER_EXTENSION
 #include "chrome/browser/chromeos/extensions/file_manager/file_manager_util.h"
@@ -74,17 +68,9 @@ using content::WebContents;
 
 namespace {
 
-const char kTranslateScriptURL[] =
-    "https://translate.google.com/translate_a/element.js";
-const char kTranslateScriptHeader[] = "Google-Translate-Element-Mode: library";
 const char kReportLanguageDetectionErrorURL[] =
     "https://translate.google.com/translate_error?client=cr&action=langidc";
 
-// Used in kTranslateScriptURL to specify a callback function name.
-const char kCallbackQueryName[] = "cb";
-const char kCallbackQueryValue[] =
-    "cr.googleTranslate.onTranslateElementLoad";
-
 // Used in kReportLanguageDetectionErrorURL to specify the original page
 // language.
 const char kSourceLanguageQueryName[] = "sl";
@@ -100,8 +86,6 @@ const int kTranslateLoadCheckDelayMs = 150;
 // loading before giving up the translation
 const int kMaxTranslateLoadCheckAttempts = 20;
 
-const int kTranslateScriptExpirationDelayDays = 1;
-
 }  // namespace
 
 TranslateManager::~TranslateManager() {
@@ -187,6 +171,14 @@ bool TranslateManager::IsAcceptLanguage(Profile* profile,
   return false;
 }
 
+void TranslateManager::SetTranslateScriptExpirationDelay(int delay_ms) {
+  if (!script_) {
+    NOTREACHED();
+    return;
+  }
+  script_->set_expiration_delay(delay_ms);
+}
+
 void TranslateManager::Observe(int type,
                                const content::NotificationSource& source,
                                const content::NotificationDetails& details) {
@@ -284,87 +276,52 @@ void TranslateManager::Observe(int type,
   }
 }
 
-void TranslateManager::OnURLFetchComplete(const net::URLFetcher* source) {
-  if (translate_script_request_pending_.get() != source) {
-    // Looks like crash on Mac is possibly caused with callback entering here
-    // with unknown fetcher when network is refreshed.
-    scoped_ptr<const net::URLFetcher> delete_ptr(source);
-    return;
-  }
-
-  bool error =
-      source->GetStatus().status() != net::URLRequestStatus::SUCCESS ||
-      source->GetResponseCode() != net::HTTP_OK;
-  if (translate_script_request_pending_.get() == source) {
-    scoped_ptr<const net::URLFetcher> delete_ptr(
-        translate_script_request_pending_.release());
-    if (!error) {
-      base::StringPiece str = ResourceBundle::GetSharedInstance().
-          GetRawDataResource(IDR_TRANSLATE_JS);
-      DCHECK(translate_script_.empty());
-      str.CopyToString(&translate_script_);
-      std::string argument = "('";
-      std::string api_key = google_apis::GetAPIKey();
-      argument += net::EscapeQueryParamValue(api_key, true);
-      argument += "');\n";
-      std::string data;
-      source->GetResponseAsString(&data);
-      translate_script_ += argument + data;
-
-      // We'll expire the cached script after some time, to make sure long
-      // running browsers still get fixes that might get pushed with newer
-      // scripts.
-      base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
-          base::Bind(&TranslateManager::ClearTranslateScript,
-                     weak_method_factory_.GetWeakPtr()),
-          translate_script_expiration_delay_);
+void TranslateManager::ProcessPendingRequests(bool error) {
+  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;
     }
-    // Process any pending requests.
-    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;
-      }
 
-      if (error) {
-        Profile* profile =
-            Profile::FromBrowserContext(web_contents->GetBrowserContext());
-        TranslateInfoBarDelegate::Create(
-            InfoBarService::FromWebContents(web_contents),
-            true,
-            TranslateInfoBarDelegate::TRANSLATION_ERROR,
-            TranslateErrors::NETWORK,
-            profile->GetPrefs(),
-            ShortcutConfig(),
-            request.source_lang,
-            request.target_lang);
-
-        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);
-        }
-      } else {
-        // Translate the page.
-        DoTranslatePage(web_contents, translate_script_,
-                        request.source_lang, request.target_lang);
+    if (error) {
+      Profile* profile =
+          Profile::FromBrowserContext(web_contents->GetBrowserContext());
+      TranslateInfoBarDelegate::Create(
+          InfoBarService::FromWebContents(web_contents),
+          true,
+          TranslateInfoBarDelegate::TRANSLATION_ERROR,
+          TranslateErrors::NETWORK,
+          profile->GetPrefs(),
+          ShortcutConfig(),
+          request.source_lang,
+          request.target_lang);
+
+      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);
       }
+    } else {
+      // Translate the page.
+      const std::string& translate_script = script_->data();
+      DoTranslatePage(web_contents, translate_script,
+                      request.source_lang, request.target_lang);
     }
-    pending_requests_.clear();
   }
+  pending_requests_.clear();
 }
 
 void TranslateManager::AddObserver(Observer* obs) {
@@ -392,8 +349,6 @@ void TranslateManager::NotifyTranslateError(
 
 TranslateManager::TranslateManager()
   : weak_method_factory_(this),
-    translate_script_expiration_delay_(base::TimeDelta::FromDays(
-        kTranslateScriptExpirationDelayDays)),
     max_reload_check_attempts_(kMaxTranslateLoadCheckAttempts) {
   notification_registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
                               content::NotificationService::AllSources());
@@ -404,6 +359,7 @@ TranslateManager::TranslateManager()
                               content::NotificationService::AllSources());
   language_list_.reset(new TranslateLanguageList);
   accept_languages_.reset(new TranslateAcceptLanguages);
+  script_.reset(new TranslateScript);
 }
 
 void TranslateManager::InitiateTranslation(WebContents* web_contents,
@@ -586,8 +542,9 @@ void TranslateManager::TranslatePage(WebContents* web_contents,
       TranslateInfoBarDelegate::TRANSLATING, TranslateErrors::NONE,
       profile->GetPrefs(), ShortcutConfig(), source_lang, target_lang);
 

[Takashi Toyoshima, 2013/06/21 05:38:55]

Should relocate DCHECK at line 562 to here?

[hajimehoshi, 2013/06/21 07:01:44]

Done.

-  if (!translate_script_.empty()) {
-    DoTranslatePage(web_contents, translate_script_, source_lang, target_lang);
+  const std::string& translate_script = script_->data();
+  if (!translate_script.empty()) {
+    DoTranslatePage(web_contents, translate_script, source_lang, target_lang);
     return;
   }
 
@@ -601,7 +558,14 @@ void TranslateManager::TranslatePage(WebContents* web_contents,
   request.source_lang = source_lang;
   request.target_lang = target_lang;
   pending_requests_.push_back(request);
-  RequestTranslateScript();
+
+  DCHECK(script_ != NULL);

[Takashi Toyoshima, 2013/06/21 05:38:55]

script_.get()

[hajimehoshi, 2013/06/21 07:01:44]

Done.

+  if (script_->HasPendingRequest())
+    return;
+
+  script_->Request(
+      base::Bind(&TranslateManager::OnTranslateScriptFetchComplete,
+                 base::Unretained(this)));
 }
 
 void TranslateManager::RevertTranslation(WebContents* web_contents) {
@@ -650,6 +614,14 @@ void TranslateManager::ReportLanguageDetectionError(WebContents* web_contents) {
                                 content::PAGE_TRANSITION_AUTO_BOOKMARK);
 }
 
+void TranslateManager::ClearTranslateScript() {
+  if (!script_) {

[Takashi Toyoshima, 2013/06/21 05:38:55]

script_.get() == NULL

[hajimehoshi, 2013/06/21 07:01:44]

Done.

+    NOTREACHED();
+    return;
+  }
+  script_->Clear();
+}
+
 void TranslateManager::DoTranslatePage(WebContents* web_contents,
                                        const std::string& translate_script,
                                        const std::string& source_lang,
@@ -718,51 +690,12 @@ void TranslateManager::FetchLanguageListFromTranslateServer(
 
 void TranslateManager::CleanupPendingUlrFetcher() {
   language_list_.reset();
-  translate_script_request_pending_.reset();
+  script_.reset();
 }
 
-void TranslateManager::RequestTranslateScript() {
-  if (translate_script_request_pending_.get() != NULL)
-    return;
-
-  GURL translate_script_url;
-  // Check if command-line contains an alternative URL for translate service.
-  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
-  if (command_line.HasSwitch(switches::kTranslateScriptURL)) {
-    translate_script_url = GURL(
-        command_line.GetSwitchValueASCII(switches::kTranslateScriptURL));
-    if (!translate_script_url.is_valid() ||
-        !translate_script_url.query().empty()) {
-      LOG(WARNING) << "The following translate URL specified at the "
-                   << "command-line is invalid: "
-                   << translate_script_url.spec();
-      translate_script_url = GURL();
-    }
-  }
-  // Use default URL when command-line argument is not specified, or specified
-  // URL is invalid.
-  if (translate_script_url.is_empty())
-    translate_script_url = GURL(kTranslateScriptURL);
-
-  translate_script_url = net::AppendQueryParameter(
-      translate_script_url,
-      kCallbackQueryName,
-      kCallbackQueryValue);
-
-  translate_script_url =
-      TranslateURLUtil::AddHostLocaleToUrl(translate_script_url);
-  translate_script_url =
-      TranslateURLUtil::AddApiKeyToUrl(translate_script_url);
-
-  translate_script_request_pending_.reset(net::URLFetcher::Create(
-      0, translate_script_url, net::URLFetcher::GET, this));
-  translate_script_request_pending_->SetLoadFlags(
-      net::LOAD_DO_NOT_SEND_COOKIES | net::LOAD_DO_NOT_SAVE_COOKIES);
-  translate_script_request_pending_->SetRequestContext(
-      g_browser_process->system_request_context());
-  translate_script_request_pending_->SetExtraRequestHeaders(
-      kTranslateScriptHeader);
-  translate_script_request_pending_->Start();
+void TranslateManager::OnTranslateScriptFetchComplete(
+    bool success, const std::string& data) {
+  ProcessPendingRequests(!success);

[Takashi Toyoshima, 2013/06/21 05:38:55]

How about moving ProcessPendingRequests() implementation here.
There is no reason to factor it out.

[hajimehoshi, 2013/06/21 07:01:44]

Done.

 }
 
 // static