Refactoring: Create TranslateScript from TranslateManager

chrome/browser/translate/translate_script.cc

Index: chrome/browser/translate/translate_script.cc
diff --git a/chrome/browser/translate/translate_script.cc b/chrome/browser/translate/translate_script.cc
new file mode 100644
index 0000000000000000000000000000000000000000..1bb9cc20fceefe5e9f7ca1b24cb39fdb7fdd7d7d
--- /dev/null
+++ b/chrome/browser/translate/translate_script.cc
@@ -0,0 +1,121 @@
+// Copyright 2013 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/browser/translate/translate_script.h"
+
+#include "base/bind.h"
+#include "base/command_line.h"
+#include "base/logging.h"
+#include "base/message_loop.h"
+#include "base/strings/string_util.h"
+#include "chrome/browser/translate/translate_url_fetcher.h"
+#include "chrome/browser/translate/translate_url_util.h"
+#include "chrome/common/chrome_switches.h"
+#include "google_apis/google_api_keys.h"
+#include "grit/browser_resources.h"
+#include "net/base/escape.h"
+#include "net/base/url_util.h"
+#include "ui/base/resource/resource_bundle.h"
+
+namespace {
+
+const int kExpirationDelayDays = 1;
+
+const char kTranslateScriptURL[] =

[Takashi Toyoshima, 2013/06/20 06:59:53]

We can simplify these const names.
ScriptURL?

[hajimehoshi, 2013/06/20 08:50:08]

Done.

+    "https://translate.google.com/translate_a/element.js";
+const char kTranslateScriptHeader[] = "Google-Translate-Element-Mode: library";
+
+// Used in kTranslateScriptURL to specify a callback function name.
+const char kCallbackQueryName[] = "cb";
+const char kCallbackQueryValue[] =
+    "cr.googleTranslate.onTranslateElementLoad";
+
+}  // namespace
+
+TranslateScript::TranslateScript()
+    : weak_method_factory_(this),
+      expiration_delay_(base::TimeDelta::FromDays(kExpirationDelayDays)) {
+}
+
+TranslateScript::~TranslateScript() {
+  weak_method_factory_.InvalidateWeakPtrs();
+}
+
+void TranslateScript::Request(const Callback& callback) {
+  if (fetcher_.get() != NULL)

[Takashi Toyoshima, 2013/06/20 06:59:53]

NOTREACHED, and it's better to return false as TranslateURLFetcher did.

[hajimehoshi, 2013/06/20 08:50:08]

I expect returning boolean will be implemented an commited at your another CL.

On 2013/06/20 06:59:53, Takashi Toyoshima (chromium) wrote:

+    return;
+
+  callback_ = callback;
+
+  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);
+
+  fetcher_.reset(new TranslateURLFetcher(0));

[Takashi Toyoshima, 2013/06/20 06:59:53]

Define it as an anonymous const int and use it here and DCHECK below.

[hajimehoshi, 2013/06/20 08:50:08]

Done.

+  fetcher_->set_extra_request_header(kTranslateScriptHeader);
+  fetcher_->Request(
+      translate_script_url,
+      base::Bind(&TranslateScript::OnScriptFetchComplete,
+                 base::Unretained(this)));
+}
+
+bool TranslateScript::IsFetching() const {
+  return fetcher_ != NULL;

[Takashi Toyoshima, 2013/06/20 06:59:53]

.get() != NULL

Also, implementing it in the header looks a standard way for chromium to me.

[hajimehoshi, 2013/06/20 08:50:08]

Done.

+}
+
+void TranslateScript::OnScriptFetchComplete(
+    int id, bool success, const std::string& data) {
+  DCHECK_EQ(0, id);
+
+  scoped_ptr<const TranslateURLFetcher> delete_ptr(fetcher_.release());
+
+  if (success) {
+    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";
+    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(&TranslateScript::ClearTranslateScript,
+                   weak_method_factory_.GetWeakPtr()),
+        expiration_delay_);
+  }
+
+  callback_.Run(success, data);
+}