Old zip file cannot be unmounted in some cases |
|||||||||||||
Issue descriptionChrome Version: M65 https://cs.chromium.org/chromium/src/chrome/renderer/resources/extensions/file_system_provider_custom_bindings.js?q=MassageArgumentsDefault&sq=package:chromium&l=179 > if (!verifyErrorForFailure(error)) It crashes at this line by receiving an invalid enum value for error from here: https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/zip_archiver/js/app.js?q=%22requested+before+mounting%22&sq=package:chromium&l=247 > return Promise.reject(fileSystemId + ' requested before mounting'); This was seen only with one situation. We still don't have a concrete repro steps. Here is one case which we observed: (1) Have a zip file mounted with persistent: true. This can happen only before M63, which is before we introduced the "persistent" parameters and all zip mount was remembered. (2) The zip file always gets mounted after sign in. (3) User clicks the unmount button. By some reason it fail to unmount a zip file. Since the program crashes at the above line, it cannot remove the mount information. We still don't know why it failed at step 3. One of the hypothesis is the zip file was moved to a different name or directory, but not yet confirmed. This still needs more verification, but anyways the code lines above is apparently wrong and is a bug. Background: Issue 789073 we don't mount zip files in the persistent mode (i.e. mount automaticall in the next sign in, unless user manually unmount it), but it will not affect zip files mounted before that change and still remaining in the profile.
,
Feb 14 2018
Possible workarounds would be: - if the error is caused by moving the archive, put the archive to the original name and the place. Then manually unmount it. - remove the user's profile on the device and add again. The first one worked when we saw the issue first time. The other one is not yet verified.
,
Feb 14 2018
Here is what we saw on yoshiki@'s device: - a zip file mounted after sign in, however the content of the file cannot be seen. - Clicking it in the Files app's volume list gives no response. - clicking the unmount (eject) button gave no response. - saw frequent notification message saying "operation taking time than expected" with ABORT button. ABORT button dismisses the message but it looked still running. - Renaming the zip file didn't change the situation. - After renaming the zip file to the original name, eventually succeeded to unmount it. Then it stopped.
,
Feb 14 2018
,
Feb 14 2018
,
Feb 15 2018
,
Feb 15 2018
100% Reproducible in Chromebox Panther, also available in M64 Live build (64.0.3282.144 / 10176.68.0), behaves worse in M65, slows down the whole system and can't even play any videos from youtube. please consider this as a blocker issue for M-65 release.
,
Feb 15 2018
,
Feb 15 2018
FYI, feedback report at - https://listnr.corp.google.com/report/85069418619
,
Feb 16 2018
,
Feb 20 2018
rantonysamy@, what was the exact way to reproduce it 100% if any? Did you mount the zip file before M63? Was the archive file in the original place when it the system went wrong?
,
Feb 20 2018
No special reproducibility steps, just powerwashed and restarted after installing the update from autest.
,
Feb 20 2018
Filed Issue 813780 for "system slowness after powerwash" because it's considered a different issue.
,
Feb 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6217f45cd99d80aa76542460d91e882a857067b5 commit 6217f45cd99d80aa76542460d91e882a857067b5 Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org> Date: Wed Feb 21 02:18:42 2018 Return a valid error type upon volume request before mount. Rejection from the extension should return either of the error code in ProviderError, or it will crash extension by a runtime error. Bug: 812123 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I06d858bc7cb0eb325c7e8b188cfee559d05a078e Reviewed-on: https://chromium-review.googlesource.com/926441 Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org> Reviewed-by: Yuki Awano <yawano@chromium.org> Cr-Commit-Position: refs/heads/master@{#538000} [modify] https://crrev.com/6217f45cd99d80aa76542460d91e882a857067b5/chrome/browser/resources/chromeos/zip_archiver/js/app.js
,
Feb 21 2018
I think I found one of the causes. IIUC this happens only with canary M65, and only when user had mounted a zip file before the change crrev.com/c/876705. So it has not affected stable channel yet. However, I still could not see this cause system slowness. https://cs.chromium.org/chromium/src/chrome/browser/chromeos/file_system_provider/service.cc?type=cs&sq=package:chromium&l=355 If there are any memorized zip mount info, it's remounted upon the setup process of the file system provider (Zip Archiver). It succeeds without actually communicating with the extension. By this reason the mount volume appears in the Files app. It succeeds even when the original zip file was removed. Therefore registry_->ForgetFileSystem is never called. (the other path to call it is when successfully unmounting the volume by user operation). crrev.com/c/876705 changed ensureVolumeLoaded_ function. This introduced a bug. https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/zip_archiver/js/app.js?q=ensureVolumeLoaded_&sq=package:chromium&l=245 ensureVolumeLoaded_ is required for every operation (onGetMetadataRequested, onUnmountRequested). So rejecting ensureVolumeLoaded_ will always fail any following opeartions. It also blocks unloading of the NaCl module as it makes mountProcessCounter wrong. When requests like onGetMetadataRequested is rejected, it goes to here: https://cs.chromium.org/chromium/src/chrome/browser/chromeos/file_system_provider/request_manager.cc?l=127 It notifies error to observers but returns File::OK. I haven't tracked the entire paths after this but I am guessing this is why the unmount is not triggered upon request. So the easiest fix would be to revert the change, (revert CL: https://chromium-review.googlesource.com/c/chromium/src/+/928063 ) however we'd need to reland it for the original issue it tackled. I haven't taken a look at ZIP unpacker yet.
,
Feb 22 2018
,
Feb 22 2018
I forgot to put Bug: number in the reverting changelist. https://chromium-review.googlesource.com/c/chromium/src/+/928063 Pasting it here for keeping record. ----- 928063 Revert "Zip Archiver: Disable storing mount and password info." Merged as 92067a9 This reverts commit c3169366954d82dc6b35ad6a869907cde0d714cd. Reason for revert: This caused regression Issue 812123 . This had caused that mount resuming succeeds superficially by the caller despite it actually failed, thus leaving inaccessible zip mount point. Original change's description: > Zip Archiver: Disable storing mount and password info. > > We are not restoring mount files at session restart anymore. > Therefore the mount information and passwords should not be persisted. > Othrewise it will be persisted until the user opens the same archive > again manually after a long time and the files will be opened without > requiring passwords. > > Bug: 789073 , 803752 > Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation > Change-Id: Idadcff694b20109eb7e31710b57d88400601d30a > Reviewed-on: https://chromium-review.googlesource.com/876705 > Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org> > Reviewed-by: Yuki Awano <yawano@chromium.org> > Cr-Commit-Position: refs/heads/master@{#531467} -----
,
Feb 22 2018
We would like to merge crrev.com/c/926441 and crrev.com/c/928063 to M65 for fixing this issue.
,
Feb 22 2018
This bug requires manual review: We are only 11 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 22 2018
,
Feb 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1fc14ffd3c8f02015866b848472fc300355daf87 commit 1fc14ffd3c8f02015866b848472fc300355daf87 Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org> Date: Fri Feb 23 01:36:53 2018 Return a valid error type upon volume request before mount. Rejection from the extension should return either of the error code in ProviderError, or it will crash extension by a runtime error. Bug: 812123 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I06d858bc7cb0eb325c7e8b188cfee559d05a078e Reviewed-on: https://chromium-review.googlesource.com/926441 Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org> Reviewed-by: Yuki Awano <yawano@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#538000}(cherry picked from commit 6217f45cd99d80aa76542460d91e882a857067b5) Reviewed-on: https://chromium-review.googlesource.com/933661 Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org> Cr-Commit-Position: refs/branch-heads/3325@{#564} Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369} [modify] https://crrev.com/1fc14ffd3c8f02015866b848472fc300355daf87/chrome/browser/resources/chromeos/zip_archiver/js/app.js
,
Feb 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c0dfedfad98409de991ecadf83e31f47a2a47f82 commit c0dfedfad98409de991ecadf83e31f47a2a47f82 Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org> Date: Fri Feb 23 01:42:48 2018 Revert "Zip Archiver: Disable storing mount and password info." This reverts commit c3169366954d82dc6b35ad6a869907cde0d714cd. Reason for revert: This caused regression Issue 812123 . This had caused that mount resuming succeeds superficially by the caller despite it actually failed, thus leaving inaccessible zip mount point. Original change's description: > Zip Archiver: Disable storing mount and password info. > > We are not restoring mount files at session restart anymore. > Therefore the mount information and passwords should not be persisted. > Othrewise it will be persisted until the user opens the same archive > again manually after a long time and the files will be opened without > requiring passwords. > > Bug: 789073 , 803752 > Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation > Change-Id: Idadcff694b20109eb7e31710b57d88400601d30a > Reviewed-on: https://chromium-review.googlesource.com/876705 > Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org> > Reviewed-by: Yuki Awano <yawano@chromium.org> > Cr-Commit-Position: refs/heads/master@{#531467} TBR=yawano@chromium.org,yamaguchi@chromium.org Bug: 789073 , 803752 Change-Id: I387e16717b5187389d01c5d5bc4a8b6884dcf035 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Reviewed-on: https://chromium-review.googlesource.com/928063 Reviewed-by: Naoki Fukino <fukino@chromium.org> Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org> Cr-Commit-Position: refs/heads/master@{#538338}(cherry picked from commit 92067a9e7c3867cbfb832dd8d953a4ec54a59238) Bug: 812123 Change-Id: I387e16717b5187389d01c5d5bc4a8b6884dcf035 Reviewed-on: https://chromium-review.googlesource.com/933681 Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org> Cr-Commit-Position: refs/branch-heads/3325@{#565} Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369} [modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/css/passphrase-dialog.css [modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/html/passphrase-dialog.html [modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/js/app.js [modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/js/background.js [modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/js/decompressor.js [modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/js/passphrase-dialog.js [modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/js/passphrase-manager.js [modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/manifest.json
,
Feb 23 2018
Changing to Fixed because we've reverted the cause of the issue. However we need to redo 789073 and 803752. |
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by fukino@chromium.org
, Feb 14 2018Status: Assigned (was: Unconfirmed)