google_apis: Stop accessing URLFetcher from ResponseWriter

To fix use-after-free

BUG=331822
TEST=See no crash after following the repro steps described in the BUG.

Review URL: https://codereview.chromium.org/127613002

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

2 files changed, 42 insertions, 19 deletions

diff --git a/google_apis/drive/base_requests.cc b/google_apis/drive/base_requests.cc
index 4a5ca70..be64f78 100644
--- a/google_apis/drive/base_requests.cc
+++ b/google_apis/drive/base_requests.cc
@@ -107,12 +107,11 @@ void ParseJson(base::TaskRunner* blocking_task_runner,
 }
 
 //=========================== ResponseWriter ==================================
-ResponseWriter::ResponseWriter(net::URLFetcher* url_fetcher,
-                               base::SequencedTaskRunner* file_task_runner,
+ResponseWriter::ResponseWriter(base::SequencedTaskRunner* file_task_runner,
                                const base::FilePath& file_path,
                                const GetContentCallback& get_content_callback)
-    : url_fetcher_(url_fetcher),
-      get_content_callback_(get_content_callback) {
+    : get_content_callback_(get_content_callback),
+      weak_ptr_factory_(this) {
   if (!file_path.empty()) {
     file_writer_.reset(
         new net::URLFetcherFileWriter(file_task_runner, file_path));
@@ -138,17 +137,21 @@ int ResponseWriter::Initialize(const net::CompletionCallback& callback) {
 int ResponseWriter::Write(net::IOBuffer* buffer,
                           int num_bytes,
                           const net::CompletionCallback& callback) {
-  // |get_content_callback_| and |file_writer_| are used only when the response
-  // code is successful one.
-  if (IsSuccessfulResponseCode(url_fetcher_->GetResponseCode())) {
-    if (!get_content_callback_.is_null()) {
-      get_content_callback_.Run(
-          HTTP_SUCCESS,
-          make_scoped_ptr(new std::string(buffer->data(), num_bytes)));
-    }
+  if (!get_content_callback_.is_null()) {
+    get_content_callback_.Run(
+        HTTP_SUCCESS,
+        make_scoped_ptr(new std::string(buffer->data(), num_bytes)));
+  }
 
-    if (file_writer_)
-      return file_writer_->Write(buffer, num_bytes, callback);
+  if (file_writer_) {
+    const int result = file_writer_->Write(
+        buffer, num_bytes,
+        base::Bind(&ResponseWriter::DidWrite,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   make_scoped_refptr(buffer), callback));
+    if (result != net::ERR_IO_PENDING)
+      DidWrite(buffer, net::CompletionCallback(), result);
+    return result;
   }
 
   data_.append(buffer->data(), num_bytes);
@@ -162,6 +165,24 @@ int ResponseWriter::Finish(const net::CompletionCallback& callback) {
   return net::OK;
 }
 
+void ResponseWriter::DidWrite(scoped_refptr<net::IOBuffer> buffer,
+                              const net::CompletionCallback& callback,
+                              int result) {
+  if (result > 0) {
+    // Even if file_writer_ is used, append the data to |data_|, so that it can
+    // be used to get error information in case of server side errors.
+    // The size limit is to avoid consuming too much redundant memory.
+    const size_t kMaxStringSize = 1024*1024;
+    if (data_.size() < kMaxStringSize) {
+      data_.append(buffer->data(), std::min(static_cast<size_t>(result),
+                                            kMaxStringSize - data_.size()));
+    }
+  }
+
+  if (!callback.is_null())
+    callback.Run(result);
+}
+
 //============================ UrlFetchRequestBase ===========================
 
 UrlFetchRequestBase::UrlFetchRequestBase(RequestSender* sender)
@@ -207,8 +228,7 @@ void UrlFetchRequestBase::Start(const std::string& access_token,
   GetOutputFilePath(&output_file_path, &get_content_callback);
   if (!get_content_callback.is_null())
     get_content_callback = CreateRelayCallback(get_content_callback);
-  response_writer_ = new ResponseWriter(url_fetcher_.get(),
-                                        blocking_task_runner(),
+  response_writer_ = new ResponseWriter(blocking_task_runner(),
                                         output_file_path,
                                         get_content_callback);
   url_fetcher_->SaveResponseWithWriter(
diff --git a/google_apis/drive/base_requests.h b/google_apis/drive/base_requests.h
index e3aee90..afe12a0 100644
--- a/google_apis/drive/base_requests.h
+++ b/google_apis/drive/base_requests.h
@@ -94,8 +94,7 @@ class ResponseWriter : public net::URLFetcherResponseWriter {
  public:
   // If file_path is not empty, the response will be saved with file_writer_,
   // otherwise it will be saved to data_.
-  ResponseWriter(net::URLFetcher* url_fetcher,
-                 base::SequencedTaskRunner* file_task_runner,
+  ResponseWriter(base::SequencedTaskRunner* file_task_runner,
                  const base::FilePath& file_path,
                  const GetContentCallback& get_content_callback);
   virtual ~ResponseWriter();
@@ -113,10 +112,14 @@ class ResponseWriter : public net::URLFetcherResponseWriter {
   virtual int Finish(const net::CompletionCallback& callback) OVERRIDE;
 
  private:
-  net::URLFetcher* url_fetcher_;
+  void DidWrite(scoped_refptr<net::IOBuffer> buffer,
+                const net::CompletionCallback& callback,
+                int result);
+
   const GetContentCallback get_content_callback_;
   std::string data_;
   scoped_ptr<net::URLFetcherFileWriter> file_writer_;
+  base::WeakPtrFactory<ResponseWriter> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ResponseWriter);
 };