Don't unmount zip files when canceling password input.

BUG=808300

Change-Id: Ib37a53399efd57a9306e4b47630391c265825b90
Reviewed-on: https://chromium-review.googlesource.com/c/1345692
Commit-Queue: Anand Mistry <amistry@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610336}

chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

@@ -282,6 +282,7 @@ WRAPPED_INSTANTIATE_TEST_CASE_P(
                       ZipCase("zipFileOpenDownloadsShiftJIS"),
                       ZipCase("zipFileOpenDownloadsMacOs"),
                       ZipCase("zipFileOpenDownloadsWithAbsolutePaths"),
+                      ZipCase("zipFileOpenDownloadsEncryptedCancelPassphrase"),
                       ZipCase("zipFileOpenDrive").DisableDriveFs(),
                       ZipCase("zipFileOpenDrive").EnableDriveFs(),
                       ZipCase("zipFileOpenUsb"),

chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

@@ -18,7 +18,9 @@
 #include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/strings/strcat.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
 #include "chrome/browser/browser_process.h"
@@ -1600,6 +1602,71 @@ void FileManagerBrowserTestBase::OnCommand(const std::string& name,
     return;
   }
 
+  if (name == "getAppWindowId") {
+    std::string window_url;
+    ASSERT_TRUE(value.GetString("windowUrl", &window_url));
+
+    const auto& app_windows =
+        extensions::AppWindowRegistry::Get(profile())->app_windows();
+    ASSERT_FALSE(app_windows.empty());
+    *output = "none";
+    for (auto* window : app_windows) {
+      if (!window->web_contents())
+        continue;
+
+      if (window->web_contents()->GetLastCommittedURL() == window_url) {
+        *output = base::NumberToString(window->session_id().id());
+        break;
+      }
+    }
+    return;
+  }
+
+  if (name == "countAppWindows") {
+    std::string app_id;
+    ASSERT_TRUE(value.GetString("appId", &app_id));
+
+    const auto& app_windows =
+        extensions::AppWindowRegistry::Get(profile())->app_windows();
+    ASSERT_FALSE(app_windows.empty());
+    int window_count = 0;
+    for (auto* window : app_windows) {
+      if (window->extension_id() == app_id)
+        window_count++;
+    }
+    *output = base::NumberToString(window_count);
+    return;
+  }
+
+  if (name == "runJsInAppWindow") {
+    std::string window_id_str;
+    ASSERT_TRUE(value.GetString("windowId", &window_id_str));
+    int window_id = 0;
+    ASSERT_TRUE(base::StringToInt(window_id_str, &window_id));
+    std::string script;
+    ASSERT_TRUE(value.GetString("script", &script));
+
+    const auto& app_windows =
+        extensions::AppWindowRegistry::Get(profile())->app_windows();
+    ASSERT_FALSE(app_windows.empty());
+    for (auto* window : app_windows) {
+      CHECK(window);
+      if (window->session_id().id() != window_id) {
+        continue;
+      }
+
+      if (!window->web_contents())
+        break;
+
+      CHECK(window->web_contents()->GetMainFrame());
+      window->web_contents()->GetMainFrame()->ExecuteJavaScriptForTests(
+          base::UTF8ToUTF16(script));
+
+      break;
+    }
+    return;
+  }
+
   if (name == "enableTabletMode") {
     ::test::SetAndWaitForTabletMode(true);
     *output = "tabletModeEnabled";

chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.cc

@@ -96,7 +96,7 @@ void VolumeReaderJavaScriptStream::SetPassphraseAndSignal(
     const std::string& passphrase) {
   base::AutoLock al(shared_state_lock_);
   // Signal VolumeReaderJavaScriptStream::Passphrase to continue execution.
-  available_passphrase_ = passphrase;
+  available_passphrase_ = base::make_optional(passphrase);
   available_passphrase_cond_.Signal();
 }
 
@@ -212,14 +212,12 @@ void VolumeReaderJavaScriptStream::SetRequestId(const std::string& request_id) {
 }
 
 base::Optional<std::string> VolumeReaderJavaScriptStream::Passphrase() {
-  // The error is not recoverable. Once passphrase fails to be provided, it is
-  // never asked again. Note, that still users are able to retry entering the
-  // password, unless they click Cancel.
+  // Reset the state and prompt the user for a passphrase. Assume a correct
+  // passphrase from a previous request has been saved by the requestor.
   {
     base::AutoLock al(shared_state_lock_);
-    if (passphrase_error_) {
-      return {};
-    }
+    passphrase_error_ = false;
+    available_passphrase_.reset();
   }
 
   // Request the passphrase outside of the lock.
@@ -227,13 +225,10 @@ base::Optional<std::string> VolumeReaderJavaScriptStream::Passphrase() {
 
   base::AutoLock al(shared_state_lock_);
   // Wait for the passphrase from JavaScript.
-  // TODO(amistry): Handle spurious wakeups.
-  available_passphrase_cond_.Wait();
-
-  if (passphrase_error_)
-    return {};
+  while (!passphrase_error_ && !available_passphrase_)
+    available_passphrase_cond_.Wait();
 
-  return base::make_optional<std::string>(available_passphrase_);
+  return available_passphrase_;
 }
 
 void VolumeReaderJavaScriptStream::RequestChunk(int64_t length) {

chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.h

@@ -9,6 +9,7 @@
 #include <memory>
 #include <string>
 
+#include "base/optional.h"
 #include "base/synchronization/condition_variable.h"
 #include "base/synchronization/lock.h"
 #include "chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader.h"
@@ -92,7 +93,9 @@ class VolumeReaderJavaScriptStream : public VolumeReader {
   bool available_data_;  // Indicates whether any data is available.
   bool read_error_;      // Marks an error in reading from JavaScript.
 
-  std::string available_passphrase_;  // Stores a passphrase from JavaScript.
+  // Stores a passphrase from JavaScript. Stored as a base::Optional<> because
+  // the user could have entered an empty passphrase.
+  base::Optional<std::string> available_passphrase_;
   bool passphrase_error_;  // Marks an error in getting the passphrase.
 
   // The shared_state_lock_ is used to protect members which are accessed by

chrome/browser/resources/chromeos/zip_archiver/js/app.js

@@ -188,13 +188,15 @@ unpacker.app = {
     if (chrome.extension.inIncognitoContext)
       return;
 
-    chrome.storage.local.get([unpacker.app.STORAGE_KEY], function(result) {
-      if (result[unpacker.app.STORAGE_KEY]) {
-        chrome.storage.local.clear(function() {
-          console.info('Cleaned up archive mount info from older versions.');
-        });
-      }
-    });
+    if (chrome.storage && chrome.storage.local) {
+      chrome.storage.local.get([unpacker.app.STORAGE_KEY], function(result) {
+        if (result[unpacker.app.STORAGE_KEY]) {
+          chrome.storage.local.clear(function() {
+            console.info('Cleaned up archive mount info from older versions.');
+          });
+        }
+      });
+    }
   },
 
   /**

chrome/browser/resources/chromeos/zip_archiver/js/decompressor.js

@@ -307,9 +307,5 @@ unpacker.Decompressor.prototype.readPassphrase_ = function(data, requestId) {
         this.naclModule_.postMessage(
             unpacker.request.createReadPassphraseErrorResponse(
                 this.fileSystemId_, requestId));
-        // TODO(mtomasz): Instead of unmounting just let the current operation
-        // fail and ask for password for another files. This is however
-        // impossible for now due to a bug in minizip.
-        unpacker.app.unmountVolume(this.fileSystemId_, true);
       }.bind(this));
 };

ui/file_manager/integration_tests/file_manager/zip_files.js

@@ -158,6 +158,112 @@ testcase.zipFileOpenDownloadsWithAbsolutePaths = function() {
   ]);
 };
 
+/**
+ * Tests encrypted zip file open, and canceling the passphrase dialog.
+ */
+testcase.zipFileOpenDownloadsEncryptedCancelPassphrase = function() {
+  let appId;
+
+  const zipArchiverAppId = 'dmboannefpncccogfdikhmhpmdnddgoe';
+  const zipArchiverPassphraseDialogUrl =
+      'chrome-extension://dmboannefpncccogfdikhmhpmdnddgoe/html/passphrase.html';
+
+  var cancelPassphraseDialog = function(windowId) {
+    return sendTestMessage({
+      'name': 'runJsInAppWindow',
+      'windowId': windowId,
+      'script':
+          'document.addEventListener("DOMContentLoaded", function() {document.querySelector("passphrase-dialog").shadowRoot.querySelector("#cancelButton").click();});'
+    });
+  };
+
+  var waitForAllPassphraseWindowsClosed = function() {
+    const caller = getCaller();
+
+    const passphraseWindowCountCommand = {
+      'name': 'countAppWindows',
+      'appId': zipArchiverAppId
+    };
+
+    const getPassphraseWindowIdCommand = {
+      'name': 'getAppWindowId',
+      'windowUrl': zipArchiverPassphraseDialogUrl
+    };
+
+    return repeatUntil(function() {
+      return sendTestMessage(passphraseWindowCountCommand).then((result) => {
+        if (result == 0)
+          return true;
+        return sendTestMessage(getPassphraseWindowIdCommand)
+            .then((result) => {
+              if (result == 'none')
+                return true;
+              return cancelPassphraseDialog(result);
+            })
+            .then(function() {
+              return pending(caller, 'waitForAllPassphraseWindowsClosed');
+            });
+      });
+    });
+  };
+
+  StepsRunner.run([
+    // Open Files app on Downloads containing a zip file.
+    function() {
+      setupAndWaitUntilReady(
+          null, RootPath.DOWNLOADS, this.next, [ENTRIES.zipArchiveEncrypted],
+          []);
+    },
+    // Select the zip file.
+    function(result) {
+      appId = result.windowId;
+      remoteCall.callRemoteTestUtil('selectFile', appId, ['encrypted.zip'])
+          .then(this.next);
+    },
+    // Press the Enter key.
+    function(result) {
+      chrome.test.assertTrue(!!result, 'selectFile failed');
+      const key = ['#file-list', 'Enter', false, false, false];
+      remoteCall.callRemoteTestUtil('fakeKeyDown', appId, key, this.next);
+    },
+    // Check: the zip file content should be shown (unzip).
+    function(result) {
+      chrome.test.assertTrue(!!result, 'fakeKeyDown failed');
+      const files = getUnzippedFileListRowEntries();
+      remoteCall.waitForFiles(appId, files, {'ignoreLastModifiedTime': true})
+          .then(this.next);
+    },
+    // Select the text file in the ZIP file.
+    function(result) {
+      remoteCall.callRemoteTestUtil('selectFile', appId, ['text.txt'])
+          .then(this.next);
+    },
+    // Press the Enter key.
+    function(result) {
+      chrome.test.assertTrue(!!result, 'selectFile failed');
+      const key = ['#file-list', 'Enter', false, false, false];
+      remoteCall.callRemoteTestUtil('fakeKeyDown', appId, key, this.next);
+    },
+    // Wait for the external passphrase dialog window to appear.
+    function(result) {
+      chrome.test.assertTrue(!!result, 'fakeKeyDown failed');
+      waitForAppWindow(zipArchiverPassphraseDialogUrl).then(this.next);
+    },
+    // Close the dialog by pressing the 'Cancel' button. Repeat for any new
+    // dialogs that pop up.
+    function(result) {
+      waitForAllPassphraseWindowsClosed().then(this.next);
+    },
+    // Check: the zip file content should still be shown.
+    function(result) {
+      chrome.test.assertTrue(!!result, 'fakeKeyDown failed');
+      const files = getUnzippedFileListRowEntries();
+      remoteCall.waitForFiles(appId, files, {'ignoreLastModifiedTime': true})
+          .then(this.next);
+    },
+  ]);
+};
+
 /**
  * Tests zip file open (aka unzip) from Google Drive.
  */

ui/file_manager/integration_tests/test_util.js

@@ -192,6 +192,43 @@ function sendBrowserTestCommand(command, callback, opt_debug) {
   });
 }
 
+/**
+ * Waits for an app window with the URL |windowUrl|.
+ * @param {string} windowUrl URL of the app window to wait for.
+ * @return {Promise} Promise to be fulfilled with the window ID of the
+ *     app window.
+ */
+function waitForAppWindow(windowUrl) {
+  const caller = getCaller();
+  const command = {'name': 'getAppWindowId', 'windowUrl': windowUrl};
+  return repeatUntil(function() {
+    return sendTestMessage(command).then((result) => {
+      if (result == 'none')
+        return pending(caller, 'getAppWindowId ' + windowUrl);
+      return result;
+    });
+  });
+}
+
+/**
+ * Wait for the count of windows for app |appId| to equal |expectedCount|.
+ * @param{string} appId ID of the app to count windows for.
+ * @param{number} expectedCount Number of app windows to wait for.
+ * @return {Promise} Promise to be fulfilled when the number of app windows
+ *     equals |expectedCount|.
+ */
+function waitForAppWindowCount(appId, expectedCount) {
+  const caller = getCaller();
+  const command = {'name': 'countAppWindows', 'appId': appId};
+  return repeatUntil(function() {
+    return sendTestMessage(command).then((result) => {
+      if (result != expectedCount)
+        return pending(caller, 'waitForAppWindowCount ' + appId + ' ' + result);
+      return true;
+    });
+  });
+}
+
 /**
  * Adds the givin entries to the target volume(s).
  * @param {Array<string>} volumeNames Names of target volumes.
@@ -704,6 +741,17 @@ var ENTRIES = {
     typeText: 'Zip archive'
   }),
 
+  zipArchiveEncrypted: new TestEntryInfo({
+    type: EntryType.FILE,
+    sourceFileName: 'encrypted.zip',
+    targetPath: 'encrypted.zip',
+    mimeType: 'application/x-zip',
+    lastModifiedTime: 'Jan 1, 2014, 1:00 AM',
+    nameText: 'encrypted.zip',
+    sizeText: '589 bytes',
+    typeText: 'Zip archive'
+  }),
+
   debPackage: new TestEntryInfo({
     type: EntryType.FILE,
     sourceFileName: 'package.deb',