gdrive: Get JSON feeds parsing off the UI thread.

chrome/browser/chromeos/gdata/operations_base.cc

 // Copyright (c) 2012 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/chromeos/gdata/operations_base.h"
 
 #include "base/json/json_reader.h"
 #include "base/metrics/histogram.h"
 #include "base/string_number_conversions.h"
 #include "base/stringprintf.h"
@@ skipping 27 matching lines @@
 const char kGDataVersionHeader[] = "GData-Version: 3.0";
 
 // Maximum number of attempts for re-authentication per operation.
 const int kMaxReAuthenticateAttemptsPerOperation = 1;
 
 // OAuth scope for the documents API.
 const char kDocsListScope[] = "https://docs.google.com/feeds/";
 const char kSpreadsheetsScope[] = "https://spreadsheets.google.com/feeds/";
 const char kUserContentScope[] = "https://docs.googleusercontent.com/";
 
+// Parse JSON string to base::Value object.
+void ParseJsonOnBlockingPool(const std::string& data,
+                             scoped_ptr<base::Value>* value) {
+  DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  int error_code = -1;
+  std::string error_message;
+  value->reset(base::JSONReader::ReadAndReturnError(data,
+                                                    base::JSON_PARSE_RFC,
+                                                    &error_code,
+                                                    &error_message));
+
+  if (!value->get()) {
+    LOG(ERROR) << "Error while parsing entry response: "
+               << error_message
+               << ", code: "
+               << error_code
+               << ", data:\n"
+               << data;
+  }
+}
+
 }  // namespace
 
 namespace gdata {
 
 //================================ AuthOperation ===============================
 
 AuthOperation::AuthOperation(GDataOperationRegistry* registry,
                              Profile* profile,
                              const AuthStatusCallback& callback,
                              const std::string& refresh_token)
@@ skipping 165 matching lines @@
     const URLFetcher* source) const {
   GDataErrorCode code = static_cast<GDataErrorCode>(source->GetResponseCode());
   if (code == HTTP_SUCCESS && !source->GetStatus().is_success()) {
     // If the HTTP response code is SUCCESS yet the URL request failed, it is
     // likely that the failure is due to loss of connection.
     code = GDATA_NO_CONNECTION;
   }
   return code;
 }
 
+void UrlFetchOperationBase::OnProcessURLFetchResultsComplete(bool result) {
+  if (result)
+    NotifySuccessToOperationRegistry();
+  else
+    NotifyFinish(GDataOperationRegistry::OPERATION_FAILED);
+}
+
 void UrlFetchOperationBase::OnURLFetchComplete(const URLFetcher* source) {
   GDataErrorCode code = GetErrorCode(source);
   DVLOG(1) << "Response headers:\n" << GetResponseHeadersAsString(source);
 
   if (code == HTTP_UNAUTHORIZED) {
     if (!re_authenticate_callback_.is_null() &&
         ++re_authenticate_count_ <= kMaxReAuthenticateAttemptsPerOperation) {
       re_authenticate_callback_.Run(this);
       return;
     }
 
     OnAuthFailed(code);
     return;
   }
 
   // Overridden by each specialization
-  bool success = ProcessURLFetchResults(source);
-  if (success)
-    NotifySuccessToOperationRegistry();
-  else
-    NotifyFinish(GDataOperationRegistry::OPERATION_FAILED);
+  ProcessURLFetchResults(source);
 }
 
 void UrlFetchOperationBase::NotifySuccessToOperationRegistry() {
   NotifyFinish(GDataOperationRegistry::OPERATION_COMPLETED);
 }
 
 void UrlFetchOperationBase::NotifyStartToOperationRegistry() {
   NotifyStart();
 }
 
@@ skipping 39 matching lines @@
                                            Profile* profile,
                                            const EntryActionCallback& callback,
                                            const GURL& document_url)
     : UrlFetchOperationBase(registry, profile),
       callback_(callback),
       document_url_(document_url) {
 }
 
 EntryActionOperation::~EntryActionOperation() {}
 
-bool EntryActionOperation::ProcessURLFetchResults(
-    const URLFetcher* source) {
+void EntryActionOperation::ProcessURLFetchResults(const URLFetcher* source) {
   if (!callback_.is_null()) {
     GDataErrorCode code = GetErrorCode(source);
     callback_.Run(code, document_url_);
   }
-  return true;
+  OnProcessURLFetchResultsComplete(true);

[hashimoto, 2012/07/26 02:08:44]

Do something (replace true with "const bool success = true" or add comment, etc)
to make it clear that this 'true' means success.

[yoshiki, 2012/07/26 17:31:40]

Done.

 }
 
 void EntryActionOperation::RunCallbackOnPrematureFailure(GDataErrorCode code) {
   if (!callback_.is_null())
     callback_.Run(code, document_url_);
 }
 
 //============================== GetDataOperation ==============================
 
 GetDataOperation::GetDataOperation(GDataOperationRegistry* registry,
                                    Profile* profile,
                                    const GetDataCallback& callback)
-    : UrlFetchOperationBase(registry, profile), callback_(callback) {
+    : UrlFetchOperationBase(registry, profile),
+      callback_(callback) {
 }
 
 GetDataOperation::~GetDataOperation() {}
 
-bool GetDataOperation::ProcessURLFetchResults(const URLFetcher* source) {
+void GetDataOperation::ProcessURLFetchResults(const URLFetcher* source) {
   std::string data;
   source->GetResponseAsString(&data);
   scoped_ptr<base::Value> root_value;
-  GDataErrorCode code = GetErrorCode(source);
+  GDataErrorCode fetch_error_code = GetErrorCode(source);
 
-  switch (code) {
+  switch (fetch_error_code) {
     case HTTP_SUCCESS:
-    case HTTP_CREATED: {
-      root_value.reset(ParseResponse(data));
-      if (!root_value.get())
-        code = GDATA_PARSE_ERROR;
-
+    case HTTP_CREATED:
+      ParseResponse(fetch_error_code, data);
       break;
-    }
     default:
+      RunCallback(fetch_error_code, scoped_ptr<base::Value>());
+      OnProcessURLFetchResultsComplete(false);

[hashimoto, 2012/07/26 02:08:44]

ditto.

[yoshiki, 2012/07/26 17:31:40]

Done.

       break;
   }
-
-  if (!callback_.is_null())
-    callback_.Run(code, root_value.Pass());
-  return root_value.get() != NULL;
-}
-
-void GetDataOperation::RunCallbackOnPrematureFailure(GDataErrorCode code) {
-  if (!callback_.is_null()) {
-    scoped_ptr<base::Value> root_value;
-    callback_.Run(code, root_value.Pass());
-  }
 }
 
-base::Value* GetDataOperation::ParseResponse(const std::string& data) {
-  int error_code = -1;
-  std::string error_message;
-  scoped_ptr<base::Value> root_value(base::JSONReader::ReadAndReturnError(
-      data, base::JSON_PARSE_RFC, &error_code, &error_message));
-  if (!root_value.get()) {
-    LOG(ERROR) << "Error while parsing entry response: "
-               << error_message
-               << ", code: "
-               << error_code
-               << ", data:\n"
-               << data;
-    return NULL;
+void GetDataOperation::RunCallbackOnPrematureFailure(
+    GDataErrorCode fetch_error_code) {
+  if (!callback_.is_null()) {
+    scoped_ptr<base::Value> root_value;
+    callback_.Run(fetch_error_code, root_value.Pass());
   }
-  return root_value.release();
+}
+
+void GetDataOperation::ParseResponse(GDataErrorCode fetch_error_code,
+                                     const std::string& data) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // Uses this hack to avoid deep-copy of json object because json might be so
+  // big. This pointer of scped_ptr is to ensure a deletion of the parsed json
+  // value object.
+  scoped_ptr<base::Value>* parsed_value = new scoped_ptr<base::Value>();
+
+  BrowserThread::PostBlockingPoolTaskAndReply(
+      FROM_HERE,
+      base::Bind(&ParseJsonOnBlockingPool,
+                 data,
+                 parsed_value),
+      base::Bind(&GetDataOperation::OnDataParsed,
+                 base::Unretained(this),

[hashimoto, 2012/07/26 02:08:44]

Use WeakPtr

[yoshiki, 2012/07/26 17:31:40]

Done.

+                 fetch_error_code,
+                 base::Owned(parsed_value),
+                 callback_));
+}
+
+void GetDataOperation::OnDataParsed(
+    gdata::GDataErrorCode fetch_error_code,
+    scoped_ptr<base::Value>* value,
+    const gdata::GetDataCallback& callback) {

[hashimoto, 2012/07/26 02:08:44]

Since |callback_| is always passed, this argument is no longer needed.

[yoshiki, 2012/07/26 17:31:40]

Done.

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  bool ret = true;

[hashimoto, 2012/07/26 02:08:44]

|ret| is ambiguous, rename this variable to |success| or something more
informative.

[yoshiki, 2012/07/26 17:31:40]

Done.

+  if (!value->get()) {
+    fetch_error_code = gdata::GDATA_PARSE_ERROR;
+    ret = false;
+  }
+
+  // The ownership of the parsed json object is transfered to RunCallBack(),
+  // keeping the ownership of the |value| here.
+  if (!callback.is_null())

[hashimoto, 2012/07/26 02:08:44]

Call RunCallback() instead.

[yoshiki, 2012/07/26 17:31:40]

Done.

+    callback.Run(fetch_error_code, value->Pass());
+
+  DCHECK(!value->get());
+
+  OnProcessURLFetchResultsComplete(ret);
+  // |value| will be deleted after return beause it is base::Owned()'d.
+}
+
+void GetDataOperation::RunCallback(GDataErrorCode fetch_error_code,
+                                   scoped_ptr<base::Value> value) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (!callback_.is_null())
+    callback_.Run(fetch_error_code, value.Pass());
 }
 
 }  // namespace gdata