gdrive: Get JSON feeds parsing off the UI thread

chrome/browser/chromeos/gdata/gdata_operations.cc

Index: chrome/browser/chromeos/gdata/gdata_operations.cc
diff --git a/chrome/browser/chromeos/gdata/gdata_operations.cc b/chrome/browser/chromeos/gdata/gdata_operations.cc
index 5c4adf5f14bd81b9536f884963b4d5a02853fd40..c373e62a39171990910fbdab0f9e41316a69e51a 100644
--- a/chrome/browser/chromeos/gdata/gdata_operations.cc
+++ b/chrome/browser/chromeos/gdata/gdata_operations.cc
@@ -160,6 +160,28 @@ GURL FormatDocumentListURL(const std::string& directory_resource_id) {
                                      directory_resource_id).c_str()));
 }
 
+// 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 {
@@ -448,7 +470,9 @@ void EntryActionOperation::RunCallbackOnPrematureFailure(GDataErrorCode code) {
 GetDataOperation::GetDataOperation(GDataOperationRegistry* registry,
                                    Profile* profile,
                                    const GetDataCallback& callback)
-    : UrlFetchOperationBase(registry, profile), callback_(callback) {
+    : UrlFetchOperationBase(registry, profile),
+      callback_(callback),
+      weak_ptr_factory_(this) {
 }
 
 GetDataOperation::~GetDataOperation() {}
@@ -456,49 +480,70 @@ GetDataOperation::~GetDataOperation() {}
 bool GetDataOperation::ProcessURLFetchResults(const net::URLFetcher* source) {
   std::string data;
   source->GetResponseAsString(&data);
-  scoped_ptr<base::Value> root_value;
-  GDataErrorCode code = GetErrorCode(source);
+  GDataErrorCode fetch_error_code = GetErrorCode(source);
+  bool ret = false;
 
-  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:
+      ret = ParseResponse(fetch_error_code, data);
       break;
-    }
     default:
+      RunCallback(fetch_error_code, scoped_ptr<base::Value>());
       break;
   }
 
-  if (!callback_.is_null())
-    callback_.Run(code, root_value.Pass());
-  return root_value.get() != NULL;

[hashimoto, 2012/07/11 03:32:08]

Look like this code had a bug where the return value is always false whenever
|callback_| is not null.
Good to see this bug is also fixed :)

+  return ret;
 }
 
-void GetDataOperation::RunCallbackOnPrematureFailure(GDataErrorCode code) {
+void GetDataOperation::RunCallbackOnPrematureFailure(
+    GDataErrorCode fetch_error_code) {
   if (!callback_.is_null()) {
     scoped_ptr<base::Value> root_value;
-    callback_.Run(code, root_value.Pass());
+    callback_.Run(fetch_error_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;
-  }
-  return root_value.release();
+bool 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, even when OnDataParsed() is not called.
+  scoped_ptr<base::Value>* parsed_value = new scoped_ptr<base::Value>();
+
+  return BrowserThread::PostBlockingPoolTaskAndReply(
+      FROM_HERE,
+      base::Bind(&ParseJsonOnBlockingPool,
+                 data,
+                 parsed_value),
+      base::Bind(&GetDataOperation::OnDataParsed,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 fetch_error_code,
+                 base::Owned(parsed_value)));
+}
+
+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());
+}
+
+void GetDataOperation::OnDataParsed(GDataErrorCode fetch_error_code,
+                                    scoped_ptr<base::Value>* value) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (!value->get())
+    fetch_error_code = GDATA_PARSE_ERROR;
+
+  // The ownership of the parsed json object is transfered to RunCallBack(),
+  // keeping the ownership of the |value| here.
+  RunCallback(fetch_error_code, value->Pass());
+  DCHECK(!value->get());
+
+  // |value| will be deleted after return beause it is base::Owned()'d.
 }
 
 //============================ GetDocumentsOperation ===========================
@@ -859,7 +904,8 @@ bool AuthorizeAppsOperation::GetContentData(std::string* upload_content_type,
   return true;
 }
 
-base::Value* AuthorizeAppsOperation::ParseResponse(const std::string& data) {
+bool AuthorizeAppsOperation::ParseResponse(GDataErrorCode code,
+                                           const std::string& data) {
   // Parse entry XML.
   XmlReader xml_reader;
   scoped_ptr<DocumentEntry> entry;
@@ -888,7 +934,9 @@ base::Value* AuthorizeAppsOperation::ParseResponse(const std::string& data) {
     }
   }
 
-  return link_list.release();
+  RunCallback(code, link_list.PassAs<base::Value>());
+
+  return true;
 }
 
 GURL AuthorizeAppsOperation::GetURL() const {