New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 808300 link

Starred by 2 users
Status: Fixed
Owner:
amistry@chromium.org
Closed: Dec 19
Cc:
amistry@chromium.org
█ mtomasz@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Zip Archiver: file unmounted when cancelling passphrase input

Project Member Reported by yamaguchi@chromium.org, Feb 2 2018 
Chrome Version: 66.0.3335.0

Steps To Reproduce:
(1) Create a zip file containing 1 or more encrypted text file.
(2) Open the zip file in the Files app. See the zip file appears as a volume on LHS.
(3) Open a text file inside it.
(4) Click "CANCEL" button in the passphrase input dialog.

Expected Result:
The mounted ZIP file volume stays.

Actual Result:
The mounted ZIP file volume unmounts.

How frequently does this problem reproduce? (Always, sometimes, hard to
reproduce?)
100%

Note:
If the contents of the archive contains image files, the flow will be different due to automated thumbnail loading.

(2') Open the zip file in the Files app. See the zip file appears as a volume on LHS, and also it prompts to input passphrase of the first encrypted file.
(3') Click "CANCEL" button.
Then the archive volume unmounts.

This behavior may appear reasonable UX, though I guess this is not WAI.

Comment 1 by yamaguchi@chromium.org, Feb 2 2018 

This is same behavior as M63 or before (i.e. "ZIP unpacker" instead of "Zip Archiver"; before #zip-archiver-unpacker flag is enabled).

Comment 2 by yamaguchi@chromium.org, Feb 2 2018

Owner: ----
Status: Untriaged (was: Assigned)

Comment 3 by sashab@chromium.org, Feb 22 2018

Labels: CrOS-FilesApp-Zip

Comment 4 by yamaguchi@chromium.org, Feb 23 2018

Cc: mtomasz@chromium.org
This is noted as TODO here:
https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/zip_archiver/js/decompressor.js?type=cs&q=getPassphrase+file:zip_archiver&sq=package:chromium&l=310
        // 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.

Comment 5 by sashab@chromium.org, Feb 28 2018

Labels: -CrOS-FilesApp-Zip CrOSFilesFeature-Zip

Comment 6 by sashab@chromium.org, Mar 15 2018

Status: Available (was: Untriaged)

Comment 7 by slangley@chromium.org, Oct 31

Cc: amistry@chromium.org

Comment 8 by amistry@chromium.org, Nov 5

Labels: Files-Fixit-2018
Should be fixable post minizip-uprev.

Comment 9 by amistry@chromium.org, Nov 19

Status: Started (was: Available)

Comment 10 by amistry@chromium.org, Nov 20

Owner: amistry@chromium.org
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 22 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/97df93d4a51777525091974a438f9a5e8581e0e8

commit 97df93d4a51777525091974a438f9a5e8581e0e8
Author: Anand K. Mistry <amistry@chromium.org>
Date: Thu Nov 22 07:39:51 2018

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}
[modify] https://crrev.com/97df93d4a51777525091974a438f9a5e8581e0e8/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/97df93d4a51777525091974a438f9a5e8581e0e8/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/97df93d4a51777525091974a438f9a5e8581e0e8/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.cc
[modify] https://crrev.com/97df93d4a51777525091974a438f9a5e8581e0e8/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.h
[modify] https://crrev.com/97df93d4a51777525091974a438f9a5e8581e0e8/chrome/browser/resources/chromeos/zip_archiver/js/app.js
[modify] https://crrev.com/97df93d4a51777525091974a438f9a5e8581e0e8/chrome/browser/resources/chromeos/zip_archiver/js/decompressor.js
[add] https://crrev.com/97df93d4a51777525091974a438f9a5e8581e0e8/chrome/test/data/chromeos/file_manager/encrypted.zip
[modify] https://crrev.com/97df93d4a51777525091974a438f9a5e8581e0e8/ui/file_manager/integration_tests/file_manager/zip_files.js
[modify] https://crrev.com/97df93d4a51777525091974a438f9a5e8581e0e8/ui/file_manager/integration_tests/test_util.js

Comment 12 by amistry@chromium.org, Nov 22

Status: Fixed (was: Started)
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 23 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/08b1af0683349ac3e4158929f18654e051031d0c

commit 08b1af0683349ac3e4158929f18654e051031d0c
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Fri Nov 23 19:36:34 2018

Revert "Don't unmount zip files when canceling password input."

This reverts commit 97df93d4a51777525091974a438f9a5e8581e0e8.

Reason for revert: added test was falky, see  crbug.com/908071 

Original change's description:
> 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}

TBR=noel@chromium.org,amistry@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

NOTRY=true

Bug:  808300 
Change-Id: If2c4d54fb70b57d33aeec55adc3404836345d6e0
Reviewed-on: https://chromium-review.googlesource.com/c/1349752
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610657}
[modify] https://crrev.com/08b1af0683349ac3e4158929f18654e051031d0c/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/08b1af0683349ac3e4158929f18654e051031d0c/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/08b1af0683349ac3e4158929f18654e051031d0c/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.cc
[modify] https://crrev.com/08b1af0683349ac3e4158929f18654e051031d0c/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.h
[modify] https://crrev.com/08b1af0683349ac3e4158929f18654e051031d0c/chrome/browser/resources/chromeos/zip_archiver/js/app.js
[modify] https://crrev.com/08b1af0683349ac3e4158929f18654e051031d0c/chrome/browser/resources/chromeos/zip_archiver/js/decompressor.js
[delete] https://crrev.com/b7d46787aca0dfa23e905cf6eb1cbd04e3186c20/chrome/test/data/chromeos/file_manager/encrypted.zip
[modify] https://crrev.com/08b1af0683349ac3e4158929f18654e051031d0c/ui/file_manager/integration_tests/file_manager/zip_files.js
[modify] https://crrev.com/08b1af0683349ac3e4158929f18654e051031d0c/ui/file_manager/integration_tests/test_util.js

Comment 14 by mlamouri@chromium.org, Nov 23

Status: Started (was: Fixed)
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 26 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/05522106023c6889946da0db35efae1a8151e756

commit 05522106023c6889946da0db35efae1a8151e756
Author: Anand K. Mistry <amistry@chromium.org>
Date: Mon Nov 26 11:23:03 2018

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}
[modify] https://crrev.com/05522106023c6889946da0db35efae1a8151e756/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/05522106023c6889946da0db35efae1a8151e756/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/05522106023c6889946da0db35efae1a8151e756/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.cc
[modify] https://crrev.com/05522106023c6889946da0db35efae1a8151e756/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.h
[modify] https://crrev.com/05522106023c6889946da0db35efae1a8151e756/chrome/browser/resources/chromeos/zip_archiver/js/app.js
[modify] https://crrev.com/05522106023c6889946da0db35efae1a8151e756/chrome/browser/resources/chromeos/zip_archiver/js/decompressor.js
[add] https://crrev.com/05522106023c6889946da0db35efae1a8151e756/chrome/test/data/chromeos/file_manager/encrypted.zip
[modify] https://crrev.com/05522106023c6889946da0db35efae1a8151e756/ui/file_manager/integration_tests/file_manager/zip_files.js
[modify] https://crrev.com/05522106023c6889946da0db35efae1a8151e756/ui/file_manager/integration_tests/test_util.js
Project Member

Comment 16 by bugdroid1@chromium.org, Nov 26 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b113131f8acd94191c7c6be067b19885fffe1f4f

commit b113131f8acd94191c7c6be067b19885fffe1f4f
Author: Rune Lillesveen <futhark@chromium.org>
Date: Mon Nov 26 15:00:48 2018

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

This reverts commit 05522106023c6889946da0db35efae1a8151e756.

Reason for revert: FindIt claims this is the reason for a failing
SuggestAppDialog/FilesAppBrowserTest.Test/suggestAppDialog on CROS 
MSAN:

https://findit-for-me.appspot.com/waterfall/failure?url=https://build.chromium.org/p/chromium.memory/builders/Linux%20ChromiumOS%20MSan%20Tests/builds/9831

Original change's description:
> 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}

TBR=noel@chromium.org,amistry@chromium.org

Change-Id: I1cd48d0aaf953cc3a635a016de129b20bad865f7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  808300 
Reviewed-on: https://chromium-review.googlesource.com/c/1350912
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610846}
[modify] https://crrev.com/b113131f8acd94191c7c6be067b19885fffe1f4f/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/b113131f8acd94191c7c6be067b19885fffe1f4f/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/b113131f8acd94191c7c6be067b19885fffe1f4f/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.cc
[modify] https://crrev.com/b113131f8acd94191c7c6be067b19885fffe1f4f/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.h
[modify] https://crrev.com/b113131f8acd94191c7c6be067b19885fffe1f4f/chrome/browser/resources/chromeos/zip_archiver/js/app.js
[modify] https://crrev.com/b113131f8acd94191c7c6be067b19885fffe1f4f/chrome/browser/resources/chromeos/zip_archiver/js/decompressor.js
[delete] https://crrev.com/0c8d379b43322370ad02d529be7657ce302fc790/chrome/test/data/chromeos/file_manager/encrypted.zip
[modify] https://crrev.com/b113131f8acd94191c7c6be067b19885fffe1f4f/ui/file_manager/integration_tests/file_manager/zip_files.js
[modify] https://crrev.com/b113131f8acd94191c7c6be067b19885fffe1f4f/ui/file_manager/integration_tests/test_util.js
Project Member

Comment 17 by bugdroid1@chromium.org, Dec 18 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76aa8ffd7798182903f421d81b9113de7a9004c5

commit 76aa8ffd7798182903f421d81b9113de7a9004c5
Author: Anand K. Mistry <amistry@chromium.org>
Date: Tue Dec 18 03:14:43 2018

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

The first reland was reverted attempting to fix some failing
tests. This reland changes the test to use async/await, but is
otherwise identical to the previous CLs.

BUG= 808300 

Change-Id: I3a98eed341d14378bc732546387196f83d6b9040
Reviewed-on: https://chromium-review.googlesource.com/c/1379613
Commit-Queue: Anand Mistry <amistry@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617360}
[modify] https://crrev.com/76aa8ffd7798182903f421d81b9113de7a9004c5/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/76aa8ffd7798182903f421d81b9113de7a9004c5/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/76aa8ffd7798182903f421d81b9113de7a9004c5/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.cc
[modify] https://crrev.com/76aa8ffd7798182903f421d81b9113de7a9004c5/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.h
[modify] https://crrev.com/76aa8ffd7798182903f421d81b9113de7a9004c5/chrome/browser/resources/chromeos/zip_archiver/js/app.js
[modify] https://crrev.com/76aa8ffd7798182903f421d81b9113de7a9004c5/chrome/browser/resources/chromeos/zip_archiver/js/decompressor.js
[add] https://crrev.com/76aa8ffd7798182903f421d81b9113de7a9004c5/chrome/test/data/chromeos/file_manager/encrypted.zip
[modify] https://crrev.com/76aa8ffd7798182903f421d81b9113de7a9004c5/ui/file_manager/integration_tests/file_manager/zip_files.js
[modify] https://crrev.com/76aa8ffd7798182903f421d81b9113de7a9004c5/ui/file_manager/integration_tests/test_util.js

Comment 18 by amistry@chromium.org, Dec 18

Status: Fixed (was: Started)

Comment 19 by noel@chromium.org, Dec 19

Status: Started (was: Fixed)
The password test is right on the DEBUG bot time-out limit.

Test is always TIMEOUT PASS in a test run: TIMEOUT when all tests are running and the bot is loaded, then PASS when it re-tried at the end of the run, when the bot is doing nothing to do and no load.

We should disable in DEBUG for now.  

Must be some way for us to tell the bot to give us more time in test, but I don't recall what flag or switch does that ...
Project Member

Comment 20 by bugdroid1@chromium.org, Dec 19 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a297decf71f4009494c4975b79a83398f6f6d6e

commit 6a297decf71f4009494c4975b79a83398f6f6d6e
Author: Anand K. Mistry <amistry@chromium.org>
Date: Wed Dec 19 04:37:33 2018

Disable zipFileOpenDownloadsEncryptedCancelPassphrase test in debug.

The test is close to, and often hitting the 45 second test timeout.

BUG= 808300 

Change-Id: I7de4f0685fad03846ba60cc6859a5978b2654ed4
Reviewed-on: https://chromium-review.googlesource.com/c/1382661
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617729}
[modify] https://crrev.com/6a297decf71f4009494c4975b79a83398f6f6d6e/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

Comment 21 by noel@chromium.org, Dec 19

Status: Fixed (was: Started)
►

Sign in to add a comment