[Payment Request][Desktop] State initialization task.
This patch refactors the relationship between PaymentRequestDialogView
and PaymentRequestState to be more generic. Instead of waiting for the
state event of fetching all payment instruments (specific), the view now
waits for an initialization task (abstraction) to finish initialization
(generic). This will help to add more initialization tasks in follow up
patches. For example, PaymentRequestSpec will become an initialization
task as well, so the view can wait for the spec to completely
initialized, in case if PaymentRequest.show() was called with a promise.
Bug: 817073
Change-Id: I19c133ab3084a7a26628380135c21abeeb1c7436
Reviewed-on: https://chromium-review.googlesource.com/c/1478090
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634710}diff --git a/chrome/browser/ui/views/payments/payment_request_dialog_view.cc b/chrome/browser/ui/views/payments/payment_request_dialog_view.cc
index 43a2410..4e44ab7 100644
--- a/chrome/browser/ui/views/payments/payment_request_dialog_view.cc
+++ b/chrome/browser/ui/views/payments/payment_request_dialog_view.cc
@@ -79,10 +79,13 @@
AddChildView(view_stack_.get());
SetupSpinnerOverlay();
- // Show spinner when getting all payment instruments. The spinner will be
- // hidden in OnGetAllPaymentInstrumentsFinished.
- if (!request->state()->is_get_all_instruments_finished()) {
- request->state()->AddObserver(this);
+
+ if (!request->state()->IsInitialized()) {
+ request->state()->AddInitializationObserver(this);
+ ++number_of_initialization_tasks_;
+ }
+
+ if (number_of_initialization_tasks_ > 0) {
ShowProcessingSpinner();
} else if (observer_for_testing_) {
// When testing, signal that the processing spinner events have passed, even
@@ -233,18 +236,18 @@
observer_for_testing_->OnSpecDoneUpdating();
}
-void PaymentRequestDialogView::OnGetAllPaymentInstrumentsFinished() {
+void PaymentRequestDialogView::OnInitialized(
+ InitializationTask* initialization_task) {
+ initialization_task->RemoveInitializationObserver(this);
+ if (--number_of_initialization_tasks_ > 0)
+ return;
+
HideProcessingSpinner();
+
if (request_->state()->are_requested_methods_supported()) {
request_->RecordDialogShownEventInJourneyLogger();
- if (observer_for_testing_) {
- // The OnGetAllPaymentInstrumentsFinished() method is called if the
- // payment instruments were retrieved asynchronously. This method hides
- // the "Processing" spinner, so the UI is now ready for interaction. Any
- // test that opens UI can now interact with it. The OnDialogOpened() call
- // notifies the tests of this event.
+ if (observer_for_testing_)
observer_for_testing_->OnDialogOpened();
- }
}
}
@@ -419,16 +422,14 @@
request_->spec(), request_->state(), this),
&controller_map_),
/* animate = */ false);
- if (request_->state()->is_get_all_instruments_finished() &&
- request_->state()->are_requested_methods_supported()) {
+
+ if (number_of_initialization_tasks_ > 0)
+ return;
+
+ if (request_->state()->are_requested_methods_supported()) {
request_->RecordDialogShownEventInJourneyLogger();
- if (observer_for_testing_) {
- // The is_get_all_instruments_finished() method returns true if all
- // payment instruments were retrieved synchronously. Any test that opens
- // UI can now interact with it. The OnDialogOpened() call notifies the
- // tests of this event.
+ if (observer_for_testing_)
observer_for_testing_->OnDialogOpened();
- }
}
}
diff --git a/chrome/browser/ui/views/payments/payment_request_dialog_view.h b/chrome/browser/ui/views/payments/payment_request_dialog_view.h
index 2abe09f..29d7028 100644
--- a/chrome/browser/ui/views/payments/payment_request_dialog_view.h
+++ b/chrome/browser/ui/views/payments/payment_request_dialog_view.h
@@ -11,6 +11,7 @@
#include "base/callback_forward.h"
#include "base/macros.h"
#include "chrome/browser/ui/views/payments/view_stack.h"
+#include "components/payments/content/initialization_task.h"
#include "components/payments/content/payment_request_dialog.h"
#include "components/payments/content/payment_request_spec.h"
#include "components/payments/content/payment_request_state.h"
@@ -46,7 +47,7 @@
class PaymentRequestDialogView : public views::DialogDelegateView,
public PaymentRequestDialog,
public PaymentRequestSpec::Observer,
- public PaymentRequestState::Observer {
+ public InitializationTask::Observer {
public:
class ObserverForTest {
public:
@@ -119,9 +120,8 @@
void OnStartUpdating(PaymentRequestSpec::UpdateReason reason) override;
void OnSpecUpdated() override;
- // PaymentRequestState::Observer:
- void OnGetAllPaymentInstrumentsFinished() override;
- void OnSelectedInformationChanged() override {}
+ // InitializationTask::Observer:
+ void OnInitialized(InitializationTask* initialization_task) override;
void Pay();
void GoBack();
@@ -211,6 +211,9 @@
// controller_map_.
bool being_closed_;
+ // The number of initialization tasks that are not yet initialized.
+ size_t number_of_initialization_tasks_ = 0;
+
DISALLOW_COPY_AND_ASSIGN(PaymentRequestDialogView);
};
diff --git a/components/payments/content/BUILD.gn b/components/payments/content/BUILD.gn
index 0ac8683..0c55166 100644
--- a/components/payments/content/BUILD.gn
+++ b/components/payments/content/BUILD.gn
@@ -9,6 +9,8 @@
"can_make_payment_query_factory.cc",
"can_make_payment_query_factory.h",
"content_payment_request_delegate.h",
+ "initialization_task.cc",
+ "initialization_task.h",
"payment_request.cc",
"payment_request.h",
"payment_request_converter.cc",
diff --git a/components/payments/content/initialization_task.cc b/components/payments/content/initialization_task.cc
new file mode 100644
index 0000000..900cc00
--- /dev/null
+++ b/components/payments/content/initialization_task.cc
@@ -0,0 +1,31 @@
+// Copyright 2019 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 "components/payments/content/initialization_task.h"
+
+namespace payments {
+
+InitializationTask::Observer::~Observer() = default;
+
+InitializationTask::InitializationTask() = default;
+
+InitializationTask::~InitializationTask() = default;
+
+void InitializationTask::AddInitializationObserver(Observer* observer) {
+ observers_.AddObserver(observer);
+}
+
+void InitializationTask::RemoveInitializationObserver(Observer* observer) {
+ observers_.RemoveObserver(observer);
+}
+
+void InitializationTask::NotifyInitialized() {
+ DCHECK(!has_notified_);
+ has_notified_ = true;
+ for (Observer& observer : observers_) {
+ observer.OnInitialized(this);
+ }
+}
+
+} // namespace payments
diff --git a/components/payments/content/initialization_task.h b/components/payments/content/initialization_task.h
new file mode 100644
index 0000000..16c094c
--- /dev/null
+++ b/components/payments/content/initialization_task.h
@@ -0,0 +1,105 @@
+// Copyright 2019 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 COMPONENTS_PAYMENTS_CONTENT_INITIALIZATION_TASK_H_
+#define COMPONENTS_PAYMENTS_CONTENT_INITIALIZATION_TASK_H_
+
+#include "base/macros.h"
+#include "base/observer_list.h"
+#include "base/observer_list_types.h"
+
+namespace payments {
+
+// An interface for a task that takes time to initialize. Useful for monitoring
+// initialization of several asynchronous tasks.
+//
+// Sample usage:
+//
+// class Foo : public InitializationTask {
+// public:
+// Foo() {}
+//
+// ~Foo() override {}
+//
+// // InitializationTask:
+// bool IsInitialized() override {
+// return is_initialized_;
+// }
+//
+// void SomeAction() {
+// is_initialized_ = true;
+// NotifyInitialized();
+// }
+//
+// private:
+// bool is_initialized_ = false;
+// };
+class InitializationTask {
+ public:
+ // An interface for an observer of an initialization task.
+ //
+ // Sample usage:
+ //
+ // class Bar : public InitializationTask::Observer {
+ // public:
+ // explicit Bar(Foo* foo) : foo_(foo) {
+ // if (foo_->IsInitialized()) {
+ // UseFoo();
+ // } else {
+ // foo_->AddInitializationObserver(this);
+ // }
+ // }
+ //
+ // ~Bar() override {}
+ //
+ // // InitializationTask::Observer:
+ // void OnInitialized(InitializationTask* initialization_task) override {
+ // initialization_task->RemoveInitializationObserver(this);
+ // UseFoo();
+ // }
+ //
+ // void UseFoo() {
+ // foo_->DoSomethingInteresting();
+ // }
+ //
+ // private:
+ // // Not owned. Must outlive Bar.
+ // Foo* foo_;
+ // };
+ class Observer : public base::CheckedObserver {
+ public:
+ ~Observer() override;
+
+ // Called when the observed task has initialized.
+ virtual void OnInitialized(InitializationTask* initialization_task) = 0;
+ };
+
+ InitializationTask();
+ virtual ~InitializationTask();
+
+ // Add the |observer| to be notified of initialization.
+ void AddInitializationObserver(Observer* observer);
+
+ // Remove the |observer| of initialization.
+ void RemoveInitializationObserver(Observer* observer);
+
+ // Notify all observers of initialization. Should be called at most once.
+ void NotifyInitialized();
+
+ // Whether the task has initialized.
+ virtual bool IsInitialized() const = 0;
+
+ private:
+ // The list of observers for this initialization task.
+ base::ObserverList<Observer> observers_;
+
+ // Whether NotifyInitialized() has been called.
+ bool has_notified_ = false;
+
+ DISALLOW_COPY_AND_ASSIGN(InitializationTask);
+};
+
+} // namespace payments
+
+#endif // COMPONENTS_PAYMENTS_CONTENT_INITIALIZATION_TASK_H_
diff --git a/components/payments/content/payment_request_state.cc b/components/payments/content/payment_request_state.cc
index c037288..c86818e9 100644
--- a/components/payments/content/payment_request_state.cc
+++ b/components/payments/content/payment_request_state.cc
@@ -453,6 +453,10 @@
return payment_request_delegate_->GetAddressNormalizer();
}
+bool PaymentRequestState::IsInitialized() const {
+ return get_all_instruments_finished_;
+}
+
void PaymentRequestState::PopulateProfileCache() {
std::vector<autofill::AutofillProfile*> profiles =
personal_data_manager_->GetProfilesToSuggest();
@@ -556,6 +560,7 @@
void PaymentRequestState::NotifyOnGetAllPaymentInstrumentsFinished() {
for (auto& observer : observers_)
observer.OnGetAllPaymentInstrumentsFinished();
+ NotifyInitialized();
}
void PaymentRequestState::NotifyOnSelectedInformationChanged() {
diff --git a/components/payments/content/payment_request_state.h b/components/payments/content/payment_request_state.h
index f149273..cc9dadfb 100644
--- a/components/payments/content/payment_request_state.h
+++ b/components/payments/content/payment_request_state.h
@@ -12,6 +12,7 @@
#include "base/macros.h"
#include "base/observer_list.h"
#include "components/autofill/core/browser/address_normalizer.h"
+#include "components/payments/content/initialization_task.h"
#include "components/payments/content/payment_request_spec.h"
#include "components/payments/content/payment_response_helper.h"
#include "components/payments/content/service_worker_payment_app_factory.h"
@@ -39,7 +40,8 @@
// what the merchant has specified, as input into the "is ready to pay"
// computation.
class PaymentRequestState : public PaymentResponseHelper::Delegate,
- public PaymentRequestSpec::Observer {
+ public PaymentRequestSpec::Observer,
+ public InitializationTask {
public:
// Any class call add itself as Observer via AddObserver() and receive
// notification about the state changing.
@@ -219,6 +221,9 @@
autofill::AddressNormalizer* GetAddressNormalizer();
+ // InitializationTask:
+ bool IsInitialized() const override;
+
private:
// Fetches the Autofill Profiles for this user from the PersonalDataManager,
// and stores copies of them, owned by this PaymentRequestState, in
