Skip to content

GitLab

T
tangled
Commit c95ae9dc authored by sergeyu@chromium.org's avatar sergeyu@chromium.org
Browse files
Options

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

BUG=128378
TEST=unit_tests:GData* and browser_tests:GData* passes.

using TBR for gyp changes
TBR=thakis@chromium.org


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

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@148996 0039d316-1c4b-4281-b951-d872f2087c98
parent a13fcbc4
Hide whitespace changes
Inline Side-by-side
Showing with 61 additions and 320 deletions
chrome/browser/chromeos/gdata/gdata_documents_service_browsertest.cc
View file @ c95ae9dc
......@@ -15,8 +15,6 @@
#include "net/test/test_server.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace gdata {
namespace {
class GDataTest : public InProcessBrowserTest {
......@@ -146,5 +144,3 @@ IN_PROC_BROWSER_TEST_F(GDataTest, GetDocumentsFailure) {
EXPECT_EQ(gdata::GDATA_PARSE_ERROR, result);
EXPECT_FALSE(result_data);
}
} // namespace gdata
chrome/browser/chromeos/gdata/gdata_operations.cc
View file @ c95ae9dc
......@@ -261,7 +261,8 @@ void DownloadFileOperation::OnURLFetchDownloadData(
get_download_data_callback_.Run(HTTP_SUCCESS, download_data.Pass());
}
void DownloadFileOperation::ProcessURLFetchResults(const URLFetcher* source) {
bool DownloadFileOperation::ProcessURLFetchResults(
const URLFetcher* source) {
GDataErrorCode code = GetErrorCode(source);
// Take over the ownership of the the downloaded temp file.
......@@ -274,7 +275,7 @@ void DownloadFileOperation::ProcessURLFetchResults(const URLFetcher* source) {
if (!download_action_callback_.is_null())
download_action_callback_.Run(code, document_url_, temp_file);
OnProcessURLFetchResultsComplete(code == HTTP_SUCCESS);
return code == HTTP_SUCCESS;
}
void DownloadFileOperation::RunCallbackOnPrematureFailure(GDataErrorCode code) {
......@@ -476,10 +477,11 @@ AuthorizeAppsOperation::GetExtraRequestHeaders() const {
return headers;
}
void AuthorizeAppsOperation::ProcessURLFetchResults(const URLFetcher* source) {
bool AuthorizeAppsOperation::ProcessURLFetchResults(
const URLFetcher* source) {
std::string data;
source->GetResponseAsString(&data);
GetDataOperation::ProcessURLFetchResults(source);
return GetDataOperation::ProcessURLFetchResults(source);
}
bool AuthorizeAppsOperation::GetContentData(std::string* upload_content_type,
......@@ -500,9 +502,7 @@ bool AuthorizeAppsOperation::GetContentData(std::string* upload_content_type,
return true;
}
void AuthorizeAppsOperation::ParseResponse(
GDataErrorCode fetch_error_code,
const std::string& data) {
base::Value* AuthorizeAppsOperation::ParseResponse(const std::string& data) {
// Parse entry XML.
XmlReader xml_reader;
scoped_ptr<DocumentEntry> entry;
......@@ -531,9 +531,7 @@ void AuthorizeAppsOperation::ParseResponse(
}
}
RunCallback(fetch_error_code, link_list.PassAs<base::Value>());
const bool success = true;
OnProcessURLFetchResultsComplete(success);
return link_list.release();
}
GURL AuthorizeAppsOperation::GetURL() const {
......@@ -646,7 +644,7 @@ GURL InitiateUploadOperation::GetURL() const {
return initiate_upload_url_;
}
void InitiateUploadOperation::ProcessURLFetchResults(
bool InitiateUploadOperation::ProcessURLFetchResults(
const URLFetcher* source) {
GDataErrorCode code = GetErrorCode(source);
......@@ -663,7 +661,7 @@ void InitiateUploadOperation::ProcessURLFetchResults(
if (!callback_.is_null())
callback_.Run(code, GURL(upload_location));
OnProcessURLFetchResultsComplete(code == HTTP_SUCCESS);
return code == HTTP_SUCCESS;
}
void InitiateUploadOperation::NotifySuccessToOperationRegistry() {
......@@ -750,7 +748,8 @@ GURL ResumeUploadOperation::GetURL() const {
return params_.upload_location;
}
void ResumeUploadOperation::ProcessURLFetchResults(const URLFetcher* source) {
bool ResumeUploadOperation::ProcessURLFetchResults(
const URLFetcher* source) {
GDataErrorCode code = GetErrorCode(source);
net::HttpResponseHeaders* hdrs = source->GetResponseHeaders();
int64 start_range_received = -1;
......@@ -811,8 +810,7 @@ void ResumeUploadOperation::ProcessURLFetchResults(const URLFetcher* source) {
last_chunk_completed_ = true;
}
OnProcessURLFetchResultsComplete(
last_chunk_completed_ || code == HTTP_RESUME_INCOMPLETE);
return last_chunk_completed_ || code == HTTP_RESUME_INCOMPLETE;
}
void ResumeUploadOperation::NotifyStartToOperationRegistry() {
......
chrome/browser/chromeos/gdata/gdata_operations.h
View file @ c95ae9dc
......@@ -100,7 +100,7 @@ class DownloadFileOperation : public UrlFetchOperationBase {
protected:
// Overridden from UrlFetchOperationBase.
virtual GURL GetURL() const OVERRIDE;
virtual void ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE;
virtual bool ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE;
virtual void RunCallbackOnPrematureFailure(GDataErrorCode code) OVERRIDE;
// Overridden from net::URLFetcherDelegate.
......@@ -249,12 +249,11 @@ class AuthorizeAppsOperation : public GetDataOperation {
// Overridden from GetDataOperation.
virtual GURL GetURL() const OVERRIDE;
virtual void ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE;
virtual bool ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE;
// Must override GetDataOperation's ParseResponse because the response is XML
// not JSON.
virtual void ParseResponse(GDataErrorCode fetch_error_code,
const std::string& data) OVERRIDE;
virtual base::Value* ParseResponse(const std::string& data) OVERRIDE;
private:
std::string app_id_;
GURL document_url_;
......@@ -329,7 +328,7 @@ class InitiateUploadOperation : public UrlFetchOperationBase {
protected:
// Overridden from UrlFetchOperationBase.
virtual GURL GetURL() const OVERRIDE;
virtual void ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE;
virtual bool ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE;
virtual void NotifySuccessToOperationRegistry() OVERRIDE;
virtual void RunCallbackOnPrematureFailure(GDataErrorCode code) OVERRIDE;
......@@ -361,7 +360,7 @@ class ResumeUploadOperation : public UrlFetchOperationBase {
protected:
// Overridden from UrlFetchOperationBase.
virtual GURL GetURL() const OVERRIDE;
virtual void ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE;
virtual bool ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE;
virtual void NotifyStartToOperationRegistry() OVERRIDE;
virtual void NotifySuccessToOperationRegistry() OVERRIDE;
virtual void RunCallbackOnPrematureFailure(GDataErrorCode code) OVERRIDE;
......
chrome/browser/chromeos/gdata/gdata_operations_unittest.cc deleted 100644 → 0
View file @ a13fcbc4
// 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();
}
void NotifyFailure() {
NotifyFinish(GDataOperationRegistry::OPERATION_FAILED);
}
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();
}
};
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;
}
} // 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_;
};
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();
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->Get("foo", &dict_literals[0]));
EXPECT_TRUE(dict->Get("bar", &dict_literals[1]));
EXPECT_TRUE(dict->Get("baz", &dict_strings[0]));
EXPECT_TRUE(dict->Get("moo", &dict_strings[1]));
ASSERT_EQ(2u, list->GetSize());
EXPECT_TRUE(list->Get(0, &list_values[0]));
EXPECT_TRUE(list->Get(1, &list_values[1]));
bool b = false;
double d = 0;
std::string s;
EXPECT_TRUE(dict_literals[0]->GetAsBoolean(&b));
EXPECT_TRUE(b);
EXPECT_TRUE(dict_literals[1]->GetAsDouble(&d));
EXPECT_EQ(3.14, d);
EXPECT_TRUE(dict_strings[0]->GetAsString(&s));
EXPECT_EQ("bat", s);
EXPECT_TRUE(dict_strings[1]->GetAsString(&s));
EXPECT_EQ("cow", s);
EXPECT_TRUE(list_values[0]->GetAsString(&s));
EXPECT_EQ("a", s);
EXPECT_TRUE(list_values[1]->GetAsString(&s));
EXPECT_EQ("b", s);
}
}
TEST_F(GDataOperationsTest, GetDataOperationParseInvalidJson) {
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 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();
EXPECT_EQ(GDATA_PARSE_ERROR, error);
ASSERT_TRUE(value.get() == NULL);
}
}
} // namespace gdata
chrome/browser/chromeos/gdata/operations_base.cc
View file @ c95ae9dc
......@@ -49,28 +49,6 @@ const char kUserContentScope[] = "https://docs.googleusercontent.com/";
// OAuth scope for Drive API.
const char kDriveAppsScope[] = "https://www.googleapis.com/auth/drive.apps";
// 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 {
......@@ -258,13 +236,6 @@ GDataErrorCode UrlFetchOperationBase::GetErrorCode(
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);
......@@ -281,7 +252,11 @@ void UrlFetchOperationBase::OnURLFetchComplete(const URLFetcher* source) {
}
// Overridden by each specialization
ProcessURLFetchResults(source);
bool success = ProcessURLFetchResults(source);
if (success)
NotifySuccessToOperationRegistry();
else
NotifyFinish(GDataOperationRegistry::OPERATION_FAILED);
}
void UrlFetchOperationBase::NotifySuccessToOperationRegistry() {
......@@ -341,13 +316,13 @@ EntryActionOperation::EntryActionOperation(GDataOperationRegistry* registry,
EntryActionOperation::~EntryActionOperation() {}
void EntryActionOperation::ProcessURLFetchResults(const URLFetcher* source) {
bool EntryActionOperation::ProcessURLFetchResults(
const URLFetcher* source) {
if (!callback_.is_null()) {
GDataErrorCode code = GetErrorCode(source);
callback_.Run(code, document_url_);
}
const bool success = true;
OnProcessURLFetchResultsComplete(success);
return true;
}
void EntryActionOperation::RunCallbackOnPrematureFailure(GDataErrorCode code) {
......@@ -360,86 +335,57 @@ void EntryActionOperation::RunCallbackOnPrematureFailure(GDataErrorCode code) {
GetDataOperation::GetDataOperation(GDataOperationRegistry* registry,
Profile* profile,
const GetDataCallback& callback)
: UrlFetchOperationBase(registry, profile),
callback_(callback),
weak_ptr_factory_(this) {
: UrlFetchOperationBase(registry, profile), callback_(callback) {
}
GetDataOperation::~GetDataOperation() {}
void GetDataOperation::ProcessURLFetchResults(const URLFetcher* source) {
bool GetDataOperation::ProcessURLFetchResults(const URLFetcher* source) {
std::string data;
source->GetResponseAsString(&data);
scoped_ptr<base::Value> root_value;
GDataErrorCode fetch_error_code = GetErrorCode(source);
GDataErrorCode code = GetErrorCode(source);
switch (fetch_error_code) {
switch (code) {
case HTTP_SUCCESS:
case HTTP_CREATED:
ParseResponse(fetch_error_code, data);
case HTTP_CREATED: {
root_value.reset(ParseResponse(data));
if (!root_value.get())
code = GDATA_PARSE_ERROR;
break;
}
default:
RunCallback(fetch_error_code, scoped_ptr<base::Value>());
const bool success = false;
OnProcessURLFetchResultsComplete(success);
break;
}
if (!callback_.is_null())
callback_.Run(code, root_value.Pass());
return root_value.get() != NULL;
}
void GetDataOperation::RunCallbackOnPrematureFailure(
GDataErrorCode fetch_error_code) {
void GetDataOperation::RunCallbackOnPrematureFailure(GDataErrorCode code) {
if (!callback_.is_null()) {
scoped_ptr<base::Value> root_value;
callback_.Run(fetch_error_code, root_value.Pass());
callback_.Run(code, root_value.Pass());
}
}
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,
weak_ptr_factory_.GetWeakPtr(),
fetch_error_code,
base::Owned(parsed_value)));
}
void GetDataOperation::OnDataParsed(
gdata::GDataErrorCode fetch_error_code,
scoped_ptr<base::Value>* value) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
bool success = true;
if (!value->get()) {
fetch_error_code = gdata::GDATA_PARSE_ERROR;
success = false;
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;
}
// 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());
OnProcessURLFetchResultsComplete(success);
// |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());
return root_value.release();
}
} // namespace gdata
chrome/browser/chromeos/gdata/operations_base.h
View file @ c95ae9dc
......@@ -112,7 +112,7 @@ class UrlFetchOperationBase : public GDataOperationInterface,
// Invoked by OnURLFetchComplete when the operation completes without an
// authentication error. Must be implemented by a derived class.
virtual void ProcessURLFetchResults(const net::URLFetcher* source) = 0;
virtual bool ProcessURLFetchResults(const net::URLFetcher* source) = 0;
// Invoked when it needs to notify the status. Chunked operations that
// constructs a logically single operation from multiple physical operations
......@@ -130,9 +130,6 @@ class UrlFetchOperationBase : public GDataOperationInterface,
// Overridden from URLFetcherDelegate.
virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
// Invoked when ProcessURLFetchResults() is completed.
virtual void OnProcessURLFetchResultsComplete(bool result) OVERRIDE;
// Overridden from GDataOperationInterface.
virtual void OnAuthFailed(GDataErrorCode code) OVERRIDE;
......@@ -166,7 +163,7 @@ class EntryActionOperation : public UrlFetchOperationBase {
protected:
// Overridden from UrlFetchOperationBase.
virtual void ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE;
virtual bool ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE;
virtual void RunCallbackOnPrematureFailure(GDataErrorCode code) OVERRIDE;
const GURL& document_url() const { return document_url_; }
......@@ -189,24 +186,15 @@ class GetDataOperation : public UrlFetchOperationBase {
virtual ~GetDataOperation();
// Parse GData JSON response.
virtual void ParseResponse(GDataErrorCode fetch_error_code,
const std::string& data);
virtual base::Value* ParseResponse(const std::string& data);
protected:
// Overridden from UrlFetchOperationBase.
virtual void ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE;
virtual void RunCallbackOnPrematureFailure(
GDataErrorCode fetch_error_code) OVERRIDE;
void RunCallback(GDataErrorCode fetch_error_code,
scoped_ptr<base::Value> value);
virtual bool ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE;
virtual void RunCallbackOnPrematureFailure(GDataErrorCode code) OVERRIDE;
private:
// Called when ParseJsonOnBlockingPool() is completed.
void OnDataParsed(gdata::GDataErrorCode fetch_error_code,
scoped_ptr<base::Value>* value);
GetDataCallback callback_;
base::WeakPtrFactory<GetDataOperation> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(GetDataOperation);
};
......
chrome/chrome_tests.gypi
View file @ c95ae9dc
......@@ -1104,7 +1104,6 @@
'browser/chromeos/gdata/gdata_file_system_unittest.cc',
'browser/chromeos/gdata/gdata_files_unittest.cc',
'browser/chromeos/gdata/gdata_operation_registry_unittest.cc',
'browser/chromeos/gdata/gdata_operations_unittest.cc',
'browser/chromeos/gdata/gdata_sync_client_unittest.cc',
'browser/chromeos/gdata/gdata_test_util.cc',
'browser/chromeos/gdata/gdata_test_util.h',
......
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