PaymentHandler: Implement PaymentInstruments.delete().

Remove the stored payment instrument in associated service worker registration.

Related Spec Link:
  https://w3c.github.io/webpayments-payment-handler/#paymentinstruments-interface

BUG=661608
TEST=payment_manager_unittest.cc, payment-instruments.html

Review-Url: https://codereview.chromium.org/2844463002
Cr-Commit-Position: refs/heads/master@{#467304}

components/payments/mojom/payment_app.mojom

@@ -48,9 +48,11 @@ interface PaymentManager {
       => (PaymentAppManifestError error);
   GetManifest()
       => (PaymentAppManifest payment_app_manifest, PaymentAppManifestError error);
-  SetPaymentInstrument(string instrumentKey, PaymentInstrument instrument)
+  DeletePaymentInstrument(string instrument_key)
       => (PaymentHandlerStatus status);
-  GetPaymentInstrument(string instrumentKey)
+  SetPaymentInstrument(string instrument_key, PaymentInstrument instrument)
+      => (PaymentHandlerStatus status);
+  GetPaymentInstrument(string instrument_key)
       => (PaymentInstrument instrument, PaymentHandlerStatus status);
 };
 

content/browser/payments/payment_app_database.cc

@@ -107,9 +107,23 @@ void PaymentAppDatabase::ReadAllManifests(
                  weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
+void PaymentAppDatabase::DeletePaymentInstrument(
+    const GURL& scope,
+    const std::string& instrument_key,
+    DeletePaymentInstrumentCallback callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  service_worker_context_->FindReadyRegistrationForPattern(
+      scope,
+      base::Bind(
+          &PaymentAppDatabase::DidFindRegistrationToDeletePaymentInstrument,
+          weak_ptr_factory_.GetWeakPtr(), instrument_key,
+          base::Passed(std::move(callback))));
+}
+
 void PaymentAppDatabase::ReadPaymentInstrument(
     const GURL& scope,
-    const std::string& instrumentKey,
+    const std::string& instrument_key,
     ReadPaymentInstrumentCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
@@ -117,13 +131,13 @@ void PaymentAppDatabase::ReadPaymentInstrument(
       scope,
       base::Bind(
           &PaymentAppDatabase::DidFindRegistrationToReadPaymentInstrument,
-          weak_ptr_factory_.GetWeakPtr(), instrumentKey,
+          weak_ptr_factory_.GetWeakPtr(), instrument_key,
           base::Passed(std::move(callback))));
 }
 
 void PaymentAppDatabase::WritePaymentInstrument(
     const GURL& scope,
-    const std::string& instrumentKey,
+    const std::string& instrument_key,
     PaymentInstrumentPtr instrument,
     WritePaymentInstrumentCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -132,7 +146,7 @@ void PaymentAppDatabase::WritePaymentInstrument(
       scope,
       base::Bind(
           &PaymentAppDatabase::DidFindRegistrationToWritePaymentInstrument,
-          weak_ptr_factory_.GetWeakPtr(), instrumentKey,
+          weak_ptr_factory_.GetWeakPtr(), instrument_key,
           base::Passed(std::move(instrument)),
           base::Passed(std::move(callback))));
 }
@@ -249,8 +263,54 @@ void PaymentAppDatabase::DidReadAllManifests(
   callback.Run(std::move(manifests));
 }
 
+void PaymentAppDatabase::DidFindRegistrationToDeletePaymentInstrument(
+    const std::string& instrument_key,
+    DeletePaymentInstrumentCallback callback,
+    ServiceWorkerStatusCode status,
+    scoped_refptr<ServiceWorkerRegistration> registration) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (status != SERVICE_WORKER_OK) {
+    std::move(callback).Run(PaymentHandlerStatus::NO_ACTIVE_WORKER);
+    return;
+  }
+
+  service_worker_context_->GetRegistrationUserData(
+      registration->id(), {instrument_key},
+      base::Bind(&PaymentAppDatabase::DidFindPaymentInstrument,
+                 weak_ptr_factory_.GetWeakPtr(), registration->id(),
+                 instrument_key, base::Passed(std::move(callback))));
+}
+
+void PaymentAppDatabase::DidFindPaymentInstrument(
+    int64_t registration_id,
+    const std::string& instrument_key,
+    DeletePaymentInstrumentCallback callback,
+    const std::vector<std::string>& data,
+    ServiceWorkerStatusCode status) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (status != SERVICE_WORKER_OK || data.size() != 1) {
+    std::move(callback).Run(PaymentHandlerStatus::NOT_FOUND);
+    return;
+  }
+
+  service_worker_context_->ClearRegistrationUserData(
+      registration_id, {instrument_key},
+      base::Bind(&PaymentAppDatabase::DidDeletePaymentInstrument,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 base::Passed(std::move(callback))));
+}
+
+void PaymentAppDatabase::DidDeletePaymentInstrument(
+    DeletePaymentInstrumentCallback callback,
+    ServiceWorkerStatusCode status) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  return std::move(callback).Run(status == SERVICE_WORKER_OK
+                                     ? PaymentHandlerStatus::SUCCESS
+                                     : PaymentHandlerStatus::NOT_FOUND);
+}
+
 void PaymentAppDatabase::DidFindRegistrationToReadPaymentInstrument(
-    const std::string& instrumentKey,
+    const std::string& instrument_key,
     ReadPaymentInstrumentCallback callback,
     ServiceWorkerStatusCode status,
     scoped_refptr<ServiceWorkerRegistration> registration) {
@@ -262,7 +322,7 @@ void PaymentAppDatabase::DidFindRegistrationToReadPaymentInstrument(
   }
 
   service_worker_context_->GetRegistrationUserData(
-      registration->id(), {instrumentKey},
+      registration->id(), {instrument_key},
       base::Bind(&PaymentAppDatabase::DidReadPaymentInstrument,
                  weak_ptr_factory_.GetWeakPtr(),
                  base::Passed(std::move(callback))));
@@ -290,7 +350,7 @@ void PaymentAppDatabase::DidReadPaymentInstrument(
 }
 
 void PaymentAppDatabase::DidFindRegistrationToWritePaymentInstrument(
-    const std::string& instrumentKey,
+    const std::string& instrument_key,
     PaymentInstrumentPtr instrument,
     WritePaymentInstrumentCallback callback,
     ServiceWorkerStatusCode status,
@@ -315,7 +375,7 @@ void PaymentAppDatabase::DidFindRegistrationToWritePaymentInstrument(
 
   service_worker_context_->StoreRegistrationUserData(
       registration->id(), registration->pattern().GetOrigin(),
-      {{instrumentKey, serialized}},
+      {{instrument_key, serialized}},
       base::Bind(&PaymentAppDatabase::DidWritePaymentInstrument,
                  weak_ptr_factory_.GetWeakPtr(),
                  base::Passed(std::move(callback))));

content/browser/payments/payment_app_database.h

@@ -32,6 +32,8 @@ class CONTENT_EXPORT PaymentAppDatabase {
       std::pair<int64_t, payments::mojom::PaymentAppManifestPtr>;
   using Manifests = std::vector<ManifestWithID>;
   using ReadAllManifestsCallback = base::Callback<void(Manifests)>;
+  using DeletePaymentInstrumentCallback =
+      base::OnceCallback<void(payments::mojom::PaymentHandlerStatus)>;
   using ReadPaymentInstrumentCallback =
       base::OnceCallback<void(payments::mojom::PaymentInstrumentPtr,
                               payments::mojom::PaymentHandlerStatus)>;
@@ -47,11 +49,14 @@ class CONTENT_EXPORT PaymentAppDatabase {
                      const WriteManifestCallback& callback);
   void ReadManifest(const GURL& scope, const ReadManifestCallback& callback);
   void ReadAllManifests(const ReadAllManifestsCallback& callback);
+  void DeletePaymentInstrument(const GURL& scope,
+                               const std::string& instrument_key,
+                               DeletePaymentInstrumentCallback callback);
   void ReadPaymentInstrument(const GURL& scope,
-                             const std::string& instrumentKey,
+                             const std::string& instrument_key,
                              ReadPaymentInstrumentCallback callback);
   void WritePaymentInstrument(const GURL& scope,
-                              const std::string& instrumentKey,
+                              const std::string& instrument_key,
                               payments::mojom::PaymentInstrumentPtr instrument,
                               WritePaymentInstrumentCallback callback);
 
@@ -80,9 +85,23 @@ class CONTENT_EXPORT PaymentAppDatabase {
       const std::vector<std::pair<int64_t, std::string>>& raw_data,
       ServiceWorkerStatusCode status);
 
+  // DeletePaymentInstrument callbacks
+  void DidFindRegistrationToDeletePaymentInstrument(
+      const std::string& instrument_key,
+      DeletePaymentInstrumentCallback callback,
+      ServiceWorkerStatusCode status,
+      scoped_refptr<ServiceWorkerRegistration> registration);
+  void DidFindPaymentInstrument(int64_t registration_id,
+                                const std::string& instrument_key,
+                                DeletePaymentInstrumentCallback callback,
+                                const std::vector<std::string>& data,
+                                ServiceWorkerStatusCode status);
+  void DidDeletePaymentInstrument(DeletePaymentInstrumentCallback callback,
+                                  ServiceWorkerStatusCode status);
+
   // ReadPaymentInstrument callbacks
   void DidFindRegistrationToReadPaymentInstrument(
-      const std::string& instrumentKey,
+      const std::string& instrument_key,
       ReadPaymentInstrumentCallback callback,
       ServiceWorkerStatusCode status,
       scoped_refptr<ServiceWorkerRegistration> registration);
@@ -92,7 +111,7 @@ class CONTENT_EXPORT PaymentAppDatabase {
 
   // WritePaymentInstrument callbacks
   void DidFindRegistrationToWritePaymentInstrument(
-      const std::string& instrumentKey,
+      const std::string& instrument_key,
       payments::mojom::PaymentInstrumentPtr instrument,
       WritePaymentInstrumentCallback callback,
       ServiceWorkerStatusCode status,

content/browser/payments/payment_manager.cc

@@ -57,23 +57,32 @@ void PaymentManager::GetManifest(const GetManifestCallback& callback) {
   payment_app_context_->payment_app_database()->ReadManifest(scope_, callback);
 }
 
+void PaymentManager::DeletePaymentInstrument(
+    const std::string& instrument_key,
+    const PaymentManager::DeletePaymentInstrumentCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  payment_app_context_->payment_app_database()->DeletePaymentInstrument(
+      scope_, instrument_key, callback);
+}
+
 void PaymentManager::SetPaymentInstrument(
-    const std::string& instrumentKey,
+    const std::string& instrument_key,
     payments::mojom::PaymentInstrumentPtr details,
     const PaymentManager::SetPaymentInstrumentCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   payment_app_context_->payment_app_database()->WritePaymentInstrument(
-      scope_, instrumentKey, std::move(details), callback);
+      scope_, instrument_key, std::move(details), callback);
 }
 
 void PaymentManager::GetPaymentInstrument(
-    const std::string& instrumentKey,
+    const std::string& instrument_key,
     const PaymentManager::GetPaymentInstrumentCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   payment_app_context_->payment_app_database()->ReadPaymentInstrument(
-      scope_, instrumentKey, callback);
+      scope_, instrument_key, callback);
 }
 
 void PaymentManager::OnConnectionError() {

content/browser/payments/payment_manager.h

@@ -36,12 +36,15 @@ class CONTENT_EXPORT PaymentManager
   void SetManifest(payments::mojom::PaymentAppManifestPtr manifest,
                    const SetManifestCallback& callback) override;
   void GetManifest(const GetManifestCallback& callback) override;
+  void DeletePaymentInstrument(
+      const std::string& instrument_key,
+      const DeletePaymentInstrumentCallback& callback) override;
   void SetPaymentInstrument(
-      const std::string& instrumentKey,
+      const std::string& instrument_key,
       payments::mojom::PaymentInstrumentPtr details,
       const SetPaymentInstrumentCallback& callback) override;
   void GetPaymentInstrument(
-      const std::string& instrumentKey,
+      const std::string& instrument_key,
       const GetPaymentInstrumentCallback& callback) override;
 
   // Called when an error is detected on binding_.

content/browser/payments/payment_manager_unittest.cc

@@ -40,6 +40,11 @@ void GetManifestCallback(bool* called,
   *out_error = error;
 }
 
+void DeletePaymentInstrumentCallback(PaymentHandlerStatus* out_status,
+                                     PaymentHandlerStatus status) {
+  *out_status = status;
+}
+
 void SetPaymentInstrumentCallback(PaymentHandlerStatus* out_status,
                                   PaymentHandlerStatus status) {
   *out_status = status;
@@ -65,20 +70,28 @@ class PaymentManagerTest : public PaymentAppContentUnitTestBase {
 
   PaymentManager* payment_manager() const { return manager_; }
 
-  void SetPaymentInstrument(const std::string& instrumentKey,
+  void DeletePaymentInstrument(const std::string& instrument_key,
+                               PaymentHandlerStatus* out_status) {
+    manager_->DeletePaymentInstrument(
+        instrument_key,
+        base::Bind(&DeletePaymentInstrumentCallback, out_status));
+    base::RunLoop().RunUntilIdle();
+  }
+
+  void SetPaymentInstrument(const std::string& instrument_key,
                             PaymentInstrumentPtr instrument,
                             PaymentHandlerStatus* out_status) {
     manager_->SetPaymentInstrument(
-        instrumentKey, std::move(instrument),
+        instrument_key, std::move(instrument),
         base::Bind(&SetPaymentInstrumentCallback, out_status));
     base::RunLoop().RunUntilIdle();
   }
 
-  void GetPaymentInstrument(const std::string& instrumentKey,
+  void GetPaymentInstrument(const std::string& instrument_key,
                             PaymentInstrumentPtr* out_instrument,
                             PaymentHandlerStatus* out_status) {
     manager_->GetPaymentInstrument(
-        instrumentKey,
+        instrument_key,
         base::Bind(&GetPaymentInstrumentCallback, out_instrument, out_status));
     base::RunLoop().RunUntilIdle();
   }
@@ -185,4 +198,29 @@ TEST_F(PaymentManagerTest, GetUnstoredPaymentInstrument) {
   ASSERT_EQ(PaymentHandlerStatus::NOT_FOUND, read_status);
 }
 
+TEST_F(PaymentManagerTest, DeletePaymentInstrument) {
+  PaymentHandlerStatus write_status = PaymentHandlerStatus::NOT_FOUND;
+  PaymentInstrumentPtr write_details = PaymentInstrument::New();
+  write_details->name = "Visa ending ****4756",
+  write_details->enabled_methods.push_back("visa");
+  write_details->stringified_capabilities = "{}";
+  ASSERT_EQ(PaymentHandlerStatus::NOT_FOUND, write_status);
+  SetPaymentInstrument("test_key", std::move(write_details), &write_status);
+  ASSERT_EQ(PaymentHandlerStatus::SUCCESS, write_status);
+
+  PaymentHandlerStatus read_status = PaymentHandlerStatus::NOT_FOUND;
+  PaymentInstrumentPtr read_details;
+  ASSERT_EQ(PaymentHandlerStatus::NOT_FOUND, read_status);
+  GetPaymentInstrument("test_key", &read_details, &read_status);
+  ASSERT_EQ(PaymentHandlerStatus::SUCCESS, read_status);
+
+  PaymentHandlerStatus delete_status = PaymentHandlerStatus::NOT_FOUND;
+  DeletePaymentInstrument("test_key", &delete_status);
+  ASSERT_EQ(PaymentHandlerStatus::SUCCESS, delete_status);
+
+  read_status = PaymentHandlerStatus::NOT_FOUND;
+  GetPaymentInstrument("test_key", &read_details, &read_status);
+  ASSERT_EQ(PaymentHandlerStatus::NOT_FOUND, read_status);
+}
+
 }  // namespace content

third_party/WebKit/LayoutTests/http/tests/payments/payment-instruments.html

@@ -45,4 +45,41 @@ promise_test(test => {
       .catch(unreached_rejection(test));
   }, 'PaymentInstruments set/get methods test');
 
+promise_test(test => {
+    var registration;
+    var script_url = 'resources/empty-worker.js';
+    var scope = 'resources/';
+
+    return service_worker_unregister_and_register(test, script_url, scope)
+      .then(r => {
+          registration = r;
+          return wait_for_state(test, registration.installing, 'activated');
+        })
+      .then(state => {
+          assert_equals(state, 'activated');
+          return registration.paymentManager.instruments.set(
+              'test_key',
+              {
+                name: 'Visa ending ****4756',
+                enabledMethods: ['basic-card'],
+                capabilities: {
+                  supportedNetworks: ['visa'],
+                  supportedTypes: ['credit']
+                }
+              });
+        })
+      .then(result => {
+          assert_equals(result, undefined);
+          return registration.paymentManager.instruments.delete('test_key');
+        })
+      .then(result => {
+          assert_equals(result, true);
+          return registration.paymentManager.instruments.delete('test_key');
+        })
+      .then(result => {
+          assert_equals(result, false);
+        })
+      .catch(unreached_rejection(test));
+  }, 'PaymentInstruments delete method test');
+
 </script>

third_party/WebKit/Source/modules/payments/PaymentInstruments.cpp

@@ -53,9 +53,22 @@ PaymentInstruments::PaymentInstruments(
     : manager_(manager) {}
 
 ScriptPromise PaymentInstruments::deleteInstrument(
+    ScriptState* script_state,
     const String& instrument_key) {
-  NOTIMPLEMENTED();
-  return ScriptPromise();
+  if (!manager_.is_bound()) {
+    return ScriptPromise::RejectWithDOMException(
+        script_state,
+        DOMException::Create(kInvalidStateError, kPaymentManagerUnavailable));
+  }
+
+  ScriptPromiseResolver* resolver = ScriptPromiseResolver::Create(script_state);
+  ScriptPromise promise = resolver->Promise();
+
+  manager_->DeletePaymentInstrument(
+      instrument_key, ConvertToBaseCallback(WTF::Bind(
+                          &PaymentInstruments::onDeletePaymentInstrument,
+                          WrapPersistent(this), WrapPersistent(resolver))));
+  return promise;
 }
 
 ScriptPromise PaymentInstruments::get(ScriptState* script_state,
@@ -130,6 +143,14 @@ ScriptPromise PaymentInstruments::set(ScriptState* script_state,
 
 DEFINE_TRACE(PaymentInstruments) {}
 
+void PaymentInstruments::onDeletePaymentInstrument(
+    ScriptPromiseResolver* resolver,
+    payments::mojom::blink::PaymentHandlerStatus status) {
+  DCHECK(resolver);
+  resolver->Resolve(status ==
+                    payments::mojom::blink::PaymentHandlerStatus::SUCCESS);
+}
+
 void PaymentInstruments::onGetPaymentInstrument(
     ScriptPromiseResolver* resolver,
     payments::mojom::blink::PaymentInstrumentPtr stored_instrument,

third_party/WebKit/Source/modules/payments/PaymentInstruments.h

@@ -29,7 +29,7 @@ class MODULES_EXPORT PaymentInstruments final
  public:
   explicit PaymentInstruments(const payments::mojom::blink::PaymentManagerPtr&);
 
-  ScriptPromise deleteInstrument(const String& instrument_key);
+  ScriptPromise deleteInstrument(ScriptState*, const String& instrument_key);
   ScriptPromise get(ScriptState*, const String& instrument_key);
   ScriptPromise keys();
   ScriptPromise has(const String& instrument_key);
@@ -41,11 +41,13 @@ class MODULES_EXPORT PaymentInstruments final
   DECLARE_TRACE();
 
  private:
-  void onSetPaymentInstrument(ScriptPromiseResolver*,
-                              payments::mojom::blink::PaymentHandlerStatus);
+  void onDeletePaymentInstrument(ScriptPromiseResolver*,
+                                 payments::mojom::blink::PaymentHandlerStatus);
   void onGetPaymentInstrument(ScriptPromiseResolver*,
                               payments::mojom::blink::PaymentInstrumentPtr,
                               payments::mojom::blink::PaymentHandlerStatus);
+  void onSetPaymentInstrument(ScriptPromiseResolver*,
+                              payments::mojom::blink::PaymentHandlerStatus);
 
   const payments::mojom::blink::PaymentManagerPtr& manager_;
 };

third_party/WebKit/Source/modules/payments/PaymentInstruments.idl

@@ -8,7 +8,7 @@
     RuntimeEnabled=PaymentApp,
     Exposed=ServiceWorker
 ] interface PaymentInstruments {
-    [ImplementedAs=deleteInstrument] Promise<boolean> delete(DOMString instrumentKey);
+    [CallWith=ScriptState, ImplementedAs=deleteInstrument] Promise<boolean> delete(DOMString instrumentKey);
     [CallWith=ScriptState] Promise<PaymentInstrument> get(DOMString instrumentKey);
     Promise<sequence<DOMString>> keys();
     Promise<boolean> has(DOMString instrumentKey);