Printing API: Add Observer for PrintJobHistoryService

This observer is called when new print job is saved.
This is required to fire corresponding chrome.printingMetrics events.

Bug: 992889
Change-Id: I9c8c980201e2ea181cf80b5b40bc7806bcd6c559
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1780830
Commit-Queue: Nikita Podguzov <nikitapodguzov@google.com>
Reviewed-by: Denis Kuznetsov <antrim@chromium.org>
Reviewed-by: Sean Kau <skau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#696777}

chrome/browser/chromeos/BUILD.gn

@@ -2747,6 +2747,8 @@ source_set("unit_tests") {
     "printing/history/print_job_info_conversions_unittest.cc",
     "printing/history/test_print_job_database.cc",
     "printing/history/test_print_job_database.h",
+    "printing/history/test_print_job_history_service_observer.cc",
+    "printing/history/test_print_job_history_service_observer.h",
     "printing/ppd_resolution_state_unittest.cc",
     "printing/ppd_resolution_tracker_unittest.cc",
     "printing/print_servers_provider_unittest.cc",

chrome/browser/chromeos/printing/history/print_job_history_service.cc

@@ -13,15 +13,6 @@
 
 namespace chromeos {
 
-namespace {
-
-// This callback is used in unit tests to wait until print job is saved in the
-// database and check whether the operation is successful.
-base::RepeatingCallback<void(bool success)>*
-    g_test_on_print_job_saved_callback = nullptr;
-
-}  // namespace
-
 PrintJobHistoryService::PrintJobHistoryService(
     std::unique_ptr<PrintJobDatabase> print_job_database,
     CupsPrintJobManager* print_job_manager)
@@ -42,9 +33,14 @@ void PrintJobHistoryService::GetPrintJobs(
   print_job_database_->GetPrintJobs(std::move(callback));
 }
 
-void PrintJobHistoryService::SetOnPrintJobSavedCallbackForTesting(
-    base::RepeatingCallback<void(bool success)>* callback) {
-  g_test_on_print_job_saved_callback = callback;
+void PrintJobHistoryService::AddObserver(
+    PrintJobHistoryService::Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void PrintJobHistoryService::RemoveObserver(
+    PrintJobHistoryService::Observer* observer) {
+  observers_.RemoveObserver(observer);
 }
 
 void PrintJobHistoryService::OnPrintJobDone(base::WeakPtr<CupsPrintJob> job) {
@@ -63,17 +59,21 @@ void PrintJobHistoryService::OnPrintJobCancelled(
 void PrintJobHistoryService::SavePrintJob(base::WeakPtr<CupsPrintJob> job) {
   if (!job)
     return;
+  printing::proto::PrintJobInfo print_job_info =
+      CupsPrintJobToProto(*job, /*id=*/base::GenerateGUID(), base::Time::Now());
   print_job_database_->SavePrintJob(
-      CupsPrintJobToProto(*job, /*id=*/base::GenerateGUID(), base::Time::Now()),
-      base::BindOnce(&PrintJobHistoryService::OnPrintJobSaved,
-                     base::Unretained(this)));
+      print_job_info, base::BindOnce(&PrintJobHistoryService::OnPrintJobSaved,
+                                     base::Unretained(this), print_job_info));
 }
 
-void PrintJobHistoryService::OnPrintJobSaved(bool success) {
+void PrintJobHistoryService::OnPrintJobSaved(
+    const printing::proto::PrintJobInfo& print_job_info,
+    bool success) {
   base::UmaHistogramBoolean("Printing.CUPS.PrintJobDatabasePrintJobSaved",
                             success);
-  if (g_test_on_print_job_saved_callback)
-    g_test_on_print_job_saved_callback->Run(success);
+  for (auto& observer : observers_) {
+    observer.OnPrintJobFinished(print_job_info);
+  }
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/printing/history/print_job_history_service.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_CHROMEOS_PRINTING_HISTORY_PRINT_JOB_HISTORY_SERVICE_H_
 
 #include "base/macros.h"
+#include "base/observer_list.h"
 #include "chrome/browser/chromeos/printing/cups_print_job_manager.h"
 #include "chrome/browser/chromeos/printing/history/print_job_database.h"
 #include "components/keyed_service/core/keyed_service.h"
@@ -20,6 +21,12 @@ class CupsPrintJobManager;
 class PrintJobHistoryService : public KeyedService,
                                public chromeos::CupsPrintJobManager::Observer {
  public:
+  class Observer {
+   public:
+    virtual void OnPrintJobFinished(
+        const printing::proto::PrintJobInfo& print_job_info) = 0;
+  };
+
   PrintJobHistoryService(std::unique_ptr<PrintJobDatabase> print_job_database,
                          CupsPrintJobManager* print_job_manager);
   ~PrintJobHistoryService() override;
@@ -27,10 +34,8 @@ class PrintJobHistoryService : public KeyedService,
   // Retrieves all print jobs from the database.
   void GetPrintJobs(PrintJobDatabase::GetPrintJobsCallback callback);
 
-  // Sets the callback to be called after print job is saved to the database.
-  // It is called from unit tests only.
-  void SetOnPrintJobSavedCallbackForTesting(
-      base::RepeatingCallback<void(bool success)>* callback);
+  void AddObserver(PrintJobHistoryService::Observer* observer);
+  void RemoveObserver(PrintJobHistoryService::Observer* observer);
 
  private:
   // CupsPrintJobManager::Observer
@@ -40,11 +45,14 @@ class PrintJobHistoryService : public KeyedService,
 
   void SavePrintJob(base::WeakPtr<CupsPrintJob> job);
 
-  void OnPrintJobSaved(bool success);
+  void OnPrintJobSaved(const printing::proto::PrintJobInfo& print_job_info,
+                       bool success);
 
   std::unique_ptr<PrintJobDatabase> print_job_database_;
   CupsPrintJobManager* print_job_manager_;
 
+  base::ObserverList<PrintJobHistoryService::Observer>::Unchecked observers_;
+
   DISALLOW_COPY_AND_ASSIGN(PrintJobHistoryService);
 };
 

chrome/browser/chromeos/printing/history/print_job_history_service_unittest.cc

@@ -7,6 +7,7 @@
 #include "chrome/browser/chromeos/printing/cups_print_job.h"
 #include "chrome/browser/chromeos/printing/history/print_job_info.pb.h"
 #include "chrome/browser/chromeos/printing/history/test_print_job_database.h"
+#include "chrome/browser/chromeos/printing/history/test_print_job_history_service_observer.h"
 #include "chrome/browser/chromeos/printing/test_cups_print_job_manager.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
@@ -66,18 +67,15 @@ class PrintJobHistoryServiceTest : public ::testing::Test {
 };
 
 TEST_F(PrintJobHistoryServiceTest, SaveObservedCupsPrintJob) {
+  base::RunLoop save_print_job_run_loop;
+  TestPrintJobHistoryServiceObserver observer(
+      print_job_history_service_.get(), save_print_job_run_loop.QuitClosure());
+
   std::unique_ptr<CupsPrintJob> print_job = std::make_unique<CupsPrintJob>(
       chromeos::Printer(), /*job_id=*/0, kTitle, kPagesNumber,
       ::printing::PrintJob::Source::PRINT_PREVIEW,
       /*source_id=*/"", printing::proto::PrintSettings());
   print_job_manager_->CreatePrintJob(print_job.get());
-
-  base::RunLoop save_print_job_run_loop;
-  auto callback = base::BindRepeating(
-      &PrintJobHistoryServiceTest::OnPrintJobSaved, base::Unretained(this),
-      save_print_job_run_loop.QuitClosure());
-  print_job_history_service_->SetOnPrintJobSavedCallbackForTesting(&callback);
-
   print_job_manager_->CancelPrintJob(print_job.get());
   save_print_job_run_loop.Run();
 
@@ -94,4 +92,20 @@ TEST_F(PrintJobHistoryServiceTest, SaveObservedCupsPrintJob) {
             entries_[0].status());
 }
 
+TEST_F(PrintJobHistoryServiceTest, ObserverTest) {
+  base::RunLoop run_loop;
+  TestPrintJobHistoryServiceObserver observer(print_job_history_service_.get(),
+                                              run_loop.QuitClosure());
+
+  std::unique_ptr<CupsPrintJob> print_job = std::make_unique<CupsPrintJob>(
+      chromeos::Printer(), /*job_id=*/0, kTitle, kPagesNumber,
+      ::printing::PrintJob::Source::PRINT_PREVIEW,
+      /*source_id=*/"", printing::proto::PrintSettings());
+  print_job_manager_->CreatePrintJob(print_job.get());
+  print_job_manager_->CancelPrintJob(print_job.get());
+  run_loop.Run();
+
+  EXPECT_EQ(1, observer.num_print_jobs());
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/printing/history/test_print_job_history_service_observer.cc

+// 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 "chrome/browser/chromeos/printing/history/test_print_job_history_service_observer.h"
+
+namespace chromeos {
+
+TestPrintJobHistoryServiceObserver::TestPrintJobHistoryServiceObserver(
+    PrintJobHistoryService* print_job_history_service,
+    base::RepeatingClosure run_loop_closure)
+    : print_job_history_service_(print_job_history_service),
+      run_loop_closure_(run_loop_closure) {
+  print_job_history_service_->AddObserver(this);
+}
+
+TestPrintJobHistoryServiceObserver::~TestPrintJobHistoryServiceObserver() {
+  print_job_history_service_->RemoveObserver(this);
+}
+
+void TestPrintJobHistoryServiceObserver::OnPrintJobFinished(
+    const printing::proto::PrintJobInfo& print_job_info) {
+  num_print_jobs_++;
+  run_loop_closure_.Run();
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/printing/history/test_print_job_history_service_observer.h

+// 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 CHROME_BROWSER_CHROMEOS_PRINTING_HISTORY_TEST_PRINT_JOB_HISTORY_SERVICE_OBSERVER_H_
+#define CHROME_BROWSER_CHROMEOS_PRINTING_HISTORY_TEST_PRINT_JOB_HISTORY_SERVICE_OBSERVER_H_
+
+#include "base/callback.h"
+#include "chrome/browser/chromeos/printing/history/print_job_history_service.h"
+
+namespace chromeos {
+
+// Observer that counts the number of times it has been called.
+class TestPrintJobHistoryServiceObserver
+    : public PrintJobHistoryService::Observer {
+ public:
+  TestPrintJobHistoryServiceObserver(
+      PrintJobHistoryService* print_job_history_service,
+      base::RepeatingClosure run_loop_closure);
+  ~TestPrintJobHistoryServiceObserver();
+
+  int num_print_jobs() { return num_print_jobs_; }
+
+ private:
+  // PrintJobHistoryService::Observer:
+  void OnPrintJobFinished(
+      const printing::proto::PrintJobInfo& print_job_info) override;
+
+  chromeos::PrintJobHistoryService* print_job_history_service_;
+  base::RepeatingClosure run_loop_closure_;
+
+  // The number of times the observer is called.
+  int num_print_jobs_ = 0;
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_PRINTING_HISTORY_TEST_PRINT_JOB_HISTORY_SERVICE_OBSERVER_H_