scanning: Add ScanJobObserver

Add a new ScanJobObserver to receive updates for in-progress scan jobs.
The observer is implemented by and passed from the scan app to the
browser in calls to ScanService::StartScan().

Bug: 1059779
Change-Id: Ic9e73d688f7639cf8a892e8f5d269840d1b981c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2508435
Commit-Queue: Jesse Schettler <jschettler@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825078}

chrome/browser/chromeos/scanning/scan_service.cc

@@ -31,6 +31,9 @@ constexpr char kActiveUserMyFilesPath[] = "/home/chronos/user/MyFiles";
 // The conversion quality when converting from PNG to JPG.
 constexpr int kJpgQuality = 100;
 
+// The max progress percent that can be reported for a scanned page.
+constexpr uint32_t kMaxProgressPercent = 100;
+
 // Converts |png_img| to JPG.
 std::string PngToJpg(const std::string& png_img) {
   std::vector<uint8_t> jpg_img;
@@ -76,25 +79,31 @@ void ScanService::GetScannerCapabilities(
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
-void ScanService::Scan(const base::UnguessableToken& scanner_id,
-                       mojo_ipc::ScanSettingsPtr settings,
-                       ScanCallback callback) {
+void ScanService::StartScan(
+    const base::UnguessableToken& scanner_id,
+    mojo_ipc::ScanSettingsPtr settings,
+    mojo::PendingRemote<mojo_ipc::ScanJobObserver> observer,
+    StartScanCallback callback) {
   const std::string scanner_name = GetScannerName(scanner_id);
   if (scanner_name.empty() || !FilePathSupported(settings->scan_to_path)) {
     std::move(callback).Run(false);
     return;
   }
 
+  scan_job_observer_.Bind(std::move(observer));
+
   base::Time::Now().LocalExplode(&start_time_);
   save_failed_ = false;
   lorgnette_scanner_manager_->Scan(
       scanner_name, mojo::ConvertTo<lorgnette::ScanSettings>(settings),
-      base::NullCallback(),
+      base::BindRepeating(&ScanService::OnProgressPercentReceived,
+                          weak_ptr_factory_.GetWeakPtr()),
       base::BindRepeating(&ScanService::OnPageReceived,
                           weak_ptr_factory_.GetWeakPtr(),
                           settings->scan_to_path, settings->file_type),
       base::BindOnce(&ScanService::OnScanCompleted,
-                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+                     weak_ptr_factory_.GetWeakPtr()));
+  std::move(callback).Run(true);
 }
 
 void ScanService::BindInterface(
@@ -147,10 +156,26 @@ void ScanService::OnScannerCapabilitiesReceived(
       mojo::ConvertTo<mojo_ipc::ScannerCapabilitiesPtr>(capabilities.value()));
 }
 
+void ScanService::OnProgressPercentReceived(uint32_t progress_percent,
+                                            uint32_t page_number) {
+  DCHECK_LE(progress_percent, kMaxProgressPercent);
+  DCHECK(scan_job_observer_.is_connected());
+  scan_job_observer_->OnPageProgress(page_number, progress_percent);
+}
+
 void ScanService::OnPageReceived(const base::FilePath& scan_to_path,
                                  const mojo_ipc::FileType file_type,
                                  std::string scanned_image,
                                  uint32_t page_number) {
+  // TODO(b/172670649): Update LorgnetteManagerClient to pass scan data as a
+  // vector.
+  // In case the last reported progress percent was less than 100, send one
+  // final progress event before the page complete event.
+  DCHECK(scan_job_observer_.is_connected());
+  scan_job_observer_->OnPageProgress(page_number, kMaxProgressPercent);
+  scan_job_observer_->OnPageComplete(
+      std::vector<uint8_t>(scanned_image.begin(), scanned_image.end()));
+
   std::string filename;
   std::string file_ext;
   switch (file_type) {
@@ -182,8 +207,10 @@ void ScanService::OnPageReceived(const base::FilePath& scan_to_path,
   }
 }
 
-void ScanService::OnScanCompleted(ScanCallback callback, bool success) {
-  std::move(callback).Run(success && !save_failed_);
+void ScanService::OnScanCompleted(bool success) {
+  DCHECK(scan_job_observer_.is_connected());
+  scan_job_observer_->OnScanComplete(success && !save_failed_);
+  scan_job_observer_.reset();
 }
 
 bool ScanService::FilePathSupported(const base::FilePath& file_path) {

chrome/browser/chromeos/scanning/scan_service.h

@@ -19,7 +19,9 @@
 #include "chromeos/dbus/lorgnette/lorgnette_service.pb.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver.h"
+#include "mojo/public/cpp/bindings/remote.h"
 
 namespace chromeos {
 
@@ -42,9 +44,10 @@ class ScanService : public scanning::mojom::ScanService, public KeyedService {
   void GetScanners(GetScannersCallback callback) override;
   void GetScannerCapabilities(const base::UnguessableToken& scanner_id,
                               GetScannerCapabilitiesCallback callback) override;
-  void Scan(const base::UnguessableToken& scanner_id,
-            scanning::mojom::ScanSettingsPtr settings,
-            ScanCallback callback) override;
+  void StartScan(const base::UnguessableToken& scanner_id,
+                 scanning::mojom::ScanSettingsPtr settings,
+                 mojo::PendingRemote<scanning::mojom::ScanJobObserver> observer,
+                 StartScanCallback callback) override;
 
   // Binds receiver_ by consuming |pending_receiver|.
   void BindInterface(
@@ -70,6 +73,11 @@ class ScanService : public scanning::mojom::ScanService, public KeyedService {
       GetScannerCapabilitiesCallback callback,
       const base::Optional<lorgnette::ScannerCapabilities>& capabilities);
 
+  // Receives progress updates after calling LorgnetteScannerManager::Scan().
+  // |page_number| indicates the page the |progress_percent| corresponds to.
+  void OnProgressPercentReceived(uint32_t progress_percent,
+                                 uint32_t page_number);
+
   // Processes each |scanned_image| received after calling
   // LorgnetteScannerManager::Scan(). |scan_to_path| is where images will be
   // saved, and |file_type| specifies the file type to use when saving scanned
@@ -80,7 +88,7 @@ class ScanService : public scanning::mojom::ScanService, public KeyedService {
                       uint32_t page_number);
 
   // Processes the final result of calling LorgnetteScannerManager::Scan().
-  void OnScanCompleted(ScanCallback callback, bool success);
+  void OnScanCompleted(bool success);
 
   // TODO(jschettler): Replace this with a generic helper function when one is
   // available.
@@ -100,6 +108,10 @@ class ScanService : public scanning::mojom::ScanService, public KeyedService {
   // chromeos::scanning::mojom::ScanService interface.
   mojo::Receiver<scanning::mojom::ScanService> receiver_{this};
 
+  // Used to send scan job events to an observer. The remote is bound when a
+  // scan job is started and is disconnected when the scan job is complete.
+  mojo::Remote<scanning::mojom::ScanJobObserver> scan_job_observer_;
+
   // Unowned. Used to get scanner information and perform scans.
   LorgnetteScannerManager* lorgnette_scanner_manager_;
 

chrome/browser/chromeos/scanning/scan_service_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/chromeos/scanning/scan_service.h"
 
+#include <cstdint>
 #include <map>
 #include <string>
 #include <vector>
@@ -22,6 +23,8 @@
 #include "chromeos/components/scanning/mojom/scanning.mojom-test-utils.h"
 #include "chromeos/components/scanning/mojom/scanning.mojom.h"
 #include "chromeos/dbus/lorgnette/lorgnette_service.pb.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -65,6 +68,49 @@ lorgnette::ScannerCapabilities CreateLorgnetteScannerCapabilities() {
 
 }  // namespace
 
+class FakeScanJobObserver : public mojo_ipc::ScanJobObserver {
+ public:
+  FakeScanJobObserver() = default;
+  ~FakeScanJobObserver() override = default;
+
+  FakeScanJobObserver(const FakeScanJobObserver&) = delete;
+  FakeScanJobObserver& operator=(const FakeScanJobObserver&) = delete;
+
+  // mojo_ipc::ScanJobObserver:
+  void OnPageProgress(uint32_t page_number,
+                      uint32_t progress_percent) override {
+    progress_ = progress_percent;
+  }
+
+  void OnPageComplete(const std::vector<uint8_t>& page_data) override {
+    page_complete_ = true;
+  }
+
+  void OnScanComplete(bool success) override { scan_success_ = success; }
+
+  // Creates a pending remote that can be passed in calls to
+  // ScanService::StartScan().
+  mojo::PendingRemote<mojo_ipc::ScanJobObserver> GenerateRemote() {
+    if (receiver_.is_bound())
+      receiver_.reset();
+
+    mojo::PendingRemote<mojo_ipc::ScanJobObserver> remote;
+    receiver_.Bind(remote.InitWithNewPipeAndPassReceiver());
+    return remote;
+  }
+
+  // Returns true if the scan completed successfully.
+  bool scan_success() const {
+    return progress_ == 100 && page_complete_ && scan_success_;
+  }
+
+ private:
+  uint32_t progress_ = 0;
+  bool page_complete_ = false;
+  bool scan_success_ = false;
+  mojo::Receiver<mojo_ipc::ScanJobObserver> receiver_{this};
+};
+
 class ScanServiceTest : public testing::Test {
  public:
   ScanServiceTest() = default;
@@ -95,18 +141,21 @@ class ScanServiceTest : public testing::Test {
   }
 
   // Performs a scan with the scanner identified by |scanner_id| with the given
-  // |settings| by calling ScanService::Scan() via the mojo::Remote.
+  // |settings| by calling ScanService::StartScan() via the mojo::Remote.
   bool Scan(const base::UnguessableToken& scanner_id,
             mojo_ipc::ScanSettingsPtr settings) {
     bool success;
     mojo_ipc::ScanServiceAsyncWaiter(scan_service_remote_.get())
-        .Scan(scanner_id, std::move(settings), &success);
+        .StartScan(scanner_id, std::move(settings),
+                   fake_scan_job_observer_.GenerateRemote(), &success);
+    scan_service_remote_.FlushForTesting();
     return success;
   }
 
  protected:
   base::ScopedTempDir temp_dir_;
   FakeLorgnetteScannerManager fake_lorgnette_scanner_manager_;
+  FakeScanJobObserver fake_scan_job_observer_;
   ScanService scan_service_{&fake_lorgnette_scanner_manager_, base::FilePath(),
                             base::FilePath()};
 
@@ -242,6 +291,7 @@ TEST_F(ScanServiceTest, Scan) {
     settings.file_type = type.second;
     EXPECT_TRUE(Scan(scanners[0]->id, settings.Clone()));
     EXPECT_TRUE(base::PathExists(saved_scan_path));
+    EXPECT_TRUE(fake_scan_job_observer_.scan_success());
   }
 }
 

chrome/test/data/webui/chromeos/scanning/scanning_app_test.js

@@ -55,15 +55,19 @@ class FakeScanService {
      */
     this.capabilities_ = new Map();
 
+    /** @private {?chromeos.scanning.mojom.ScanJobObserverRemote} */
+    this.scanJobObserverRemote_ = null;
+
     this.resetForTest();
   }
 
   resetForTest() {
     this.scanners_ = [];
     this.capabilities_ = new Map();
+    this.scanJobObserverRemote_ = null;
     this.resolverMap_.set('getScanners', new PromiseResolver());
     this.resolverMap_.set('getScannerCapabilities', new PromiseResolver());
-    this.resolverMap_.set('scan', new PromiseResolver());
+    this.resolverMap_.set('startScan', new PromiseResolver());
   }
 
   /**
@@ -114,6 +118,36 @@ class FakeScanService {
     this.capabilities_ = capabilities;
   }
 
+  /**
+   * @param {number} pageNumber
+   * @param {number} progressPercent
+   * @return {!Promise}
+   */
+  simulateProgress(pageNumber, progressPercent) {
+    this.scanJobObserverRemote_.onPageProgress(pageNumber, progressPercent);
+    return flushTasks();
+  }
+
+  /**
+   * @param {number} pageNumber
+   * @return {!Promise}
+   */
+  simulatePageComplete(pageNumber) {
+    this.scanJobObserverRemote_.onPageProgress(pageNumber, 100);
+    const fakePageData = [2, 57, 13, 289];
+    this.scanJobObserverRemote_.onPageComplete(fakePageData);
+    return flushTasks();
+  }
+
+  /**
+   * @param {boolean} success
+   * @return {!Promise}
+   */
+  simulateScanComplete(success) {
+    this.scanJobObserverRemote_.onScanComplete(success);
+    return flushTasks();
+  }
+
   // scanService methods:
 
   /** @return {!Promise<{scanners: !ScannerArr}>} */
@@ -139,11 +173,13 @@ class FakeScanService {
   /**
    * @param {!mojoBase.mojom.UnguessableToken} scanner_id
    * @param {!chromeos.scanning.mojom.ScanSettings} settings
+   * @param {!chromeos.scanning.mojom.ScanJobObserverRemote} remote
    * @return {!Promise<{success: boolean}>}
    */
-  scan(scanner_id, settings) {
+  startScan(scanner_id, settings, remote) {
     return new Promise(resolve => {
-      this.methodCalled('scan');
+      this.scanJobObserverRemote_ = remote;
+      this.methodCalled('startScan');
       resolve({success: true});
     });
   }
@@ -305,11 +341,14 @@ export function scanningAppTest() {
           assertFalse(scanButton.disabled);
           assertEquals('', statusText.textContent.trim());
 
+          // Click the Scan button and wait till the scan is started.
           scanButton.click();
-
-          // After the scan button is clicked, the settings and scan button
-          // should be disabled, and the scan status should indicate that
-          // scanning is in progress.
+          return fakeScanService_.whenCalled('startScan');
+        })
+        .then(() => {
+          // After the scan button is clicked and the scan has started, the
+          // settings and scan button should be disabled, and the scan status
+          // should indicate that scanning is in progress.
           assertTrue(scannerSelect.disabled);
           assertTrue(sourceSelect.disabled);
           assertTrue(fileTypeSelect.disabled);
@@ -317,8 +356,23 @@ export function scanningAppTest() {
           assertTrue(pageSizeSelect.disabled);
           assertTrue(resolutionSelect.disabled);
           assertTrue(scanButton.disabled);
-          assertEquals('Scanning...', statusText.textContent.trim());
-          return fakeScanService_.whenCalled('scan');
+          assertEquals('Scanning page 1: 0%', statusText.textContent.trim());
+
+          // Simulate a progress update and verify the status is set correctly.
+          return fakeScanService_.simulateProgress(1, 17);
+        })
+        .then(() => {
+          assertEquals('Scanning page 1: 17%', statusText.textContent.trim());
+
+          // Simulate a page complete update and verify the status is set
+          // correctly.
+          return fakeScanService_.simulatePageComplete(1);
+        })
+        .then(() => {
+          assertEquals('Scanning page 1: 100%', statusText.textContent.trim());
+
+          // Complete the scan.
+          return fakeScanService_.simulateScanComplete(true);
         })
         .then(() => {
           // After scanning is complete, the settings and scan button should be

chromeos/components/scanning/mojom/scanning.mojom

@@ -89,6 +89,24 @@ struct Scanner {
   mojo_base.mojom.String16 display_name;
 };
 
+// Observer interface used to send remote updates about an in-progress scan job
+// to the Scan app (chrome://scanning) receiver. When the corresponding scan job
+// is complete, the remote and receiver are disconnected.
+interface ScanJobObserver {
+  // Called when the progress percent of the page currently being scanned
+  // changes. |page_number| indicates which page the update is for.
+  OnPageProgress(uint32 page_number, uint32 progress_percent);
+
+  // Called when scanning a page is complete. |page_data| contains the page's
+  // image data encoded as a PNG.
+  // TODO(jschettler): Send a lower resolution preview.
+  OnPageComplete(array<uint8> page_data);
+
+  // Called when the scan is complete. |success| indicates whether the scan
+  // completed successfully.
+  OnScanComplete(bool success);
+};
+
 // Interface used to obtain information about and interact with connected
 // scanners. It is implemented in the browser and exposed to the Scan app
 // (chrome://scanning).
@@ -104,8 +122,9 @@ interface ScanService {
   GetScannerCapabilities(mojo_base.mojom.UnguessableToken scanner_id)
       => (ScannerCapabilities capabilities);
 
-  // Performs a scan using the provided |settings|.
-  // TODO(jschettler): Send a ScanJobObserver to get scan job updates.
-  Scan(mojo_base.mojom.UnguessableToken scanner_id, ScanSettings settings)
-      => (bool success);
+  // Starts a scan with the scanner identified by |scanner_id| using the
+  // provided |settings|. Scan job events are reported to the client via the
+  // |observer|. |success| indicates whether the scan started successfully.
+  StartScan(mojo_base.mojom.UnguessableToken scanner_id, ScanSettings settings,
+      pending_remote<ScanJobObserver> observer) => (bool success);
 };

chromeos/components/scanning/resources/scanning_app.js

@@ -45,6 +45,12 @@ Polymer({
 
   behaviors: [I18nBehavior],
 
+  /**
+   * Receives scan job notifications.
+   * @private {?chromeos.scanning.mojom.ScanJobObserverReceiver}
+   */
+  scanJobObserverReceiver_: null,
+
   /** @private {?chromeos.scanning.mojom.ScanServiceInterface} */
   scanService_: null,
 
@@ -85,6 +91,12 @@ Polymer({
     /** @type {string} */
     selectedResolution: String,
 
+    /** @private {!Array<string>} */
+    objectUrls_: {
+      type: Array,
+      value: () => [],
+    },
+
     /** @private {!Array<chromeos.scanning.mojom.PageSize>} */
     selectedSourcePageSizes_: {
       type: Array,
@@ -128,6 +140,48 @@ Polymer({
         });
   },
 
+  /** @override */
+  detached() {
+    if (this.scanJobObserverReceiver_) {
+      this.scanJobObserverReceiver_.$.close();
+    }
+  },
+
+  /**
+   * Overrides chromeos.scanning.mojom.ScanJobObserverInterface.
+   * @param {number} pageNumber
+   * @param {number} progressPercent
+   */
+  onPageProgress(pageNumber, progressPercent) {
+    // TODO(jschettler): Move this text to the preview area and add a progress
+    // bar to display the progress.
+    this.statusText_ =
+        'Scanning page ' + pageNumber + ': ' + progressPercent + '%';
+  },
+
+  /**
+   * Overrides chromeos.scanning.mojom.ScanJobObserverInterface.
+   * @param {!Array<number>} pageData
+   */
+  onPageComplete(pageData) {
+    // TODO(jschettler): Display the scanned images in the preview area when the
+    // scan is complete.
+    const blob = new Blob([Uint8Array.from(pageData)], {'type': 'image/png'});
+    this.push('objectUrls_', URL.createObjectURL(blob));
+  },
+
+  /**
+   * Overrides chromeos.scanning.mojom.ScanJobObserverInterface.
+   * @param {boolean} success
+   */
+  onScanComplete(success) {
+    this.statusText_ = success ?
+        'Scan complete! File(s) saved to ' + this.selectedFilePath + '.' :
+        'Scan failed.';
+    this.settingsDisabled_ = false;
+    this.scanButtonDisabled_ = false;
+  },
+
   /**
    * @param {string} selectedSource
    * @return {!Array<chromeos.scanning.mojom.PageSize>}
@@ -222,9 +276,7 @@ Polymer({
       return;
     }
 
-    this.statusText_ = 'Scanning...';
-    this.settingsDisabled_ = true;
-    this.scanButtonDisabled_ = true;
+    this.objectUrls_ = [];
 
     const settings = {
       'sourceName': this.selectedSource,
@@ -234,11 +286,23 @@ Polymer({
       'pageSize': pageSizeFromString(this.selectedPageSize),
       'resolutionDpi': Number(this.selectedResolution),
     };
+
+    if (!this.scanJobObserverReceiver_) {
+      this.scanJobObserverReceiver_ =
+          new chromeos.scanning.mojom.ScanJobObserverReceiver(
+              /**
+               * @type {!chromeos.scanning.mojom.ScanJobObserverInterface}
+               */
+              (this));
+    }
+
     this.scanService_
-        .scan(this.scannerIds_.get(this.selectedScannerId), settings)
+        .startScan(
+            this.scannerIds_.get(this.selectedScannerId), settings,
+            this.scanJobObserverReceiver_.$.bindNewPipeAndPassRemote())
         .then(
             /*@type {!{success: boolean}}*/ (response) => {
-              this.onScanCompleted_(response);
+              this.onStartScanResponse_(response);
             });
   },
 
@@ -246,16 +310,15 @@ Polymer({
    * @param {!{success: boolean}} response
    * @private
    */
-  onScanCompleted_(response) {
-    if (response.success) {
-      this.statusText_ =
-          'Scan complete! File(s) saved to ' + this.selectedFilePath + '.';
-    } else {
-      this.statusText_ = 'Scan failed.';
+  onStartScanResponse_(response) {
+    if (!response.success) {
+      this.statusText_ = 'Failed to start scan.';
+      return;
     }
 
-    this.settingsDisabled_ = false;
-    this.scanButtonDisabled_ = false;
+    this.statusText_ = 'Scanning page 1: 0%';
+    this.settingsDisabled_ = true;
+    this.scanButtonDisabled_ = true;
   },
 
   /** @private */