Converted ResumeUploadOperation to use JSON in response instead of XML.

Converted ResumeUploadOperation to use JSON in response instead of XML.

This was the last operation using xml in response. It is now converted to json, what simplifies the code significantly.

BUG=163303
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173427

[mtomasz, 2012-12-13 09:51:00]

@satorux: PTAL.

[satorux1, 2012-12-14 00:17:24]

https://codereview.chromium.org/11577002/diff/2001/chrome/browser/google_apis...
File chrome/browser/google_apis/base_operations.cc (right):

https://codereview.chromium.org/11577002/diff/2001/chrome/browser/google_apis...
chrome/browser/google_apis/base_operations.cc:300:
base::PostTaskAndReplyWithResult(
I think you can use ParseJson() here.

https://codereview.chromium.org/11577002/diff/2001/chrome/browser/google_apis...
File chrome/browser/google_apis/base_operations.h (right):

https://codereview.chromium.org/11577002/diff/2001/chrome/browser/google_apis...
chrome/browser/google_apis/base_operations.h:40: void parseJson(const
std::string& data, const ParseJsonCallback& callback);
parseJson -> ParseJson

Calls |callback| when finished -> Runs |callback| when finished on the calling
thread.


Could you also add a unit test for this?

[mtomasz, 2012-12-14 04:58:36]

https://codereview.chromium.org/11577002/diff/2001/chrome/browser/google_apis...
File chrome/browser/google_apis/base_operations.cc (right):

https://codereview.chromium.org/11577002/diff/2001/chrome/browser/google_apis...
chrome/browser/google_apis/base_operations.cc:300:
base::PostTaskAndReplyWithResult(
On 2012/12/14 00:17:24, satorux1 wrote:
> I think you can use ParseJson() here.

Done.

https://codereview.chromium.org/11577002/diff/2001/chrome/browser/google_apis...
File chrome/browser/google_apis/base_operations.h (right):

https://codereview.chromium.org/11577002/diff/2001/chrome/browser/google_apis...
chrome/browser/google_apis/base_operations.h:40: void parseJson(const
std::string& data, const ParseJsonCallback& callback);
On 2012/12/14 00:17:24, satorux1 wrote:
> parseJson -> ParseJson
> 
> Calls |callback| when finished -> Runs |callback| when finished on the calling
> thread.
> 
> 
> Could you also add a unit test for this?

Done.

[satorux1, 2012-12-14 05:02:42]

LGTM with nits:

https://codereview.chromium.org/11577002/diff/7001/chrome/browser/google_apis...
File chrome/browser/google_apis/base_operations.h (right):

https://codereview.chromium.org/11577002/diff/7001/chrome/browser/google_apis...
chrome/browser/google_apis/base_operations.h:36: typedef
base::Callback<void(scoped_ptr<base::Value>)> ParseJsonCallback;
Please add a parameter name:

typedef base::Callback<void(scoped_ptr<base::Value> value)> ParseJsonCallback;

https://codereview.chromium.org/11577002/diff/7001/chrome/browser/google_apis...
chrome/browser/google_apis/base_operations.h:41: void ParseJson(const
std::string& data, const ParseJsonCallback& callback);
nit: data -> json?

https://codereview.chromium.org/11577002/diff/7001/chrome/browser/google_apis...
File chrome/browser/google_apis/base_operations_unittest.cc (right):

https://codereview.chromium.org/11577002/diff/7001/chrome/browser/google_apis...
chrome/browser/google_apis/base_operations_unittest.cc:59: bool
get_data_callback_called;
class data members should end with _;

[mtomasz, 2012-12-14 05:43:48]

https://codereview.chromium.org/11577002/diff/7001/chrome/browser/google_apis...
File chrome/browser/google_apis/base_operations.h (right):

https://codereview.chromium.org/11577002/diff/7001/chrome/browser/google_apis...
chrome/browser/google_apis/base_operations.h:36: typedef
base::Callback<void(scoped_ptr<base::Value>)> ParseJsonCallback;
On 2012/12/14 05:02:42, satorux1 wrote:
> Please add a parameter name:
> 
> typedef base::Callback<void(scoped_ptr<base::Value> value)> ParseJsonCallback;

Done.

https://codereview.chromium.org/11577002/diff/7001/chrome/browser/google_apis...
chrome/browser/google_apis/base_operations.h:36: typedef
base::Callback<void(scoped_ptr<base::Value>)> ParseJsonCallback;
On 2012/12/14 05:02:42, satorux1 wrote:
> Please add a parameter name:
> 
> typedef base::Callback<void(scoped_ptr<base::Value> value)> ParseJsonCallback;

Done.

https://codereview.chromium.org/11577002/diff/7001/chrome/browser/google_apis...
chrome/browser/google_apis/base_operations.h:41: void ParseJson(const
std::string& data, const ParseJsonCallback& callback);
On 2012/12/14 05:02:42, satorux1 wrote:
> nit: data -> json?

Done.

https://codereview.chromium.org/11577002/diff/7001/chrome/browser/google_apis...
File chrome/browser/google_apis/base_operations_unittest.cc (right):

https://codereview.chromium.org/11577002/diff/7001/chrome/browser/google_apis...
chrome/browser/google_apis/base_operations_unittest.cc:59: bool
get_data_callback_called;
On 2012/12/14 05:02:42, satorux1 wrote:
> class data members should end with _;

Done.

[commit-bot: I haz the power, 2012-12-14 05:46:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mtomasz@chromium.org/11577002/1012

[commit-bot: I haz the power, 2012-12-14 05:46:26]

Failed to apply patch for
chrome/browser/google_apis/gdata_wapi_operations_unittest.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/google_apis/gdata_wapi_operations_unittest.cc
  Hunk #1 FAILED at 351.
  Hunk #2 succeeded at 893 (offset -22 lines).
  Hunk #3 succeeded at 992 (offset -22 lines).
  Hunk #4 succeeded at 1116 (offset -22 lines).
  Hunk #5 succeeded at 1212 (offset -22 lines).
  1 out of 5 hunks FAILED -- saving rejects to file
chrome/browser/google_apis/gdata_wapi_operations_unittest.cc.rej

Patch:       chrome/browser/google_apis/gdata_wapi_operations_unittest.cc
Index: chrome/browser/google_apis/gdata_wapi_operations_unittest.cc
diff --git a/chrome/browser/google_apis/gdata_wapi_operations_unittest.cc
b/chrome/browser/google_apis/gdata_wapi_operations_unittest.cc
index
8225658430198983b884f36a5f6761869eff4fb9..74f6eeaa05eb02588fad344bc3ca3a8723a4aae3
100644
--- a/chrome/browser/google_apis/gdata_wapi_operations_unittest.cc
+++ b/chrome/browser/google_apis/gdata_wapi_operations_unittest.cc
@@ -351,11 +351,11 @@ class GDataWapiOperationsTest : public testing::Test {
       return scoped_ptr<test_server::HttpResponse>();
     }
 
-    // TODO(satorux): We should create a correct entry for the uploaded file,
-    // but for now, just return entry.xml.
+    // TODO(satorux): We should create a correct JSON data for the uploaded
+    // file, but for now, just return file_entry.json.
     scoped_ptr<test_server::HttpResponse> response =
         CreateHttpResponseFromFile(
-            test_util::GetTestFilePath("gdata/entry.xml"));
+            test_util::GetTestFilePath("gdata/file_entry.json"));
     // response.code() is set to SUCCESS. Change it to CREATED if it's a new
     // file.
     if (absolute_url.path() == "/upload_new_file")
@@ -915,7 +915,8 @@ TEST_F(GDataWapiOperationsTest, UploadNewFile) {
   EXPECT_EQ(test_server_.GetURL("/upload_new_file"), upload_url);
   EXPECT_EQ(test_server::METHOD_POST, http_request_.method);
   // convert=false should be passed as files should be uploaded as-is.
-  EXPECT_EQ("/feeds/upload/create-session/default/private/full?convert=false",
+  EXPECT_EQ("/feeds/upload/create-session/default/private/full"
+            "?convert=false&v=3&alt=json",
             http_request_.relative_url);
   EXPECT_EQ("text/plain", http_request_.headers["X-Upload-Content-Type"]);
   EXPECT_EQ("application/atom+xml", http_request_.headers["Content-Type"]);
@@ -1013,7 +1014,8 @@ TEST_F(GDataWapiOperationsTest, UploadNewLargeFile) {
   EXPECT_EQ(test_server_.GetURL("/upload_new_file"), upload_url);
   EXPECT_EQ(test_server::METHOD_POST, http_request_.method);
   // convert=false should be passed as files should be uploaded as-is.
-  EXPECT_EQ("/feeds/upload/create-session/default/private/full?convert=false",
+  EXPECT_EQ("/feeds/upload/create-session/default/private/full?convert=false"
+            "&v=3&alt=json",
             http_request_.relative_url);
   EXPECT_EQ("text/plain", http_request_.headers["X-Upload-Content-Type"]);
   EXPECT_EQ("application/atom+xml", http_request_.headers["Content-Type"]);
@@ -1136,7 +1138,8 @@ TEST_F(GDataWapiOperationsTest, UploadNewEmptyFile) {
   EXPECT_EQ(test_server_.GetURL("/upload_new_file"), upload_url);
   EXPECT_EQ(test_server::METHOD_POST, http_request_.method);
   // convert=false should be passed as files should be uploaded as-is.
-  EXPECT_EQ("/feeds/upload/create-session/default/private/full?convert=false",
+  EXPECT_EQ("/feeds/upload/create-session/default/private/full?convert=false"
+            "&v=3&alt=json",
             http_request_.relative_url);
   EXPECT_EQ("text/plain", http_request_.headers["X-Upload-Content-Type"]);
   EXPECT_EQ("application/atom+xml", http_request_.headers["Content-Type"]);
@@ -1231,7 +1234,7 @@ TEST_F(GDataWapiOperationsTest, UploadExistingFile) {
   EXPECT_EQ(test_server::METHOD_PUT, http_request_.method);
   // convert=false should be passed as files should be uploaded as-is.
   EXPECT_EQ("/feeds/upload/create-session/default/private/full/file:foo"
-            "?convert=false",
+            "?convert=false&v=3&alt=json",
             http_request_.relative_url);
   // Even though the body is empty, the content type should be set to
   // "text/plain".

[commit-bot: I haz the power, 2012-12-14 05:52:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mtomasz@chromium.org/11577002/7002

[commit-bot: I haz the power, 2012-12-14 06:22:49]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[mtomasz, 2012-12-14 07:28:53]

@satorux: I had to fix the tests. Before, I had compiled them in Release dir,
while checking older version from Debug/ dir so didn't spot the errors. If you
have any concerns I'll stop the CQ.

[commit-bot: I haz the power, 2012-12-14 07:42:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mtomasz@chromium.org/11577002/8014

[commit-bot: I haz the power, 2012-12-14 09:40:38]

Retried try job too often on win_rel for step(s) nacl_integration

[commit-bot: I haz the power, 2012-12-14 10:22:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mtomasz@chromium.org/11577002/8014

[commit-bot: I haz the power, 2012-12-17 04:11:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mtomasz@chromium.org/11577002/15001

[commit-bot: I haz the power, 2012-12-17 06:53:22]

Retried try job too often on linux_chromeos for step(s) browser_tests