summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--chrome/browser/google_apis/base_requests.cc39
-rw-r--r--chrome/browser/google_apis/base_requests.h11
-rw-r--r--chrome/browser/google_apis/base_requests_unittest.cc10
-rw-r--r--chrome/browser/google_apis/drive_uploader.cc30
-rw-r--r--chrome/browser/google_apis/request_registry.cc77
-rw-r--r--chrome/browser/google_apis/request_registry.h13
-rw-r--r--chrome/browser/google_apis/request_registry_unittest.cc128
-rw-r--r--chrome/chrome_tests_unit.gypi1
8 files changed, 11 insertions, 298 deletions
diff --git a/chrome/browser/google_apis/base_requests.cc b/chrome/browser/google_apis/base_requests.cc
index 12d419e..0f3dfce 100644
--- a/chrome/browser/google_apis/base_requests.cc
+++ b/chrome/browser/google_apis/base_requests.cc
@@ -205,7 +205,7 @@ void UrlFetchRequestBase::Start(const std::string& access_token,
}
// Register to request registry.
- NotifyStartToRequestRegistry();
+ NotifyStart();
url_fetcher_->Start();
started_ = true;
@@ -252,10 +252,7 @@ GDataErrorCode UrlFetchRequestBase::GetErrorCode(const URLFetcher* source) {
}
void UrlFetchRequestBase::OnProcessURLFetchResultsComplete(bool result) {
- if (result)
- NotifySuccessToRequestRegistry();
- else
- NotifyFinish(REQUEST_FAILED);
+ NotifyFinish(result ? REQUEST_COMPLETED : REQUEST_FAILED);
}
void UrlFetchRequestBase::OnURLFetchComplete(const URLFetcher* source) {
@@ -279,14 +276,6 @@ void UrlFetchRequestBase::OnURLFetchComplete(const URLFetcher* source) {
ProcessURLFetchResults(source);
}
-void UrlFetchRequestBase::NotifySuccessToRequestRegistry() {
- NotifyFinish(REQUEST_COMPLETED);
-}
-
-void UrlFetchRequestBase::NotifyStartToRequestRegistry() {
- NotifyStart();
-}
-
void UrlFetchRequestBase::OnAuthFailed(GDataErrorCode code) {
RunCallbackOnPrematureFailure(code);
@@ -449,10 +438,6 @@ void InitiateUploadRequestBase::ProcessURLFetchResults(
OnProcessURLFetchResultsComplete(code == HTTP_SUCCESS);
}
-void InitiateUploadRequestBase::NotifySuccessToRequestRegistry() {
- NotifySuspend();
-}
-
void InitiateUploadRequestBase::RunCallbackOnPrematureFailure(
GDataErrorCode code) {
callback_.Run(code, GURL());
@@ -498,7 +483,6 @@ UploadRangeRequestBase::UploadRangeRequestBase(
drive_file_path),
drive_file_path_(drive_file_path),
upload_url_(upload_url),
- last_chunk_completed_(false),
weak_ptr_factory_(this) {
}
@@ -575,20 +559,9 @@ void UploadRangeRequestBase::OnDataParsed(GDataErrorCode code,
scoped_ptr<base::Value> value) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- // For a new file, HTTP_CREATED is returned.
- // For an existing file, HTTP_SUCCESS is returned.
- if (code == HTTP_CREATED || code == HTTP_SUCCESS)
- last_chunk_completed_ = true;
-
OnRangeRequestComplete(UploadRangeResponse(code, -1, -1), value.Pass());
- OnProcessURLFetchResultsComplete(last_chunk_completed_);
-}
-
-void UploadRangeRequestBase::NotifySuccessToRequestRegistry() {
- if (last_chunk_completed_)
- NotifyFinish(REQUEST_COMPLETED);
- else
- NotifySuspend();
+ OnProcessURLFetchResultsComplete(
+ code == HTTP_CREATED || code == HTTP_SUCCESS);
}
void UploadRangeRequestBase::RunCallbackOnPrematureFailure(
@@ -667,10 +640,6 @@ bool ResumeUploadRequestBase::GetContentFile(
return true;
}
-void ResumeUploadRequestBase::NotifyStartToRequestRegistry() {
- NotifyResume();
-}
-
//======================== GetUploadStatusRequestBase ========================
GetUploadStatusRequestBase::GetUploadStatusRequestBase(
diff --git a/chrome/browser/google_apis/base_requests.h b/chrome/browser/google_apis/base_requests.h
index 7a87760..f187b15 100644
--- a/chrome/browser/google_apis/base_requests.h
+++ b/chrome/browser/google_apis/base_requests.h
@@ -142,12 +142,6 @@ class UrlFetchRequestBase : public AuthenticatedRequestInterface,
// authentication error. Must be implemented by a derived class.
virtual void ProcessURLFetchResults(const net::URLFetcher* source) = 0;
- // Invoked when it needs to notify the status. Chunked requests that
- // constructs a logically single request from multiple physical requests
- // should notify resume/suspend instead of start/finish.
- virtual void NotifyStartToRequestRegistry();
- virtual void NotifySuccessToRequestRegistry();
-
// Invoked by this base class upon an authentication error or cancel by
// a user request. Must be implemented by a derived class.
virtual void RunCallbackOnPrematureFailure(GDataErrorCode code) = 0;
@@ -308,7 +302,6 @@ class InitiateUploadRequestBase : public UrlFetchRequestBase {
// UrlFetchRequestBase overrides.
virtual void ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE;
- virtual void NotifySuccessToRequestRegistry() OVERRIDE;
virtual void RunCallbackOnPrematureFailure(GDataErrorCode code) OVERRIDE;
virtual std::vector<std::string> GetExtraRequestHeaders() const OVERRIDE;
@@ -362,7 +355,6 @@ class UploadRangeRequestBase : public UrlFetchRequestBase {
virtual GURL GetURL() const OVERRIDE;
virtual net::URLFetcher::RequestType GetRequestType() const OVERRIDE;
virtual void ProcessURLFetchResults(const net::URLFetcher* source) OVERRIDE;
- virtual void NotifySuccessToRequestRegistry() OVERRIDE;
virtual void RunCallbackOnPrematureFailure(GDataErrorCode code) OVERRIDE;
// This method will be called when the request is done, regardless of
@@ -391,8 +383,6 @@ class UploadRangeRequestBase : public UrlFetchRequestBase {
const base::FilePath drive_file_path_;
const GURL upload_url_;
- bool last_chunk_completed_;
-
// Note: This should remain the last member so it'll be destroyed and
// invalidate its weak pointers before any other members are destroyed.
base::WeakPtrFactory<UploadRangeRequestBase> weak_ptr_factory_;
@@ -437,7 +427,6 @@ class ResumeUploadRequestBase : public UploadRangeRequestBase {
int64* range_offset,
int64* range_length,
std::string* upload_content_type) OVERRIDE;
- virtual void NotifyStartToRequestRegistry() OVERRIDE;
private:
// The parameters for the request. See ResumeUploadParams for the details.
diff --git a/chrome/browser/google_apis/base_requests_unittest.cc b/chrome/browser/google_apis/base_requests_unittest.cc
index aaca989..a0a3f36 100644
--- a/chrome/browser/google_apis/base_requests_unittest.cc
+++ b/chrome/browser/google_apis/base_requests_unittest.cc
@@ -30,9 +30,7 @@ class FakeGetDataRequest : public GetDataRequest {
virtual ~FakeGetDataRequest() {
}
- void NotifyStart() {
- NotifyStartToRequestRegistry();
- }
+ using RequestRegistry::Request::NotifyStart;
protected:
virtual GURL GetURL() const OVERRIDE {
@@ -64,9 +62,9 @@ class BaseRequestsTest : public testing::Test {
virtual void SetUp() OVERRIDE {
profile_.reset(new TestingProfile);
runner_.reset(new RequestSender(profile_.get(),
- NULL /* url_request_context_getter */,
- std::vector<std::string>() /* scopes */,
- std::string() /* custom user agent */));
+ NULL /* url_request_context_getter */,
+ std::vector<std::string>() /* scopes */,
+ std::string() /* custom user agent */));
runner_->Initialize();
LOG(ERROR) << "Initialized.";
}
diff --git a/chrome/browser/google_apis/drive_uploader.cc b/chrome/browser/google_apis/drive_uploader.cc
index 5716798..26c3969 100644
--- a/chrome/browser/google_apis/drive_uploader.cc
+++ b/chrome/browser/google_apis/drive_uploader.cc
@@ -9,7 +9,6 @@
#include "base/bind.h"
#include "base/callback.h"
#include "base/file_util.h"
-#include "base/message_loop.h"
#include "base/strings/string_number_conversions.h"
#include "base/task_runner_util.h"
#include "base/threading/sequenced_worker_pool.h"
@@ -17,8 +16,6 @@
#include "chrome/browser/google_apis/gdata_wapi_parser.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/power_save_blocker.h"
-#include "net/base/file_stream.h"
-#include "net/base/net_errors.h"
using content::BrowserThread;
@@ -289,17 +286,7 @@ void DriveUploader::OnUploadLocationReceived(
}
upload_file_info->upload_location = upload_location;
-
- // Start the upload from the beginning of the file.
- // PostTask is necessary because we have to finish
- // InitiateUpload's callback before calling ResumeUpload, due to the
- // implementation of RequestRegistry. (http://crbug.com/134814)
- base::MessageLoop::current()->PostTask(
- FROM_HERE,
- base::Bind(&DriveUploader::UploadNextChunk,
- weak_ptr_factory_.GetWeakPtr(),
- base::Passed(&upload_file_info),
- 0)); // Upload from the beginning of the file.
+ UploadNextChunk(upload_file_info.Pass(), 0); // start_position
}
void DriveUploader::StartGetUploadStatus(
@@ -325,9 +312,6 @@ void DriveUploader::UploadNextChunk(
DCHECK_GE(start_position, 0);
DCHECK_LE(start_position, upload_file_info->content_length);
- // TODO(kinaba) this will become not needed if the redundant workaround
- // base::MessageLoop::current()->PostTask for calling UploadNextChunk is
- // removed.
if (upload_file_info->cancelled) {
UploadFailed(upload_file_info.Pass(), GDATA_CANCELLED);
return;
@@ -402,17 +386,7 @@ void DriveUploader::OnUploadRangeResponseReceived(
<< "-" << response.end_position_received
<< " for [" << upload_file_info->drive_path.value() << "]";
- // Continue uploading the remaining chunk. The start position of the
- // remaining data is |response.end_position_received|.
- // PostTask is necessary because we have to finish previous ResumeUpload's
- // callback before calling ResumeUpload again, due to the implementation of
- // RequestRegistry. (http://crbug.com/134814)
- base::MessageLoop::current()->PostTask(
- FROM_HERE,
- base::Bind(&DriveUploader::UploadNextChunk,
- weak_ptr_factory_.GetWeakPtr(),
- base::Passed(&upload_file_info),
- response.end_position_received));
+ UploadNextChunk(upload_file_info.Pass(), response.end_position_received);
}
void DriveUploader::OnUploadProgress(const ProgressCallback& callback,
diff --git a/chrome/browser/google_apis/request_registry.cc b/chrome/browser/google_apis/request_registry.cc
index f2f773c..e7b8f74 100644
--- a/chrome/browser/google_apis/request_registry.cc
+++ b/chrome/browser/google_apis/request_registry.cc
@@ -29,7 +29,6 @@ RequestRegistry::Request::Request(RequestRegistry* registry,
RequestRegistry::Request::~Request() {
DCHECK(progress_status_.transfer_state == REQUEST_COMPLETED ||
- progress_status_.transfer_state == REQUEST_SUSPENDED ||
progress_status_.transfer_state == REQUEST_FAILED);
}
@@ -56,21 +55,6 @@ void RequestRegistry::Request::NotifyFinish(
registry_->OnRequestFinish(progress_status().request_id);
}
-void RequestRegistry::Request::NotifySuspend() {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- DCHECK(progress_status_.transfer_state >= REQUEST_STARTED);
- progress_status_.transfer_state = REQUEST_SUSPENDED;
- registry_->OnRequestSuspend(progress_status().request_id);
-}
-
-void RequestRegistry::Request::NotifyResume() {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- if (progress_status_.transfer_state == REQUEST_NOT_STARTED) {
- progress_status_.transfer_state = REQUEST_IN_PROGRESS;
- registry_->OnRequestResume(this, &progress_status_);
- }
-}
-
RequestRegistry::RequestRegistry() {
in_flight_requests_.set_check_on_null_data(true);
}
@@ -109,16 +93,7 @@ bool RequestRegistry::CancelForFilePath(const base::FilePath& file_path) {
}
void RequestRegistry::CancelRequest(Request* request) {
- if (request->progress_status().transfer_state == REQUEST_SUSPENDED) {
- // SUSPENDED request already completed its job (like calling back to
- // its client code). Invoking request->Cancel() again on it is a kind of
- // 'double deletion'. So here we directly call OnRequestFinish and just
- // unregister the request from the registry.
- // TODO(kinaba): http://crbug.com/164098 Get rid of the hack.
- OnRequestFinish(request->progress_status().request_id);
- } else {
- request->Cancel();
- }
+ request->Cancel();
}
void RequestRegistry::OnRequestStart(
@@ -140,54 +115,4 @@ void RequestRegistry::OnRequestFinish(RequestID id) {
in_flight_requests_.Remove(id);
}
-void RequestRegistry::OnRequestResume(
- RequestRegistry::Request* request,
- RequestProgressStatus* new_status) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
- // Find the corresponding suspended task.
- Request* suspended = NULL;
- for (RequestIDMap::iterator iter(&in_flight_requests_);
- !iter.IsAtEnd();
- iter.Advance()) {
- Request* in_flight_request = iter.GetCurrentValue();
- const RequestProgressStatus& status =
- in_flight_request->progress_status();
- if (status.transfer_state == REQUEST_SUSPENDED &&
- status.file_path == request->progress_status().file_path) {
- suspended = in_flight_request;
- break;
- }
- }
-
- if (!suspended) {
- // Preceding suspended requests was not found. Assume it was canceled.
- //
- // request->Cancel() needs to be called to properly shut down the
- // current request, but request->Cancel() tries to unregister itself
- // from the registry. So, as a hack, temporarily assign it an ID.
- // TODO(kinaba): http://crbug.com/164098 Get rid of it.
- new_status->request_id = in_flight_requests_.Add(request);
- CancelRequest(request);
- return;
- }
-
- // Remove the old one and initiate the new request.
- const RequestProgressStatus& old_status = suspended->progress_status();
- RequestID old_id = old_status.request_id;
- in_flight_requests_.Remove(old_id);
- new_status->request_id = in_flight_requests_.Add(request);
- DVLOG(1) << "Request[" << old_id << " -> " <<
- new_status->request_id << "] resumed.";
-}
-
-void RequestRegistry::OnRequestSuspend(RequestID id) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
- Request* request = in_flight_requests_.Lookup(id);
- DCHECK(request);
-
- DVLOG(1) << "Request[" << id << "] suspended.";
-}
-
} // namespace google_apis
diff --git a/chrome/browser/google_apis/request_registry.h b/chrome/browser/google_apis/request_registry.h
index ff1c678..571fbfc 100644
--- a/chrome/browser/google_apis/request_registry.h
+++ b/chrome/browser/google_apis/request_registry.h
@@ -22,7 +22,6 @@ enum RequestTransferState {
REQUEST_IN_PROGRESS,
REQUEST_COMPLETED,
REQUEST_FAILED,
- REQUEST_SUSPENDED,
};
// Returns string representations of the request state.
@@ -70,15 +69,6 @@ class RequestRegistry {
// Notifies the registry about current status.
void NotifyStart();
void NotifyFinish(RequestTransferState status);
- // Notifies suspend/resume, used for chunked upload requests.
- // The initial upload request should issue "start" "progress"* "suspend".
- // The subsequent requests will call "resume" "progress"* "suspend",
- // and the last one will do "resume" "progress"* "finish".
- // In other words, "suspend" is similar to "finish" except it lasts to live
- // until the next "resume" comes. "Resume" is similar to "start", except
- // that it removes the existing "suspend" request.
- void NotifySuspend();
- void NotifyResume();
private:
// Does the cancellation.
@@ -105,9 +95,6 @@ class RequestRegistry {
// the request to the registry. A fresh request ID is returned to *id.
void OnRequestStart(Request* request, RequestID* id);
void OnRequestFinish(RequestID request_id);
- void OnRequestSuspend(RequestID request_id);
- void OnRequestResume(Request* request,
- RequestProgressStatus* new_status);
typedef IDMap<Request, IDMapOwnPointer> RequestIDMap;
RequestIDMap in_flight_requests_;
diff --git a/chrome/browser/google_apis/request_registry_unittest.cc b/chrome/browser/google_apis/request_registry_unittest.cc
deleted file mode 100644
index d2f7b34..0000000
--- a/chrome/browser/google_apis/request_registry_unittest.cc
+++ /dev/null
@@ -1,128 +0,0 @@
-// 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/google_apis/request_registry.h"
-
-#include "base/compiler_specific.h"
-#include "base/memory/weak_ptr.h"
-#include "chrome/browser/google_apis/request_registry.h"
-#include "content/public/test/test_browser_thread_bundle.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace google_apis {
-
-namespace {
-
-class MockRequest : public RequestRegistry::Request,
- public base::SupportsWeakPtr<MockRequest> {
- public:
- explicit MockRequest(RequestRegistry* registry)
- : RequestRegistry::Request(
- registry,
- base::FilePath(FILE_PATH_LITERAL("/dummy/download"))) {
- }
-
- MOCK_METHOD0(DoCancel, void());
-
- // Make them public so that they can be called from test cases.
- using RequestRegistry::Request::NotifyStart;
- using RequestRegistry::Request::NotifyFinish;
- using RequestRegistry::Request::NotifySuspend;
- using RequestRegistry::Request::NotifyResume;
-};
-
-} // namespace
-
-class RequestRegistryTest : public testing::Test {
- protected:
- content::TestBrowserThreadBundle thread_bundle_;
-};
-
-TEST_F(RequestRegistryTest, OneSuccess) {
- RequestRegistry registry;
-
- base::WeakPtr<MockRequest> op1 =
- (new MockRequest(&registry))->AsWeakPtr();
- EXPECT_CALL(*op1, DoCancel()).Times(0);
-
- op1->NotifyStart();
- op1->NotifyFinish(REQUEST_COMPLETED);
- EXPECT_EQ(NULL, op1.get()); // deleted
-}
-
-TEST_F(RequestRegistryTest, OneCancel) {
- RequestRegistry registry;
-
- base::WeakPtr<MockRequest> op1 =
- (new MockRequest(&registry))->AsWeakPtr();
- EXPECT_CALL(*op1, DoCancel());
-
- op1->NotifyStart();
- registry.CancelAll();
- EXPECT_EQ(NULL, op1.get()); // deleted
-}
-
-TEST_F(RequestRegistryTest, TwoSuccess) {
- RequestRegistry registry;
-
- base::WeakPtr<MockRequest> op1 =
- (new MockRequest(&registry))->AsWeakPtr();
- base::WeakPtr<MockRequest> op2 =
- (new MockRequest(&registry))->AsWeakPtr();
- EXPECT_CALL(*op1, DoCancel()).Times(0);
- EXPECT_CALL(*op2, DoCancel()).Times(0);
-
- op1->NotifyStart();
- op2->NotifyStart();
- op1->NotifyFinish(REQUEST_COMPLETED);
- op2->NotifyFinish(REQUEST_COMPLETED);
- EXPECT_EQ(NULL, op1.get()); // deleted
- EXPECT_EQ(NULL, op2.get()); // deleted
-}
-
-TEST_F(RequestRegistryTest, RestartRequest) {
- RequestRegistry registry;
-
- base::WeakPtr<MockRequest> op1 =
- (new MockRequest(&registry))->AsWeakPtr();
- EXPECT_CALL(*op1, DoCancel()).Times(0);
-
- op1->NotifyStart();
- op1->NotifyStart(); // restart
- op1->NotifyFinish(REQUEST_COMPLETED);
- EXPECT_EQ(NULL, op1.get()); // deleted
-}
-
-TEST_F(RequestRegistryTest, SuspendCancel) {
- RequestRegistry registry;
-
- // Suspend-then-resume is a hack in RequestRegistry to tie physically
- // split but logically single request (= chunked uploading split into
- // multiple HTTP requests). When the "logically-single" request is
- // canceled between the two physical requests,
- // |----op1----| CANCEL! |----op2----|
- // the cancellation is notified to the callback function associated with
- // op2, not op1. This is because, op1's callback is already invoked at this
- // point to notify the completion of the physical request. Completion
- // callback must not be called more than once.
-
- base::WeakPtr<MockRequest> op1 =
- (new MockRequest(&registry))->AsWeakPtr();
- EXPECT_CALL(*op1, DoCancel()).Times(0);
-
- op1->NotifyStart();
- op1->NotifySuspend();
- registry.CancelAll();
- EXPECT_EQ(NULL, op1.get()); // deleted
-
- base::WeakPtr<MockRequest> op2 =
- (new MockRequest(&registry))->AsWeakPtr();
- EXPECT_CALL(*op2, DoCancel()).Times(1);
-
- op2->NotifyResume();
- EXPECT_EQ(NULL, op2.get()); // deleted
-}
-
-} // namespace google_apis
diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi
index 4bf8ce8..063b719 100644
--- a/chrome/chrome_tests_unit.gypi
+++ b/chrome/chrome_tests_unit.gypi
@@ -863,7 +863,6 @@
'browser/google_apis/fake_drive_service_unittest.cc',
'browser/google_apis/mock_drive_service.cc',
'browser/google_apis/mock_drive_service.h',
- 'browser/google_apis/request_registry_unittest.cc',
'browser/google_apis/request_util_unittest.cc',
'browser/google_apis/time_util_unittest.cc',
'browser/history/android/android_cache_database_unittest.cc',
generated by cgit v1.1 at 2025-05-29 11:10:59 +0000