Introduce TranslateService and TranslateDownloadManager

components/translate/core/browser/translate_url_fetcher.cc

 // Copyright 2014 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 "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"
 #include "net/url_request/url_request_status.h"
 
 namespace {
 
 // Retry parameter for fetching.
 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() {
 }
 
 bool TranslateURLFetcher::Request(
     const GURL& url,
     const TranslateURLFetcher::Callback& callback) {
   // This function is not supposed to be called before previous operaion is not
   // finished.
@@ skipping 10 matching lines @@
   url_ = url;
   callback_ = callback;
 
   fetcher_.reset(net::URLFetcher::Create(
       id_,
       url_,
       net::URLFetcher::GET,
       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.
   fetcher_->SetMaxRetriesOn5xx(max_retry_on_5xx_);
   if (!extra_request_header_.empty())
     fetcher_->SetExtraRequestHeaders(extra_request_header_);
 
   fetcher_->Start();
 
   return true;
 }
 
 void TranslateURLFetcher::OnURLFetchComplete(const net::URLFetcher* source) {
   DCHECK(fetcher_.get() == source);
 
   std::string data;
   if (source->GetStatus().status() == net::URLRequestStatus::SUCCESS &&
       source->GetResponseCode() == net::HTTP_OK) {
     state_ = COMPLETED;
     source->GetResponseAsString(&data);
   } else {
     state_ = FAILED;
   }
 
   // Transfer URLFetcher's ownership before invoking a callback.
   scoped_ptr<const net::URLFetcher> delete_ptr(fetcher_.release());
   callback_.Run(id_, state_ == COMPLETED, data);
 }