Cannot drag file to Linux Files until opened |
||||||||||
Issue descriptionChrome Version: 68.0.3438.0 OS Version: 10715.0.0 What steps will reproduce the problem? 1. Optional: start Crostini by launching Terminal 2. Open Files.app 3. Drag a file from Downloads to Linux Files Expected: Linux Files connects to the container and copies the file over. Actual: File fails to copy until after you have opened Linux Files the first time.
,
Jun 12 2018
Currently if you drag and hold for 2s, it will start the container and you can then drop. I am investigating about making the container start immediately. This change should be simple enough. When starting immediately, you would still have to wait until the container is started and mounted. This typically takes a few seconds for me. It would be a bit tricky to allow immediate drop which then mounts the container and then copies the file once the container is started and mounted since the current code relies on getting volume information for the destination prior to starting the copy.
,
Jun 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/df70b8889254a236a74e141c84cd41fb1c607b4b commit df70b8889254a236a74e141c84cd41fb1c607b4b Author: Joel Hockey <joelhockey@chromium.org> Date: Tue Jun 12 05:06:42 2018 Refactor crostini mounting code into test.mountCrostini to make it reusable Bug: 846917 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: Ifd4b52472de613ed5ef1d75218ce0cf02b25fb1d Reviewed-on: https://chromium-review.googlesource.com/1096576 Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Commit-Queue: Joel Hockey <joelhockey@chromium.org> Cr-Commit-Position: refs/heads/master@{#566320} [modify] https://crrev.com/df70b8889254a236a74e141c84cd41fb1c607b4b/ui/file_manager/file_manager/test/crostini.js [modify] https://crrev.com/df70b8889254a236a74e141c84cd41fb1c607b4b/ui/file_manager/file_manager/test/js/chrome_file_manager.js [modify] https://crrev.com/df70b8889254a236a74e141c84cd41fb1c607b4b/ui/file_manager/file_manager/test/js/test_util.js
,
Jun 13 2018
,
Jun 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/28744569f6d88d923529366ac2e18bbdda830680 commit 28744569f6d88d923529366ac2e18bbdda830680 Author: Joel Hockey <joelhockey@chromium.org> Date: Wed Jun 13 02:28:07 2018 FilesApp mount crostini immediately for drag and drop Currently FileTransferController starts a 2s timer to change to a directory if you drag and then hover over a root. This happens on the dragenter event. The dragleave event will cancel the timer if it has not already fired. For crostini, we want to change to the directory immediately. Test fires dragenter and then dragleave events immediately after each other. Usually this would cause no change if the events are received within 2s of each other. We test for crostini that even if these events are fired right after each other, we still change directory. Bug: 846917 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I4f0f463ff1c4bc164328a2764ff235ee399d6e4c Reviewed-on: https://chromium-review.googlesource.com/1096801 Commit-Queue: Joel Hockey <joelhockey@chromium.org> Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#566693} [modify] https://crrev.com/28744569f6d88d923529366ac2e18bbdda830680/chrome/app/chromeos_strings.grdp [modify] https://crrev.com/28744569f6d88d923529366ac2e18bbdda830680/chrome/browser/chromeos/extensions/file_manager/private_api_strings.cc [modify] https://crrev.com/28744569f6d88d923529366ac2e18bbdda830680/ui/file_manager/file_manager/foreground/js/empty_folder_controller.js [modify] https://crrev.com/28744569f6d88d923529366ac2e18bbdda830680/ui/file_manager/file_manager/foreground/js/file_transfer_controller.js [modify] https://crrev.com/28744569f6d88d923529366ac2e18bbdda830680/ui/file_manager/file_manager/test/crostini.js [modify] https://crrev.com/28744569f6d88d923529366ac2e18bbdda830680/ui/file_manager/file_manager/test/js/chrome_file_manager.js [modify] https://crrev.com/28744569f6d88d923529366ac2e18bbdda830680/ui/file_manager/file_manager/test/js/strings.js [modify] https://crrev.com/28744569f6d88d923529366ac2e18bbdda830680/ui/file_manager/file_manager/test/js/test_util.js
,
Jun 13 2018
Tom, I've now got it that as soon as you drag over 'Linux Files' it will start/mount container if not already running. However you can't drop the files right away - you have to wait a few seconds for the mounting to complete. Let me know if this fix is OK.
,
Jun 21 2018
,
Jul 24
Any updates to share here? Is the current implementation OK?
,
Jul 28
+mcirimele, sbroslawsky Sorry for the delay. While this is an improvement, I worry that users will still expect it to copy the file if they immediately release it on top of Linux Files the first time. Is there some signal we can send to Files.app so that it starts mounting when Crostini is started instead of when the user first interacts with "Linux files"? Or is there some way we can let the user know to wait to drop the file?
,
Jul 30
Hi there, A few thoughts for context. 1. The default operation associated with dragging a file to another root or folder is for it to move, not copy. There are plenty of cases where we can't do that right now so no worries for now but Crostini should consider whether this is something to allow in the future. I would love for it to always move on default. The user can use a keyboard modifier (believe it's SHIFT) to change from move to copy. 2. I like Tom's suggestion of mounting Crostini without waiting for the user to interact with "Linux files". This must be similar to what we do for other drives, like Google Drive? 3. If we can't do that, how about this? Rather than having the user adopt to the wait time, can we "fake" the operation and just delay it? We make it looks like we allow the drop (chip with green plus sign for ocpy), and give feedback that it's copying, even though some of that time is spent waiting for the mount?
,
Aug 1
Moving to M70. Also moving to P2 since the worst case is that the file isn't copied (which is annoying, but not destructive). Will mount SSHFS as soon as container starts up. @mcirimele/sbroslawsky, let's discuss copy vs move.
,
Aug 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/047d1ba3bcb48861f937b1518d75213979a14488 commit 047d1ba3bcb48861f937b1518d75213979a14488 Author: Joel Hockey <joelhockey@chromium.org> Date: Thu Aug 02 04:42:15 2018 Track calls to GetContainerSshKeys in FakeConciergeClient. This will be used in a followup CL for CrostiniManager unit tests. Bug: 846917 Change-Id: I4938a4bf75b986adb4a2dc283ffac7d0509e1c5c Reviewed-on: https://chromium-review.googlesource.com/1158319 Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org> Commit-Queue: Joel Hockey <joelhockey@chromium.org> Cr-Commit-Position: refs/heads/master@{#580068} [modify] https://crrev.com/047d1ba3bcb48861f937b1518d75213979a14488/chromeos/dbus/fake_concierge_client.cc [modify] https://crrev.com/047d1ba3bcb48861f937b1518d75213979a14488/chromeos/dbus/fake_concierge_client.h
,
Aug 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/64ab6403e0d694c98d4da8a2dd961907e1ddf056 commit 64ab6403e0d694c98d4da8a2dd961907e1ddf056 Author: Joel Hockey <joelhockey@chromium.org> Date: Thu Aug 02 06:47:16 2018 CrOS crostini: sshfs mount immediately when container starts Perform sshfs mount to termina/penguin immediately when container starts. Mounting code has moved from the chrome.fileManagerPrivate extension functions into CrostiniManager. This change will make FilesApp more responsive to users who are likely to use terminal or some other crostini app before using FilesApp. Bug: 846917 Change-Id: I9fc246e45e54390e7a75f15a5571a24218cf2d5c Reviewed-on: https://chromium-review.googlesource.com/1158317 Reviewed-by: Nicholas Verne <nverne@chromium.org> Commit-Queue: Joel Hockey <joelhockey@chromium.org> Cr-Commit-Position: refs/heads/master@{#580094} [modify] https://crrev.com/64ab6403e0d694c98d4da8a2dd961907e1ddf056/chrome/app/chromeos_strings.grdp [modify] https://crrev.com/64ab6403e0d694c98d4da8a2dd961907e1ddf056/chrome/browser/chromeos/crostini/crostini_manager.cc [modify] https://crrev.com/64ab6403e0d694c98d4da8a2dd961907e1ddf056/chrome/browser/chromeos/crostini/crostini_manager.h [modify] https://crrev.com/64ab6403e0d694c98d4da8a2dd961907e1ddf056/chrome/browser/chromeos/crostini/crostini_manager_unittest.cc [modify] https://crrev.com/64ab6403e0d694c98d4da8a2dd961907e1ddf056/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc [modify] https://crrev.com/64ab6403e0d694c98d4da8a2dd961907e1ddf056/chrome/browser/chromeos/extensions/file_manager/private_api_misc.h [modify] https://crrev.com/64ab6403e0d694c98d4da8a2dd961907e1ddf056/chrome/browser/chromeos/file_manager/path_util.cc [modify] https://crrev.com/64ab6403e0d694c98d4da8a2dd961907e1ddf056/chrome/browser/chromeos/file_manager/path_util.h [modify] https://crrev.com/64ab6403e0d694c98d4da8a2dd961907e1ddf056/chrome/browser/ui/views/crostini/crostini_installer_view.cc [modify] https://crrev.com/64ab6403e0d694c98d4da8a2dd961907e1ddf056/chrome/browser/ui/views/crostini/crostini_installer_view.h
,
Aug 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1c3b10d1bacd74ecfc6cfb77e4e93b9f1fddc5c4 commit 1c3b10d1bacd74ecfc6cfb77e4e93b9f1fddc5c4 Author: Christos Froussios <cfroussios@chromium.org> Date: Thu Aug 02 12:40:08 2018 Revert "CrOS crostini: sshfs mount immediately when container starts" This reverts commit 64ab6403e0d694c98d4da8a2dd961907e1ddf056. Reason for revert: Suspected of making CrostiniInstallerViewBrowserTest.InstallFlow very flaky on linux-chromeos-rel Original change's description: > CrOS crostini: sshfs mount immediately when container starts > > Perform sshfs mount to termina/penguin immediately when container > starts. Mounting code has moved from the chrome.fileManagerPrivate > extension functions into CrostiniManager. > > This change will make FilesApp more responsive to users who are > likely to use terminal or some other crostini app before using FilesApp. > > Bug: 846917 > Change-Id: I9fc246e45e54390e7a75f15a5571a24218cf2d5c > Reviewed-on: https://chromium-review.googlesource.com/1158317 > Reviewed-by: Nicholas Verne <nverne@chromium.org> > Commit-Queue: Joel Hockey <joelhockey@chromium.org> > Cr-Commit-Position: refs/heads/master@{#580094} TBR=joelhockey@chromium.org,nverne@chromium.org Change-Id: Ib00e4417faaa482d62f03bed708dd46b1e0f7902 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 846917, 870289 Reviewed-on: https://chromium-review.googlesource.com/1160541 Reviewed-by: Christos Froussios <cfroussios@chromium.org> Commit-Queue: Christos Froussios <cfroussios@chromium.org> Cr-Commit-Position: refs/heads/master@{#580145} [modify] https://crrev.com/1c3b10d1bacd74ecfc6cfb77e4e93b9f1fddc5c4/chrome/app/chromeos_strings.grdp [modify] https://crrev.com/1c3b10d1bacd74ecfc6cfb77e4e93b9f1fddc5c4/chrome/browser/chromeos/crostini/crostini_manager.cc [modify] https://crrev.com/1c3b10d1bacd74ecfc6cfb77e4e93b9f1fddc5c4/chrome/browser/chromeos/crostini/crostini_manager.h [modify] https://crrev.com/1c3b10d1bacd74ecfc6cfb77e4e93b9f1fddc5c4/chrome/browser/chromeos/crostini/crostini_manager_unittest.cc [modify] https://crrev.com/1c3b10d1bacd74ecfc6cfb77e4e93b9f1fddc5c4/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc [modify] https://crrev.com/1c3b10d1bacd74ecfc6cfb77e4e93b9f1fddc5c4/chrome/browser/chromeos/extensions/file_manager/private_api_misc.h [modify] https://crrev.com/1c3b10d1bacd74ecfc6cfb77e4e93b9f1fddc5c4/chrome/browser/chromeos/file_manager/path_util.cc [modify] https://crrev.com/1c3b10d1bacd74ecfc6cfb77e4e93b9f1fddc5c4/chrome/browser/chromeos/file_manager/path_util.h [modify] https://crrev.com/1c3b10d1bacd74ecfc6cfb77e4e93b9f1fddc5c4/chrome/browser/ui/views/crostini/crostini_installer_view.cc [modify] https://crrev.com/1c3b10d1bacd74ecfc6cfb77e4e93b9f1fddc5c4/chrome/browser/ui/views/crostini/crostini_installer_view.h
,
Aug 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/114391232f315969b5f1c5cd16ee670e4a8a56a0 commit 114391232f315969b5f1c5cd16ee670e4a8a56a0 Author: Joel Hockey <joelhockey@chromium.org> Date: Tue Aug 07 01:56:46 2018 Call SetCrostiniUIAllowedForTesting in crostini browser tests to fix dcheck Bug: 846917 Change-Id: Ica3471cb3931a7e9368c386aca012674242e60cc Reviewed-on: https://chromium-review.googlesource.com/1164146 Commit-Queue: Joel Hockey <joelhockey@chromium.org> Reviewed-by: Nicholas Verne <nverne@chromium.org> Cr-Commit-Position: refs/heads/master@{#581091} [modify] https://crrev.com/114391232f315969b5f1c5cd16ee670e4a8a56a0/chrome/browser/ui/views/crostini/crostini_browser_test_util.cc [modify] https://crrev.com/114391232f315969b5f1c5cd16ee670e4a8a56a0/chrome/browser/ui/views/crostini/crostini_browser_test_util.h
,
Aug 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b785be60dcb4387d89d725db914d2b70be02dc92 commit b785be60dcb4387d89d725db914d2b70be02dc92 Author: Joel Hockey <joelhockey@chromium.org> Date: Wed Aug 08 02:51:31 2018 Reland "CrOS crostini: sshfs mount immediately when container starts" This is a reland of 64ab6403e0d694c98d4da8a2dd961907e1ddf056 Updates: * Fixed CrostiniInstallerView::SetupResult to not change kErrorOffline value. * VolumeManager::AddSshfsCrostiniVolume ignores if crostini is already mounted. * CrostiniInstallerViewBrowserTest is using a WaitingDiskMountManagerObserver to manage the run loop and ensure test runs all the way through new mounting code. * Added a custom mount point manager to FakeCrosDisksClient to fake the sshfs mounting in CrostiniInstallerViewBrowserTest. Original change's description: > CrOS crostini: sshfs mount immediately when container starts > > Perform sshfs mount to termina/penguin immediately when container > starts. Mounting code has moved from the chrome.fileManagerPrivate > extension functions into CrostiniManager. > > This change will make FilesApp more responsive to users who are > likely to use terminal or some other crostini app before using FilesApp. > > Bug: 846917 > Change-Id: I9fc246e45e54390e7a75f15a5571a24218cf2d5c > Reviewed-on: https://chromium-review.googlesource.com/1158317 > Reviewed-by: Nicholas Verne <nverne@chromium.org> > Commit-Queue: Joel Hockey <joelhockey@chromium.org> > Cr-Commit-Position: refs/heads/master@{#580094} Bug: 846917 Change-Id: I3677f88fc17031a2beaa6c82fc731a3c4545f4a8 Reviewed-on: https://chromium-review.googlesource.com/1161761 Commit-Queue: Joel Hockey <joelhockey@chromium.org> Reviewed-by: Nicholas Verne <nverne@chromium.org> Cr-Commit-Position: refs/heads/master@{#581450} [modify] https://crrev.com/b785be60dcb4387d89d725db914d2b70be02dc92/chrome/app/chromeos_strings.grdp [modify] https://crrev.com/b785be60dcb4387d89d725db914d2b70be02dc92/chrome/browser/chromeos/crostini/crostini_manager.cc [modify] https://crrev.com/b785be60dcb4387d89d725db914d2b70be02dc92/chrome/browser/chromeos/crostini/crostini_manager.h [modify] https://crrev.com/b785be60dcb4387d89d725db914d2b70be02dc92/chrome/browser/chromeos/crostini/crostini_manager_unittest.cc [modify] https://crrev.com/b785be60dcb4387d89d725db914d2b70be02dc92/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc [modify] https://crrev.com/b785be60dcb4387d89d725db914d2b70be02dc92/chrome/browser/chromeos/extensions/file_manager/private_api_misc.h [modify] https://crrev.com/b785be60dcb4387d89d725db914d2b70be02dc92/chrome/browser/chromeos/file_manager/path_util.cc [modify] https://crrev.com/b785be60dcb4387d89d725db914d2b70be02dc92/chrome/browser/chromeos/file_manager/path_util.h [modify] https://crrev.com/b785be60dcb4387d89d725db914d2b70be02dc92/chrome/browser/chromeos/file_manager/volume_manager.cc [modify] https://crrev.com/b785be60dcb4387d89d725db914d2b70be02dc92/chrome/browser/ui/views/crostini/crostini_installer_view.cc [modify] https://crrev.com/b785be60dcb4387d89d725db914d2b70be02dc92/chrome/browser/ui/views/crostini/crostini_installer_view.h [modify] https://crrev.com/b785be60dcb4387d89d725db914d2b70be02dc92/chrome/browser/ui/views/crostini/crostini_installer_view_browsertest.cc
,
Sep 4
,
Sep 5
Joel, is this fixed already?
,
Sep 5
There have been 2 improvements: 1/ When doing drag and drop, there is no longer any delay before we trigger the container to start up. However, there is still the delay of the container starting. So you must hover over the target until the container starts before you can successfully drop. 2/ We now mount the container as soon as the container starts such as using the terminal or any other linux app. This means it is more likely that when a user does drag and drop, we will have already mounted linux files. The full solution requires modification to the files app so that it can receive a drop target on a fake item and then wait until the mount completes before doing the actual copy. I don't expect to make a change like this any time soon.
,
Sep 5
OK. It doesn't sound like this is blocking issue 880175, is that right?
,
Sep 5
,
Sep 12
Most common flows have been fixed, lowering priority for remaining work. |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by vapier@chromium.org
, May 29 2018