Skip to content

GitLab

T
tangled
Commit bcdb8fac authored by joi@chromium.org's avatar joi@chromium.org
Browse files
Options

Revert 146104 - gdrive: Get JSON feeds parsing off the UI thread. 

BUG=128378
TEST=GDataOperationsTest.GetDataOperationParseJson passes.

Review URL: https://chromiumcodereview.appspot.com/10749015

TBR=yoshiki@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10704158

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@146110 0039d316-1c4b-4281-b951-d872f2087c98
parent 9eb87bb9
Hide whitespace changes
Inline Side-by-side
Showing with 35 additions and 253 deletions
chrome/browser/chromeos/gdata/gdata_operations.cc
View file @ bcdb8fac
...@@ -159,28 +159,6 @@ GURL FormatDocumentListURL(const std::string& directory_resource_id) { ...@@ -159,28 +159,6 @@ GURL FormatDocumentListURL(const std::string& directory_resource_id) {
directory_resource_id).c_str())); 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
namespace gdata { namespace gdata {
...@@ -469,9 +447,7 @@ void EntryActionOperation::RunCallbackOnPrematureFailure(GDataErrorCode code) { ...@@ -469,9 +447,7 @@ void EntryActionOperation::RunCallbackOnPrematureFailure(GDataErrorCode code) {
GetDataOperation::GetDataOperation(GDataOperationRegistry* registry, GetDataOperation::GetDataOperation(GDataOperationRegistry* registry,
Profile* profile, Profile* profile,
const GetDataCallback& callback) const GetDataCallback& callback)
: UrlFetchOperationBase(registry, profile), : UrlFetchOperationBase(registry, profile), callback_(callback) {
callback_(callback),
weak_ptr_factory_(this) {
} }
GetDataOperation::~GetDataOperation() {} GetDataOperation::~GetDataOperation() {}
...@@ -479,70 +455,49 @@ GetDataOperation::~GetDataOperation() {} ...@@ -479,70 +455,49 @@ GetDataOperation::~GetDataOperation() {}
bool GetDataOperation::ProcessURLFetchResults(const net::URLFetcher* source) { bool GetDataOperation::ProcessURLFetchResults(const net::URLFetcher* source) {
std::string data; std::string data;
source->GetResponseAsString(&data); source->GetResponseAsString(&data);
GDataErrorCode fetch_error_code = GetErrorCode(source); scoped_ptr<base::Value> root_value;
bool ret = false; GDataErrorCode code = GetErrorCode(source);
switch (fetch_error_code) { switch (code) {
case HTTP_SUCCESS: case HTTP_SUCCESS:
case HTTP_CREATED: case HTTP_CREATED: {
ret = ParseResponse(fetch_error_code, data); root_value.reset(ParseResponse(data));
if (!root_value.get())
code = GDATA_PARSE_ERROR;
break; break;
}
default: default:
RunCallback(fetch_error_code, scoped_ptr<base::Value>());
break; break;
} }
return ret; if (!callback_.is_null())
callback_.Run(code, root_value.Pass());
return root_value.get() != NULL;
} }
void GetDataOperation::RunCallbackOnPrematureFailure( void GetDataOperation::RunCallbackOnPrematureFailure(GDataErrorCode code) {
GDataErrorCode fetch_error_code) {
if (!callback_.is_null()) { if (!callback_.is_null()) {
scoped_ptr<base::Value> root_value; scoped_ptr<base::Value> root_value;
callback_.Run(fetch_error_code, root_value.Pass()); callback_.Run(code, root_value.Pass());
} }
} }
bool GetDataOperation::ParseResponse(GDataErrorCode fetch_error_code, base::Value* GetDataOperation::ParseResponse(const std::string& data) {
const std::string& data) { int error_code = -1;
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); std::string error_message;
scoped_ptr<base::Value> root_value(base::JSONReader::ReadAndReturnError(
// Uses this hack to avoid deep-copy of json object because json might be so data, base::JSON_PARSE_RFC, &error_code, &error_message));
// big. if (!root_value.get()) {
// This pointer of scped_ptr is to ensure a deletion of the parsed json value LOG(ERROR) << "Error while parsing entry response: "
// object, even when OnDataParsed() is not called. << error_message
scoped_ptr<base::Value>* parsed_value = new scoped_ptr<base::Value>(); << ", code: "
<< error_code
return BrowserThread::PostBlockingPoolTaskAndReply( << ", data:\n"
FROM_HERE, << data;
base::Bind(&ParseJsonOnBlockingPool, return NULL;
data, }
parsed_value), return root_value.release();
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 =========================== //============================ GetDocumentsOperation ===========================
...@@ -903,8 +858,7 @@ bool AuthorizeAppsOperation::GetContentData(std::string* upload_content_type, ...@@ -903,8 +858,7 @@ bool AuthorizeAppsOperation::GetContentData(std::string* upload_content_type,
return true; return true;
} }
bool AuthorizeAppsOperation::ParseResponse(GDataErrorCode code, base::Value* AuthorizeAppsOperation::ParseResponse(const std::string& data) {
const std::string& data) {
// Parse entry XML. // Parse entry XML.
XmlReader xml_reader; XmlReader xml_reader;
scoped_ptr<DocumentEntry> entry; scoped_ptr<DocumentEntry> entry;
...@@ -933,9 +887,7 @@ bool AuthorizeAppsOperation::ParseResponse(GDataErrorCode code, ...@@ -933,9 +887,7 @@ bool AuthorizeAppsOperation::ParseResponse(GDataErrorCode code,
} }
} }
RunCallback(code, link_list.PassAs<base::Value>()); return link_list.release();
return true;
} }
GURL AuthorizeAppsOperation::GetURL() const { GURL AuthorizeAppsOperation::GetURL() const {
......
chrome/browser/chromeos/gdata/gdata_operations.h
View file @ bcdb8fac
...@@ -191,25 +191,15 @@ class GetDataOperation : public UrlFetchOperationBase { ...@@ -191,25 +191,15 @@ class GetDataOperation : public UrlFetchOperationBase {
virtual ~GetDataOperation(); virtual ~GetDataOperation();
// Parse GData JSON response. // Parse GData JSON response.
virtual bool ParseResponse(GDataErrorCode fetch_error_code, virtual base::Value* ParseResponse(const std::string& data);
const std::string& data);
protected: protected:
// Overridden from UrlFetchOperationBase. // Overridden from UrlFetchOperationBase.
virtual bool ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE; virtual bool ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE;
virtual void RunCallbackOnPrematureFailure( virtual void RunCallbackOnPrematureFailure(GDataErrorCode code) OVERRIDE;
GDataErrorCode fetch_error_code) OVERRIDE;
void RunCallback(GDataErrorCode fetch_error_code,
scoped_ptr<base::Value> value);
private: private:
void OnDataParsed(GDataErrorCode fetch_error_code,
scoped_ptr<base::Value>* value);
GetDataCallback callback_; GetDataCallback callback_;
base::WeakPtrFactory<GetDataOperation> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(GetDataOperation); DISALLOW_COPY_AND_ASSIGN(GetDataOperation);
}; };
...@@ -450,8 +440,7 @@ class AuthorizeAppsOperation : public GetDataOperation { ...@@ -450,8 +440,7 @@ class AuthorizeAppsOperation : public GetDataOperation {
// Must override GetDataOperation's ParseResponse because the response is XML // Must override GetDataOperation's ParseResponse because the response is XML
// not JSON. // not JSON.
virtual bool ParseResponse(GDataErrorCode code, virtual base::Value* ParseResponse(const std::string& data) OVERRIDE;
const std::string& data) OVERRIDE;
private: private:
std::string app_id_; std::string app_id_;
GURL document_url_; GURL document_url_;
......
chrome/browser/chromeos/gdata/gdata_operations_unittest.cc deleted 100644 → 0
View file @ 9eb87bb9
// 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 "base/memory/scoped_ptr.h"
#include "base/message_loop.h"
#include "base/values.h"
#include "chrome/browser/chromeos/gdata/gdata_operations.h"
#include "chrome/browser/chromeos/gdata/gdata_operation_runner.h"
#include "chrome/browser/chromeos/gdata/gdata_test_util.h"
#include "chrome/test/base/testing_profile.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/test/test_browser_thread.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace gdata {
namespace {
class JsonParseTestGetDataOperation : public GetDataOperation {
public:
JsonParseTestGetDataOperation(GDataOperationRegistry* registry,
Profile* profile,
const GetDataCallback& callback)
: GetDataOperation(registry, profile, callback) {
}
virtual ~JsonParseTestGetDataOperation() {
}
void NotifyStart() {
NotifyStartToOperationRegistry();
}
void NotifySuccess() {
NotifySuccessToOperationRegistry();
}
protected:
// GetDataOperation overrides:
virtual GURL GetURL() const OVERRIDE {
// This method is never called because this test does not fetch json from
// network.
NOTREACHED();
return GURL();
}
};
} // namespace
class GDataOperationsTest : public testing::Test {
protected:
GDataOperationsTest()
: ui_thread_(content::BrowserThread::UI, &message_loop_) {
}
virtual void SetUp() OVERRIDE {
profile_.reset(new TestingProfile);
runner_.reset(new GDataOperationRunner(profile_.get()));
runner_->Initialize();
}
protected:
MessageLoopForUI message_loop_;
content::TestBrowserThread ui_thread_;
scoped_ptr<TestingProfile> profile_;
scoped_ptr<GDataOperationRunner> runner_;
};
void GetDataOperationParseJsonCallback(GDataErrorCode* error_out,
scoped_ptr<base::Value>* value_out,
GDataErrorCode error_in,
scoped_ptr<base::Value> value_in) {
value_out->swap(value_in);
*error_out = error_in;
}
TEST_F(GDataOperationsTest, GetDataOperationParseJson) {
scoped_ptr<base::Value> value;
GDataErrorCode error;
gdata::GetDataCallback cb = base::Bind(&GetDataOperationParseJsonCallback,
&error,
&value);
JsonParseTestGetDataOperation* getData =
new JsonParseTestGetDataOperation(runner_->operation_registry(),
profile_.get(),
cb);
getData->NotifyStart();
// Parses a valid json string.
{
std::string valid_json_str =
"{"
" \"test\": {"
" \"foo\": true,"
" \"bar\": 3.14,"
" \"baz\": \"bat\","
" \"moo\": \"cow\""
" },"
" \"list\": ["
" \"a\","
" \"b\""
" ]"
"}";
getData->ParseResponse(HTTP_SUCCESS, valid_json_str);
test_util::RunBlockingPoolTask();
message_loop_.RunAllPending();
EXPECT_EQ(HTTP_SUCCESS, error);
ASSERT_TRUE(value.get());
DictionaryValue* root_dict = NULL;
ASSERT_TRUE(value->GetAsDictionary(&root_dict));
DictionaryValue* dict = NULL;
ListValue* list = NULL;
ASSERT_TRUE(root_dict->GetDictionary("test", &dict));
ASSERT_TRUE(root_dict->GetList("list", &list));
Value* dict_literals[2] = {0};
Value* dict_strings[2] = {0};
Value* list_values[2] = {0};
EXPECT_TRUE(dict->Remove("foo", &dict_literals[0]));
EXPECT_TRUE(dict->Remove("bar", &dict_literals[1]));
EXPECT_TRUE(dict->Remove("baz", &dict_strings[0]));
EXPECT_TRUE(dict->Remove("moo", &dict_strings[1]));
ASSERT_EQ(2u, list->GetSize());
EXPECT_TRUE(list->Remove(0, &list_values[0]));
EXPECT_TRUE(list->Remove(0, &list_values[1]));
}
// Parses an invalid json string.
{
std::string invalid_json_str =
"/* hogehoge *"
" \"test\": {"
" \"moo\": \"cow"
" "
" \"list\": ["
" \"foo\","
" \"bar\""
" ]";
getData->ParseResponse(HTTP_SUCCESS, invalid_json_str);
test_util::RunBlockingPoolTask();
message_loop_.RunAllPending();
ASSERT_TRUE(value.get() == NULL);
EXPECT_EQ(GDATA_PARSE_ERROR, error);
}
getData->NotifySuccess();
}
} // namespace gdata
chrome/chrome_tests.gypi
View file @ bcdb8fac
...@@ -1108,7 +1108,6 @@ ...@@ -1108,7 +1108,6 @@
'browser/chromeos/gdata/gdata_file_system_unittest.cc', 'browser/chromeos/gdata/gdata_file_system_unittest.cc',
'browser/chromeos/gdata/gdata_files_unittest.cc', 'browser/chromeos/gdata/gdata_files_unittest.cc',
'browser/chromeos/gdata/gdata_operation_registry_unittest.cc', 'browser/chromeos/gdata/gdata_operation_registry_unittest.cc',
'browser/chromeos/gdata/gdata_operations_unittest.cc',
'browser/chromeos/gdata/gdata_parser_unittest.cc', 'browser/chromeos/gdata/gdata_parser_unittest.cc',
'browser/chromeos/gdata/gdata_sync_client_unittest.cc', 'browser/chromeos/gdata/gdata_sync_client_unittest.cc',
'browser/chromeos/gdata/gdata_util_unittest.cc', 'browser/chromeos/gdata/gdata_util_unittest.cc',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment