New issue
Advanced search Search tips

Issue 834675 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Cannot zip a file in the Guest mode

Project Member Reported by yamaguchi@chromium.org, Apr 19 2018

Issue description

Chrome Version: 68.0.3400.0

Steps To Reproduce:
(1) enter guest mode by [Browse as Guest] in the login screen
(2) Open Files app.
(3) Create a new folder. Right-click and select "zip selection".

Expected Result:
A zip file is created there.

Actual Result:
Error message appears in the notification center. "Zip Archiver" / "Packing failed."

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


The inspector for the extension says:
> DOMException: It was determined that certain files are unsafe for access within a Web application, or that too many calls are being made on file resources.

The error comes out from unpacker.Compressor.prototype.getArchiveFile_, upon the invocation of webkitRequestFileSystem.
https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/zip_archiver/js/compressor.js?q=webkitRequestFileSystem+zip_archiver&sq=package:chromium&l=224
 

Comment 1 Deleted

Project Member

Comment 2 by bugdroid1@chromium.org, Apr 25 2018

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

commit e035dc127f0e9fdea0de71838d0e5b966b7a319d
Author: Tatsuhisa Yamaguchi <yamaguchi@google.com>
Date: Wed Apr 25 08:22:48 2018

Refactoring. Extract some blocks as private functions.

This is preparation for fixing  Issue 834675 , where we will be changing
execution paths for these procedures based on input file locations.

Test: manually tested by zipping files on My Drive
Bug:  834675 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Iac3637d46760066b18c36e799448af5d7f6cd705
Reviewed-on: https://chromium-review.googlesource.com/1027332
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Yuki Awano <yawano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553484}
[modify] https://crrev.com/e035dc127f0e9fdea0de71838d0e5b966b7a319d/chrome/browser/resources/chromeos/zip_archiver/js/compressor.js

Labels: -M-67 M-68
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 26 2018

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

commit 58759a5327c26ce1484c7765d1751e3f42a91eed
Author: Tatsuhisa Yamaguchi <yamaguchi@google.com>
Date: Thu Apr 26 11:41:05 2018

Directly write ZIP file to destination when it's not on Drive or MTP.

Existing code always wrote zip file to temporary filesystem and then
moved to the destination, as a workaround for crbug.com/785086.
It is not needed for local volumes like Downloads.
Requesting temporary file system in the guest mode caused runtime error.
We can avoid that error by this because there is no Drive volume in the
guest mode.

Bug:  834675 
Test: manually verified by zipping files in Drive, Downloads and in guest mode
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: If920f49564309a43ddcf5ca02ee8aa6baa3e6482
Reviewed-on: https://chromium-review.googlesource.com/1027394
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Yuki Awano <yawano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553985}
[modify] https://crrev.com/58759a5327c26ce1484c7765d1751e3f42a91eed/chrome/browser/resources/chromeos/zip_archiver/js/app.js
[modify] https://crrev.com/58759a5327c26ce1484c7765d1751e3f42a91eed/chrome/browser/resources/chromeos/zip_archiver/js/compressor.js
[modify] https://crrev.com/58759a5327c26ce1484c7765d1751e3f42a91eed/chrome/browser/resources/chromeos/zip_archiver/manifest.json
[modify] https://crrev.com/58759a5327c26ce1484c7765d1751e3f42a91eed/ui/file_manager/file_manager/foreground/js/file_manager_commands.js
[modify] https://crrev.com/58759a5327c26ce1484c7765d1751e3f42a91eed/ui/file_manager/file_manager/foreground/js/file_tasks.js

Labels: -M-68 Merge-Request-67 M-67
We would like to merge 2 patches, 1027332 and 1027394.
This was discovered in M68; Is it a M67-regression (bug created in M67)?

Have these been fully tested?
Cc: weifangsun@chromium.org
We confirmed this happens with current M67 release branch.
We can ask testers test canary again if that is required.
Project Member

Comment 8 by sheriffbot@chromium.org, Apr 27 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
*some* testing is required to make sure no unanticipated impact.  Has that happened?
Due to the system structure, the range of the impact is limited to the Files app and the Zip Archiver extension.
I tested the patch locally as usual before submission by zipping/unzipping some files.

Should I double check it on canary (M68) builds?
The latter patch will be merged since 68.0.3410.0. We can test on canary when the version is available.
Awaiting test results for M67 merge request.
Cc: yamaguchi@chromium.org
Owner: songsuk@chromium.org
Owner: yamaguchi@chromium.org
Verified the fix on Chrome 68.0.3416.0/ CrOS 10639.0.0 - Peppy
Thanks Song; assume no ill affects noted?  Possible to test on more than one device?
Verified the fix on Chrome 68.0.3416.0/ CrOS 10635.0.0 - Candy. No new/regression issue happens. 
Verified the fix on Chrome 68.0.3416.0/ CrOS 10640.0.0 - Reks
Labels: -Merge-Review-67 Merge-Approved-67
Approving merge to M67 Chrome OS.

Project Member

Comment 19 by bugdroid1@chromium.org, May 7 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c8e2c7cecb6a9f545910f487f83eebda495ceb0b

commit c8e2c7cecb6a9f545910f487f83eebda495ceb0b
Author: Tatsuhisa Yamaguchi <yamaguchi@google.com>
Date: Mon May 07 01:26:40 2018

Refactoring. Extract some blocks as private functions.

This is preparation for fixing  Issue 834675 , where we will be changing
execution paths for these procedures based on input file locations.

Test: manually tested by zipping files on My Drive
Bug:  834675 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Iac3637d46760066b18c36e799448af5d7f6cd705
Reviewed-on: https://chromium-review.googlesource.com/1027332
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Yuki Awano <yawano@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#553484}(cherry picked from commit e035dc127f0e9fdea0de71838d0e5b966b7a319d)
Reviewed-on: https://chromium-review.googlesource.com/1046265
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#496}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/c8e2c7cecb6a9f545910f487f83eebda495ceb0b/chrome/browser/resources/chromeos/zip_archiver/js/compressor.js

Project Member

Comment 20 by bugdroid1@chromium.org, May 7 2018

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

commit ba13529be585a5dc423a24147c46de3fcba68fc2
Author: Tatsuhisa Yamaguchi <yamaguchi@google.com>
Date: Mon May 07 01:27:42 2018

Directly write ZIP file to destination when it's not on Drive or MTP.

Existing code always wrote zip file to temporary filesystem and then
moved to the destination, as a workaround for crbug.com/785086.
It is not needed for local volumes like Downloads.
Requesting temporary file system in the guest mode caused runtime error.
We can avoid that error by this because there is no Drive volume in the
guest mode.

Bug:  834675 
Test: manually verified by zipping files in Drive, Downloads and in guest mode
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: If920f49564309a43ddcf5ca02ee8aa6baa3e6482
Reviewed-on: https://chromium-review.googlesource.com/1027394
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Yuki Awano <yawano@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#553985}(cherry picked from commit 58759a5327c26ce1484c7765d1751e3f42a91eed)
Reviewed-on: https://chromium-review.googlesource.com/1046285
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#497}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/ba13529be585a5dc423a24147c46de3fcba68fc2/chrome/browser/resources/chromeos/zip_archiver/js/app.js
[modify] https://crrev.com/ba13529be585a5dc423a24147c46de3fcba68fc2/chrome/browser/resources/chromeos/zip_archiver/js/compressor.js
[modify] https://crrev.com/ba13529be585a5dc423a24147c46de3fcba68fc2/chrome/browser/resources/chromeos/zip_archiver/manifest.json
[modify] https://crrev.com/ba13529be585a5dc423a24147c46de3fcba68fc2/ui/file_manager/file_manager/foreground/js/file_manager_commands.js
[modify] https://crrev.com/ba13529be585a5dc423a24147c46de3fcba68fc2/ui/file_manager/file_manager/foreground/js/file_tasks.js

Status: Fixed (was: Started)
The two patches were merged right after the M67 release branch is versioned 67.0.3396.36. It needs to be verified in the following version.

Comment 23 Deleted

Verified the fix in Chrome 67.0.3396.41/ CrOS 10575.32.0 - Paine, Mickey
Status: Verified (was: Fixed)
Verified the fix in Chrome 67.0.3396.41/ CrOS 10575.32.0 - Candy

Sign in to add a comment