Revert 86340 - Move response_container_ into the URLFetcher::Core.

It is accessed from the io thread after the fetcher is destroyed on the ui thread.

BUG=none
TEST=Reliability bots.

Review URL: http://codereview.chromium.org/7065008

TBR=skerner@chromium.org
Review URL: http://codereview.chromium.org/6966021

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

chrome/common/net/test_url_fetcher_factory.cc

@@ -35,18 +35,18 @@ void TestURLFetcher::set_status(const net::URLRequestStatus& status) {
 }
 
 void TestURLFetcher::SetResponseString(const std::string& response) {
-  SetResponseDestinationForTesting(STRING);
+  response_destination_ = STRING;
   fake_response_string_ = response;
 }
 
 void TestURLFetcher::SetResponseFilePath(const FilePath& path) {
-  SetResponseDestinationForTesting(TEMP_FILE);
+  response_destination_ = TEMP_FILE;
   fake_response_file_path_ = path;
 }
 
 bool TestURLFetcher::GetResponseAsString(
     std::string* out_response_string) const {
-  if (GetResponseDestinationForTesting() != STRING)
+  if (response_destination_ != STRING)
     return false;
 
   *out_response_string = fake_response_string_;
@@ -55,7 +55,7 @@ bool TestURLFetcher::GetResponseAsString(
 
 bool TestURLFetcher::GetResponseAsFilePath(
     bool take_ownership, FilePath* out_response_path) const {
-  if (GetResponseDestinationForTesting() != TEMP_FILE)
+  if (response_destination_ != TEMP_FILE)
     return false;
 
   *out_response_path = fake_response_file_path_;

chrome/common/net/url_fetcher.cc

@@ -192,7 +192,7 @@ class URLFetcher::Core
   void AppendChunkToUpload(const std::string& data, bool is_last_chunk);
 
   // Store the response bytes in |buffer_| in the container indicated by
-  // |response_destination_|. Return true if the write has been
+  // |fetcher_->response_destination_|. Return true if the write has been
   // done, and another read can overwrite |buffer_|.  If this function
   // returns false, it will post a task that will read more bytes once the
   // write is complete.
@@ -265,9 +265,6 @@ class URLFetcher::Core
   // writing, and destruction of that file.
   scoped_ptr<TempFileWriter> temp_file_writer_;
 
-  // Where should responses be saved?
-  ResponseDestinationType response_destination_;
-
   static base::LazyInstance<Registry> g_registry;
 
   friend class URLFetcher;
@@ -469,6 +466,8 @@ void URLFetcher::Delegate::OnURLFetchComplete(const URLFetcher* source) {
   // parameter list to OnURLFetchComplete(). If a user asked to save
   // the response to a file, they must use the new parameter list,
   // in which case we can not get here.
+  CHECK(source->response_destination_ == STRING);
+
   // To avoid updating all callers, thunk to the old prototype for now.
   OnURLFetchComplete(source,
                      source->url(),
@@ -487,7 +486,8 @@ URLFetcher::URLFetcher(const GURL& url,
     : ALLOW_THIS_IN_INITIALIZER_LIST(
       core_(new Core(this, url, request_type, d))),
       automatically_retry_on_5xx_(true),
-      max_retries_(0) {
+      max_retries_(0),
+      response_destination_(STRING) {
 }
 
 URLFetcher::~URLFetcher() {
@@ -517,8 +517,7 @@ URLFetcher::Core::Core(URLFetcher* fetcher,
       buffer_(new net::IOBuffer(kBufferSize)),
       is_chunked_upload_(false),
       num_retries_(0),
-      was_cancelled_(false),
-      response_destination_(STRING) {
+      was_cancelled_(false) {
 }
 
 URLFetcher::Core::~Core() {
@@ -533,7 +532,7 @@ void URLFetcher::Core::Start() {
   io_message_loop_proxy_ = request_context_getter_->GetIOMessageLoopProxy();
   CHECK(io_message_loop_proxy_.get()) << "We need an IO message loop proxy";
 
-  switch (response_destination_) {
+  switch (fetcher_->response_destination_) {
     case STRING:
       io_message_loop_proxy_->PostTask(
           FROM_HERE,
@@ -615,7 +614,7 @@ void URLFetcher::Core::AppendChunkToUpload(const std::string& content,
 // be done later.
 bool URLFetcher::Core::WriteBuffer(int num_bytes) {
   bool write_complete = false;
-  switch (response_destination_) {
+  switch (fetcher_->response_destination_) {
     case STRING:
       data_.append(buffer_->data(), num_bytes);
       write_complete = true;
@@ -914,7 +913,7 @@ void URLFetcher::set_automatically_retry_on_5xx(bool retry) {
 void URLFetcher::SaveResponseToTemporaryFile(
     scoped_refptr<base::MessageLoopProxy> file_message_loop_proxy) {
   core_->file_message_loop_proxy_ = file_message_loop_proxy;
-  core_->response_destination_ = TEMP_FILE;
+  response_destination_ = TEMP_FILE;
 }
 
 net::HttpResponseHeaders* URLFetcher::response_headers() const {
@@ -973,7 +972,7 @@ void URLFetcher::ReceivedContentWasMalformed() {
 }
 
 bool URLFetcher::GetResponseAsString(std::string* out_response_string) const {
-  if (core_->response_destination_ != STRING)
+  if (response_destination_ != STRING)
     return false;
 
   *out_response_string = core_->data_;
@@ -981,24 +980,13 @@ bool URLFetcher::GetResponseAsString(std::string* out_response_string) const {
 }
 
 const std::string& URLFetcher::GetResponseStringRef() const {
-  CHECK(core_->response_destination_ == STRING);
+  CHECK(response_destination_ == STRING);
   return core_->data_;
 }
 
-void URLFetcher::SetResponseDestinationForTesting(
-    ResponseDestinationType value) {
-  core_->response_destination_ = value;
-}
-
-URLFetcher::ResponseDestinationType
-URLFetcher::GetResponseDestinationForTesting() const {
-  return core_->response_destination_;
-}
-
 bool URLFetcher::GetResponseAsFilePath(bool take_ownership,
                                        FilePath* out_response_path) const {
-  if (core_->response_destination_ != TEMP_FILE ||
-      !core_->temp_file_writer_.get())
+  if (response_destination_ != TEMP_FILE || !core_->temp_file_writer_.get())
     return false;
 
   *out_response_path = core_->temp_file_writer_->temp_file();

chrome/common/net/url_fetcher.h

@@ -263,12 +263,6 @@ class URLFetcher {
   static void CancelAll();
 
  protected:
-  // How should the response be stored?
-  enum ResponseDestinationType {
-    STRING,  // Default: In a std::string
-    TEMP_FILE  // Write to a temp file
-  };
-
   // Returns the delegate.
   Delegate* delegate() const;
 
@@ -281,13 +275,16 @@ class URLFetcher {
   // of crbug.com/83592 .
   const std::string& GetResponseStringRef() const;
 
-  void SetResponseDestinationForTesting(ResponseDestinationType);
-  ResponseDestinationType GetResponseDestinationForTesting() const;
-
  private:
   friend class URLFetcherTest;
   friend class TestURLFetcher;
 
+  // How should the response be stored?
+  enum ResponseDestinationType {
+    STRING,  // Default: In a std::string
+    TEMP_FILE  // Write to a temp file
+  };
+
   // Only used by URLFetcherTest, returns the number of URLFetcher::Core objects
   // actively running.
   static int GetNumFetcherCores();
@@ -307,6 +304,9 @@ class URLFetcher {
   // Maximum retries allowed.
   int max_retries_;
 
+  // Where should responses be saved?
+  ResponseDestinationType response_destination_;
+
   static bool g_interception_enabled;
 
   DISALLOW_COPY_AND_ASSIGN(URLFetcher);