Introduce TranslateService and TranslateDownloadManager

components/translate/core/browser/translate_url_fetcher.cc

Index: components/translate/core/browser/translate_url_fetcher.cc
diff --git a/components/translate/core/browser/translate_url_fetcher.cc b/components/translate/core/browser/translate_url_fetcher.cc
index a02936941b0d892b05e07e849d96ce080d25720d..85988a9125e93559975b5fcf8d7bfbbb771e6daf 100644
--- a/components/translate/core/browser/translate_url_fetcher.cc
+++ b/components/translate/core/browser/translate_url_fetcher.cc
@@ -4,7 +4,7 @@
 
 #include "components/translate/core/browser/translate_url_fetcher.h"
 
-#include "components/translate/core/browser/translate_delegate.h"
+#include "components/translate/core/browser/translate_download_manager.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/url_fetcher.h"
@@ -17,11 +17,9 @@ const int kMaxRetry = 16;
 
 }  // namespace
 
-TranslateURLFetcher::TranslateURLFetcher(int id, TranslateDelegate* delegate)
-    : id_(id),
-      translate_delegate_(delegate),
-      state_(IDLE),
-      retry_count_(0) {
+TranslateURLFetcher::TranslateURLFetcher(int id) : id_(id),
+                                                   state_(IDLE),
+                                                   retry_count_(0) {
 }
 
 TranslateURLFetcher::~TranslateURLFetcher() {
@@ -52,7 +50,8 @@ bool TranslateURLFetcher::Request(
       this));
   fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
                          net::LOAD_DO_NOT_SAVE_COOKIES);
-  fetcher_->SetRequestContext(translate_delegate_->GetURLRequestContext());
+  fetcher_->SetRequestContext(
+      TranslateDownloadManager::GetInstance()->request_context());

[blundell, 2014/01/24 12:36:59]

Could you explain this a little more to me? It's not obvious why these two
request contexts should necessarily be the same one at a conceptual level.

[droger, 2014/01/24 13:44:31]

I'm not sure what your concern is here.
Previously it was using TranslateDelegate, which was calling into
g_browser_process.
Now we're using the request context that was initialized at startup (by
TranslateService) with the same request context from g_browser_process.

[blundell, 2014/01/24 15:01:46]

I agree that the change is safe to make at the current time, i.e., it doesn't
change behavior. What I'm wondering is whether it's conceptually correct, i.e.,
is there some inherent reason that TranslateURLFetcher should always be using
TranslateDownloadManager's request context (e.g., vs. one passed in from the
embedder).

On 2014/01/24 13:44:31, droger wrote:

[droger, 2014/01/24 15:14:54]

Do you mean that somehow g_browser_process->system_request_context() could
change?

Or do you mean that some hypothetical embedder other than chrome might want to
change it?

If it's the latter, then there is the set_request_context() method on
TranslateDownloadManager that can be used, and I don't think we need more
flexibility: this fetcher is typically only used twice (once for the translate
script and once for the language list), so it doesn't need a complex logic.

   // Set retry parameter for HTTP status code 5xx. This doesn't work against
   // 106 (net::ERR_INTERNET_DISCONNECTED) and so on.
   // TranslateLanguageList handles network status, and implements retry.