PaymentApp: Introduce PaymentAppDatabase class.

content/browser/payments/payment_app_context_impl.h

 // Copyright 2016 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.
 
 #ifndef CONTENT_BROWSER_PAYMENTS_PAYMENT_APP_CONTEXT_IMPL_H_
 #define CONTENT_BROWSER_PAYMENTS_PAYMENT_APP_CONTEXT_IMPL_H_
 
 #include <map>
 #include <memory>
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "components/payments/payment_app.mojom.h"
+#include "content/browser/payments/payment_app_database.h"

[please use gerrit instead, 2016/12/14 19:44:00]

Forward-declare instead?

[zino, 2016/12/15 17:56:52]

Done.

 #include "content/common/content_export.h"
 #include "content/public/browser/payment_app_context.h"
 
 namespace content {
 
 class PaymentAppManager;
 class ServiceWorkerContextWrapper;
 

[please use gerrit instead, 2016/12/14 19:44:00]

Add a class comment about the expected order of lifetime calls. Is it this?

1) Constructor
2) Init()
3) CreateService()

Can call GetAllManifests() on UI thread.
Can call ServiceHadConnectionError() and payment_app_database() on IO thread.

4) Shutdown()

[zino, 2016/12/15 17:56:52]

Done.

 class CONTENT_EXPORT PaymentAppContextImpl
     : public base::RefCountedThreadSafe<PaymentAppContextImpl>,
       NON_EXPORTED_BASE(public PaymentAppContext) {
  public:
-  explicit PaymentAppContextImpl(
-      scoped_refptr<ServiceWorkerContextWrapper> service_worker_context);
+  PaymentAppContextImpl();
+
+  // Init and Shutdown are for use on the UI thread when the
+  // StoragePartition is being setup and torn down.
+  void Init(scoped_refptr<ServiceWorkerContextWrapper> service_worker_context);
 
   // Shutdown must be called before deleting this. Call on the UI thread.
   void Shutdown();
 
   // Create a PaymentAppManager that is owned by this. Call on the UI
   // thread.
   void CreateService(
       mojo::InterfaceRequest<payments::mojom::PaymentAppManager> request);
 
   // Called by PaymentAppManager objects so that they can
   // be deleted. Call on the IO thread.
   void ServiceHadConnectionError(PaymentAppManager* service);
 
-  ServiceWorkerContextWrapper* service_worker_context() const;
+  // Should be accessed only on the IO thread.
+  PaymentAppDatabase* payment_app_database() const;
 
   // PaymentAppContext implementation:
   void GetAllManifests(const GetAllManifestsCallback& callback) override;
 
  protected:
   friend class PaymentAppManagerTest;
 
  private:
   friend class base::RefCountedThreadSafe<PaymentAppContextImpl>;
   ~PaymentAppContextImpl() override;
 
-  void CreateServiceOnIOThread(
+  void CreatePaymentAppDatabaseOnIO(
+      scoped_refptr<ServiceWorkerContextWrapper> service_worker_context);
+
+  void CreateServiceOnIO(
       mojo::InterfaceRequest<payments::mojom::PaymentAppManager> request);
 
   void ShutdownOnIO();
 
-  scoped_refptr<ServiceWorkerContextWrapper> service_worker_context_;
+  // Only accessed on the IO thread.
+  std::unique_ptr<PaymentAppDatabase> payment_app_database_;
 
   // The services are owned by this. They're either deleted
   // during ShutdownOnIO or when the channel is closed via
   // ServiceHadConnectionError. Only accessed on the IO thread.
   std::map<PaymentAppManager*, std::unique_ptr<PaymentAppManager>> services_;

[please use gerrit instead, 2016/12/14 19:44:00]

It's strange that "services_" is a collection of *Manager objects. Can you pick
one name and use it everywhere? Either managers_ here or PaymentAppService
instead of PaymentAppManager.

[zino, 2016/12/15 17:56:52]

Done.

 
   DISALLOW_COPY_AND_ASSIGN(PaymentAppContextImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_PAYMENTS_PAYMENT_APP_CONTEXT_IMPL_H_