Re-land "Don't unmount zip files when canceling password input."

The original change didn't handle the case where the injected code to
click "Cancel" button was run after the dialog page has fully loaded. In
this case, the DOMContentLoaded event has already fired.

BUG=808300

No-try: true
Change-Id: Ibf8408a7a44ef8bc8029764d31bc9a5171119cc8
Reviewed-on: https://chromium-review.googlesource.com/c/1350444
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610812}

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,123 @@ 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';
+
+  const passphraseCloseScript = `
+      function tryClose() {
+        let dialog = document.querySelector("passphrase-dialog");
+        if (dialog) {
+          dialog.shadowRoot.querySelector("#cancelButton").click();
+        }
+      }
+      document.addEventListener("DOMContentLoaded", tryClose);
+      // It's possible the page has already loaded, so try to click the close
+      // button now.
+      tryClose();
+      `;
+  var cancelPassphraseDialog = function(windowId) {
+    return sendTestMessage({
+      'name': 'runJsInAppWindow',
+      'windowId': windowId,
+      'script': passphraseCloseScript
+    });
+  };
+
+  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',