Separate WebDataRequest functionality from WebDataService

chrome/browser/webdata/web_data_request_manager.cc

Index: chrome/browser/webdata/web_data_request_manager.cc
diff --git a/chrome/browser/webdata/web_data_request_manager.cc b/chrome/browser/webdata/web_data_request_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..681d99fe14acebd84eb74911542aec4db4b67bd5
--- /dev/null
+++ b/chrome/browser/webdata/web_data_request_manager.cc
@@ -0,0 +1,131 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.

[dhollowa, 2013/01/04 01:34:50]

nit: remove the "(c)" bit.

[Cait (Slow), 2013/01/04 22:59:29]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/webdata/web_data_request_manager.h"
+
+#include "base/bind.h"
+#include "base/message_loop.h"
+#include "base/stl_util.h"
+#include "base/threading/thread.h"

[dhollowa, 2013/01/04 01:34:50]

not used

[Cait (Slow), 2013/01/04 22:59:29]

Done.

+#include "chrome/browser/autofill/autofill_profile.h"
+#include "chrome/browser/autofill/credit_card.h"
+#include "chrome/browser/webdata/web_data_service.h"
+
+WebDataRequestManager::WebDataRequestManager()
+    : next_request_handle_(1) {
+}
+
+void WebDataRequestManager::RegisterRequest(WebDataRequest* request) {
+  base::AutoLock l(pending_lock_);
+  pending_requests_[request->GetHandle()] = request;
+}
+
+int WebDataRequestManager::GetNextRequestHandle() {
+  base::AutoLock l(pending_lock_);
+  return ++next_request_handle_;
+}
+
+void WebDataRequestManager::CancelRequest(WebDataServiceBase::Handle h) {
+  base::AutoLock l(pending_lock_);
+  RequestMap::iterator i = pending_requests_.find(h);
+  if (i == pending_requests_.end()) {
+    NOTREACHED() << "Canceling a nonexistent web data service request";
+    return;
+  }
+  i->second->Cancel();
+}
+
+void WebDataRequestManager::RequestCompleted(WebDataServiceBase::Handle h) {
+  pending_lock_.Acquire();
+  RequestMap::iterator i = pending_requests_.find(h);
+  if (i == pending_requests_.end()) {
+    NOTREACHED() << "Request completed called for an unknown request";
+    pending_lock_.Release();
+    return;
+  }
+
+  // Take ownership of the request object and remove it from the map.
+  scoped_ptr<WebDataRequest> request(i->second);
+  pending_requests_.erase(i);
+  pending_lock_.Release();
+
+  // Notify the consumer if needed.
+  WebDataServiceConsumer* consumer = NULL;
+  if (!request->IsCancelled(&consumer) && consumer) {
+    consumer->OnWebDataServiceRequestDone(request->GetHandle(),
+                                          request->GetResult());
+  // TODO(caitkp): get rid of Autofill DEPS here.

[dhollowa, 2013/01/04 01:34:50]

Yes, this is unfortunate.  A possible solution would be to extend the WDResult
class to take an optional callback to be called when its (new) |Destroy| method
is called.

[Cait (Slow), 2013/01/04 22:59:29]

I added the optional callback and |Destroy| method to WDResult to break this
dependency. It would be nice if we could avoid passing in "dummy" callbacks for
all the other cases though.

On 2013/01/04 01:34:50, dhollowa wrote:

[dhollowa, 2013/01/04 23:57:36]

Agreed.  A second constructor would solve this.

On 2013/01/04 22:59:29, caitkp wrote:

+  } else {
+    // Nobody is taken ownership of the result, either because it is cancelled
+    // or there is no consumer. Destroy results that require special handling.
+    WDTypedResult const *result = request->GetResult();
+    if (result) {
+      if (result->GetType() == AUTOFILL_PROFILES_RESULT) {
+        const WDResult<std::vector<AutofillProfile*> >* r =
+            static_cast<const WDResult<std::vector<AutofillProfile*> >*>(
+                result);
+        std::vector<AutofillProfile*> profiles = r->GetValue();
+        STLDeleteElements(&profiles);
+      } else if (result->GetType() == AUTOFILL_CREDITCARDS_RESULT) {
+        const WDResult<std::vector<CreditCard*> >* r =
+            static_cast<const WDResult<std::vector<CreditCard*> >*>(result);
+
+        std::vector<CreditCard*> credit_cards = r->GetValue();
+        STLDeleteElements(&credit_cards);
+      }
+    }
+  }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+//
+// WebDataRequest implementation.
+//
+////////////////////////////////////////////////////////////////////////////////
+
+WebDataRequest::WebDataRequest(WebDataService* service,
+                               WebDataService::Handle handle,
+                               WebDataServiceConsumer* consumer)
+    : service_(service),
+      handle_(handle),
+      cancelled_(false),
+      consumer_(consumer),
+      result_(NULL) {
+  message_loop_ = MessageLoop::current();
+}
+
+WebDataRequest::~WebDataRequest() {
+  delete result_;
+}
+
+WebDataService::Handle WebDataRequest::GetHandle() const {
+  return handle_;
+}
+
+bool WebDataRequest::IsCancelled(
+    WebDataServiceConsumer** consumer) const {
+  base::AutoLock l(cancel_lock_);
+  if (consumer)
+    *consumer = consumer_;
+  return cancelled_;
+}
+
+void WebDataRequest::Cancel() {
+  base::AutoLock l(cancel_lock_);
+  cancelled_ = true;
+  consumer_ = NULL;
+}
+
+void WebDataRequest::SetResult(WDTypedResult* r) {
+  result_ = r;
+}
+
+const WDTypedResult* WebDataRequest::GetResult() const {
+  return result_;
+}
+
+void WebDataRequest::RequestComplete() {
+  message_loop_->PostTask(FROM_HERE, Bind(&WebDataService::RequestCompleted,
+                                          service_.get(), handle_));
+}