scanning: Return saved file path on scan completion

On completion of a scan job, return the file path of the last scanned
file to the Scanning App. Will be used to open the Files app.

Bug: 1059779
Change-Id: Ia3d33155daf472fc65fa2a959c068caddb3bff2f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2545743Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Jesse Schettler <jschettler@chromium.org>
Commit-Queue: Gavin Williams <gavinwill@chromium.org>
Cr-Commit-Position: refs/heads/master@{#830744}

chrome/browser/chromeos/extensions/document_scan/document_scan_api_unittest.cc

@@ -97,7 +97,8 @@ TEST_F(DocumentScanScanFunctionTest, ScanImageError) {
 
 TEST_F(DocumentScanScanFunctionTest, Success) {
   GetLorgnetteScannerManager()->SetGetScannerNamesResponse({kTestScannerName});
-  GetLorgnetteScannerManager()->SetScanResponse("PrettyPicture");
+  const std::vector<std::string> scan_data = {"PrettyPicture"};
+  GetLorgnetteScannerManager()->SetScanResponse(scan_data);
   std::unique_ptr<base::DictionaryValue> result(RunFunctionAndReturnDictionary(
       function_.get(), "[{\"mimeTypes\": [\"image/png\"]}]"));
   ASSERT_NE(nullptr, result.get());

chrome/browser/chromeos/scanning/fake_lorgnette_scanner_manager.cc

@@ -36,18 +36,19 @@ void FakeLorgnetteScannerManager::Scan(const std::string& scanner_name,
                                        PageCallback page_callback,
                                        CompletionCallback completion_callback) {
   if (scan_data_.has_value()) {
-    constexpr uint32_t page_number = 1;
-    if (progress_callback) {
-      for (const uint32_t progress : {7, 22, 40, 42, 59, 74, 95}) {
-        base::ThreadTaskRunnerHandle::Get()->PostTask(
-            FROM_HERE,
-            base::BindOnce(progress_callback, progress, page_number));
+    uint32_t page_number = 1;
+    for (const std::string& page_data : scan_data_.value()) {
+      if (progress_callback) {
+        for (const uint32_t progress : {7, 22, 40, 42, 59, 74, 95}) {
+          base::ThreadTaskRunnerHandle::Get()->PostTask(
+              FROM_HERE,
+              base::BindOnce(progress_callback, progress, page_number));
+        }
       }
-    }
 
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE,
-        base::BindOnce(page_callback, scan_data_.value(), page_number));
+      base::ThreadTaskRunnerHandle::Get()->PostTask(
+          FROM_HERE, base::BindOnce(page_callback, page_data, page_number++));
+    }
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
@@ -72,7 +73,7 @@ void FakeLorgnetteScannerManager::SetGetScannerCapabilitiesResponse(
 }
 
 void FakeLorgnetteScannerManager::SetScanResponse(
-    const base::Optional<std::string>& scan_data) {
+    const base::Optional<std::vector<std::string>>& scan_data) {
   scan_data_ = scan_data;
 }
 

chrome/browser/chromeos/scanning/fake_lorgnette_scanner_manager.h

@@ -45,12 +45,13 @@ class FakeLorgnetteScannerManager final : public LorgnetteScannerManager {
           scanner_capabilities);
 
   // Sets the response returned by Scan().
-  void SetScanResponse(const base::Optional<std::string>& scan_data);
+  void SetScanResponse(
+      const base::Optional<std::vector<std::string>>& scan_data);
 
  private:
   std::vector<std::string> scanner_names_;
   base::Optional<lorgnette::ScannerCapabilities> scanner_capabilities_;
-  base::Optional<std::string> scan_data_;
+  base::Optional<std::vector<std::string>> scan_data_;
 };
 
 }  // namespace chromeos

chrome/browser/chromeos/scanning/scan_service.cc

@@ -52,14 +52,13 @@ std::string PngToJpg(const std::string& png_img) {
   return std::string(jpg_img.begin(), jpg_img.end());
 }
 
-// Saves |scanned_image| to a file after converting it if necessary. Returns
-// true if the save succeeds.
-bool SavePage(const base::FilePath& scan_to_path,
-              const mojo_ipc::FileType file_type,
-              std::string scanned_image,
-              uint32_t page_number,
-              const base::Time::Exploded& start_time) {
-  std::string filename;
+// Saves |scanned_image| to a file after converting it if necessary. Returns the
+// file path to the saved file if the save succeeds.
+base::FilePath SavePage(const base::FilePath& scan_to_path,
+                        const mojo_ipc::FileType file_type,
+                        std::string scanned_image,
+                        uint32_t page_number,
+                        const base::Time::Exploded& start_time) {
   std::string file_ext;
   switch (file_type) {
     case mojo_ipc::FileType::kPng:
@@ -69,25 +68,25 @@ bool SavePage(const base::FilePath& scan_to_path,
       file_ext = "jpg";
       scanned_image = PngToJpg(scanned_image);
       if (scanned_image.empty())
-        return false;
+        return base::FilePath();
 
       break;
     default:
       LOG(ERROR) << "Selected file type not supported.";
-      return false;
+      return base::FilePath();
   }
 
-  filename = base::StringPrintf(
+  const std::string filename = base::StringPrintf(
       "scan_%02d%02d%02d-%02d%02d%02d_%d.%s", start_time.year, start_time.month,
       start_time.day_of_month, start_time.hour, start_time.minute,
       start_time.second, page_number, file_ext.c_str());
   const auto file_path = scan_to_path.Append(filename);
   if (!base::WriteFile(file_path, scanned_image)) {
     LOG(ERROR) << "Failed to save scanned image: " << file_path.value().c_str();
-    return false;
+    return base::FilePath();
   }
 
-  return true;
+  return file_path;
 }
 
 }  // namespace
@@ -147,6 +146,7 @@ void ScanService::StartScan(
 
   base::Time::Now().LocalExplode(&start_time_);
   save_failed_ = false;
+  last_scanned_file_path_.clear();
   lorgnette_scanner_manager_->Scan(
       scanner_name, mojo::ConvertTo<lorgnette::ScanSettings>(settings),
       base::BindRepeating(&ScanService::OnProgressPercentReceived,
@@ -254,13 +254,17 @@ void ScanService::OnCancelCompleted(bool success) {
   scan_job_observer_->OnCancelComplete(success);
 }
 
-void ScanService::OnPageSaved(bool success) {
-  if (!success)
-    save_failed_ = true;
+void ScanService::OnPageSaved(const base::FilePath& saved_file_path) {
+  save_failed_ = save_failed_ || saved_file_path.empty();
+  last_scanned_file_path_ = save_failed_ ? base::FilePath() : saved_file_path;
 }
 
 void ScanService::OnAllPagesSaved(bool success) {
-  scan_job_observer_->OnScanComplete(success && !save_failed_);
+  save_failed_ = !success || save_failed_;
+  if (save_failed_)
+    last_scanned_file_path_.clear();
+
+  scan_job_observer_->OnScanComplete(!save_failed_, last_scanned_file_path_);
 }
 
 bool ScanService::FilePathSupported(const base::FilePath& file_path) {

chrome/browser/chromeos/scanning/scan_service.h

@@ -101,7 +101,7 @@ class ScanService : public scanning::mojom::ScanService, public KeyedService {
   void OnCancelCompleted(bool success);
 
   // Called once the task runner finishes saving a page of a scan.
-  void OnPageSaved(bool success);
+  void OnPageSaved(const base::FilePath& saved_file_path);
 
   // Called once the task runner finishes saving the last page of a scan.
   void OnAllPagesSaved(bool success);
@@ -142,6 +142,9 @@ class ScanService : public scanning::mojom::ScanService, public KeyedService {
   // The time a scan was started. Used in filenames when saving scanned images.
   base::Time::Exploded start_time_;
 
+  // The file path of the last page scanned in a scan job.
+  base::FilePath last_scanned_file_path_;
+
   // Task runner used to convert and save scanned images.
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
 

chrome/browser/chromeos/scanning/scan_service_unittest.cc

@@ -66,6 +66,23 @@ lorgnette::ScannerCapabilities CreateLorgnetteScannerCapabilities() {
   return caps;
 }
 
+// Returns a vector of FilePaths to mimic saved scans.
+std::vector<base::FilePath> CreateSavedScanPaths(
+    const base::FilePath& dir,
+    const base::Time::Exploded& scan_time,
+    const std::string& type,
+    int num_pages_to_scan) {
+  std::vector<base::FilePath> file_paths;
+  file_paths.reserve(num_pages_to_scan);
+  for (int i = 1; i <= num_pages_to_scan; i++) {
+    file_paths.push_back(dir.Append(base::StringPrintf(
+        "scan_%02d%02d%02d-%02d%02d%02d_%d.%s", scan_time.year, scan_time.month,
+        scan_time.day_of_month, scan_time.hour, scan_time.minute,
+        scan_time.second, i, type.c_str())));
+  }
+  return file_paths;
+}
+
 }  // namespace
 
 class FakeScanJobObserver : public mojo_ipc::ScanJobObserver {
@@ -86,7 +103,11 @@ class FakeScanJobObserver : public mojo_ipc::ScanJobObserver {
     page_complete_ = true;
   }
 
-  void OnScanComplete(bool success) override { scan_success_ = success; }
+  void OnScanComplete(bool success,
+                      const base::FilePath& last_scanned_file_path) override {
+    scan_success_ = success;
+    last_scanned_file_path_ = last_scanned_file_path;
+  }
 
   void OnCancelComplete(bool success) override {
     cancel_scan_success_ = success;
@@ -111,11 +132,17 @@ class FakeScanJobObserver : public mojo_ipc::ScanJobObserver {
   // Returns true if the cancel scan request completed successfully.
   bool cancel_scan_success() const { return cancel_scan_success_; }
 
+  // Returns the file path of the file saved last.
+  base::FilePath last_scanned_file_path() const {
+    return last_scanned_file_path_;
+  }
+
  private:
   uint32_t progress_ = 0;
   bool page_complete_ = false;
   bool scan_success_ = false;
   bool cancel_scan_success_ = false;
+  base::FilePath last_scanned_file_path_;
   mojo::Receiver<mojo_ipc::ScanJobObserver> receiver_{this};
 };
 
@@ -265,7 +292,8 @@ TEST_F(ScanServiceTest, ScanWithBadScannerId) {
 TEST_F(ScanServiceTest, ScanWithUnsupportedFilePath) {
   fake_lorgnette_scanner_manager_.SetGetScannerNamesResponse(
       {kFirstTestScannerName});
-  fake_lorgnette_scanner_manager_.SetScanResponse("TestData");
+  const std::vector<std::string> scan_data = {"TestData"};
+  fake_lorgnette_scanner_manager_.SetScanResponse(scan_data);
   auto scanners = GetScanners();
   ASSERT_EQ(scanners.size(), 1u);
 
@@ -281,7 +309,9 @@ TEST_F(ScanServiceTest, ScanWithUnsupportedFilePath) {
 TEST_F(ScanServiceTest, Scan) {
   fake_lorgnette_scanner_manager_.SetGetScannerNamesResponse(
       {kFirstTestScannerName});
-  fake_lorgnette_scanner_manager_.SetScanResponse("TestData");
+  const std::vector<std::string> scan_data = {"TestData1", "TestData2",
+                                              "TestData3"};
+  fake_lorgnette_scanner_manager_.SetScanResponse(scan_data);
   auto scanners = GetScanners();
   ASSERT_EQ(scanners.size(), 1u);
 
@@ -294,18 +324,20 @@ TEST_F(ScanServiceTest, Scan) {
   settings.scan_to_path = temp_dir_.GetPath();
   std::map<std::string, mojo_ipc::FileType> file_types = {
       {"png", mojo_ipc::FileType::kPng}, {"jpg", mojo_ipc::FileType::kJpg}};
-  base::FilePath saved_scan_path;
   for (const auto& type : file_types) {
-    saved_scan_path = temp_dir_.GetPath().Append(base::StringPrintf(
-        "scan_%02d%02d%02d-%02d%02d%02d_1.%s", scan_time.year, scan_time.month,
-        scan_time.day_of_month, scan_time.hour, scan_time.minute,
-        scan_time.second, type.first.c_str()));
-    EXPECT_FALSE(base::PathExists(saved_scan_path));
+    const std::vector<base::FilePath> saved_scan_paths = CreateSavedScanPaths(
+        temp_dir_.GetPath(), scan_time, type.first, scan_data.size());
+    for (const auto& saved_scan_path : saved_scan_paths)
+      EXPECT_FALSE(base::PathExists(saved_scan_path));
 
     settings.file_type = type.second;
     EXPECT_TRUE(Scan(scanners[0]->id, settings.Clone()));
-    EXPECT_TRUE(base::PathExists(saved_scan_path));
+    for (const auto& saved_scan_path : saved_scan_paths)
+      EXPECT_TRUE(base::PathExists(saved_scan_path));
+
     EXPECT_TRUE(fake_scan_job_observer_.scan_success());
+    EXPECT_EQ(saved_scan_paths.back(),
+              fake_scan_job_observer_.last_scanned_file_path());
   }
 }
 
@@ -313,7 +345,8 @@ TEST_F(ScanServiceTest, Scan) {
 TEST_F(ScanServiceTest, CancelScanBeforeScanCompletes) {
   fake_lorgnette_scanner_manager_.SetGetScannerNamesResponse(
       {kFirstTestScannerName});
-  fake_lorgnette_scanner_manager_.SetScanResponse("TestData");
+  const std::vector<std::string> scan_data = {"TestData"};
+  fake_lorgnette_scanner_manager_.SetScanResponse(scan_data);
   auto scanners = GetScanners();
   ASSERT_EQ(scanners.size(), 1u);
 

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

@@ -168,7 +168,7 @@ class FakeScanService {
    * @return {!Promise}
    */
   simulateScanComplete(success) {
-    this.scanJobObserverRemote_.onScanComplete(success);
+    this.scanJobObserverRemote_.onScanComplete(success, {'path': ''});
     return flushTasks();
   }
 

chromeos/components/scanning/mojom/scanning.mojom

@@ -103,8 +103,9 @@ interface ScanJobObserver {
   OnPageComplete(array<uint8> page_data);
 
   // Called when the scan is complete. |success| indicates whether the scan
-  // completed successfully.
-  OnScanComplete(bool success);
+  // completed successfully. |last_scanned_file_path| is the file path of
+  // the last page scanned in a scan job.
+  OnScanComplete(bool success, mojo_base.mojom.FilePath last_scanned_file_path);
 
   // Called when canceling the current scan job is complete. |success|
   // indicates whether the scan job was cancelled successfully.

chromeos/components/scanning/resources/scanning_app.js

@@ -171,6 +171,13 @@ Polymer({
       type: Boolean,
       value: false,
     },
+
+    /**
+     * The file path of the last scanned page of a successful scan job. Used to
+     * open the Files app with the correct file highlighted.
+     * @private {?mojoBase.mojom.FilePath}
+     */
+    lastScannedFilePath_: Object,
   },
 
   /** @override */
@@ -228,9 +235,11 @@ Polymer({
   /**
    * Overrides chromeos.scanning.mojom.ScanJobObserverInterface.
    * @param {boolean} success
+   * @param {!mojoBase.mojom.FilePath} lastScannedFilePath
    */
-  onScanComplete(success) {
+  onScanComplete(success, lastScannedFilePath) {
     if (success) {
+      this.lastScannedFilePath_ = lastScannedFilePath;
       this.setAppState_(AppState.DONE);
       return;
     }