I can add this, but it's dependent on the Files app being done first.

We also need 9p integration for this too.

and this one

This bug seems a bit vague... is it referring to:
a. The file is in the crostini home dir already, so already available to a linux app, or
b. In the CrOS downloads folder, so needs to be shuffled across to 9p so it can be opened by a linux app, or
c. Both the above.

?

I think this means that when you right-click on a file in Files.app, installed Crostini apps are shown to the user as "Open with" targets in the context menu. There may need to be backend shuffling depending on where the file actually is, but the UX should be uniform: if I have VSCode installed, right clicking on a source file of some kind should show VSCode as an open-with target.

Got it.

In the PRD this bug is listed as "Files in Crostini (Linux $HOME) can be opened by Crostini apps from Files app" - so perhaps we should create a different bug for that work? 

Allowing files in Crostini's $HOME directory to be visible in Files.app feels like a separate piece of work, so a fresh bug for that SGTM. However, the UI-side work should be here - ideally, Files.app internally exposes the $HOME Crostini directory, and the UI is backend-agnostic and just knows that certain apps can open certain file types.

So luckily we already support browsing $HOME from the files app :)

I guess the delta is that is the file is already in crostini $HOME then we don't need to do anything special to allow a linux app to access the file, while if the file is in CrOS /Downloads we'll need to do some magic to make the file visible a linux app. 

I guess this bug is fixed when you can click on a file in crostini $HOME from the files app, and it opens with a linux app.

Then  crbug.com/852417  adds the ability to click on a file in /Downloads from the files app, and it opens with a linux app.

At least that's how I understand it :)

I agree with #13 about this bug being to hook everything up for the case when we have access to the file (i.e. it's in Crostini $HOME). It should also gracefully handle the case where there is no access to the file (i.e. it should not crash and should have some kind of decent UI).

 Issue 852417  is about deb files specifically. We probably need another issue to handle the non-$HOME non-deb files.

Tim - does that match your understanding?

I think the intent of the bug was to cover opening from both Crostini and Downloads (e.g. in GIMP or whatever). I don't mind splitting the Downloads portion into a separate bug.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/0e2a5fd43dc395b3fd526c8f726911871597dd82

commit 0e2a5fd43dc395b3fd526c8f726911871597dd82
Author: Timothy Loh <timloh@chromium.org>
Date: Wed Jun 20 21:23:48 2018

Add a files argument to vm_tools.cicerone.LaunchContainerApplicationRequest

This CL adds a |files| argument to the LaunchContainerApplicationRequest
proto so that we can open files in the file manager directly in Crostini
apps.

Bug:  822513 
Change-Id: Ib3ece65999adbd7a3fb49c608b6c13c613daba4b
Reviewed-on: https://chromium-review.googlesource.com/1103995
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Timothy Loh <timloh@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Jeffrey Kardatzke <jkardatzke@google.com>

[modify] https://crrev.com/0e2a5fd43dc395b3fd526c8f726911871597dd82/dbus/vm_cicerone/cicerone_service.proto

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

commit 686c6c7e47037dae68a8624da93fff0f94ac2e91
Author: Timothy Loh <timloh@chromium.org>
Date: Thu Jun 21 04:29:27 2018

Roll src/third_party/cros_system_api/ 557c37af3..0e2a5fd43 (3 commits)

https://chromium.googlesource.com/chromiumos/platform/system_api.git/+log/557c37af39f5..0e2a5fd43dc3

$ git log 557c37af3..0e2a5fd43 --date=short --no-merges --format='%ad %ae %s'
2018-06-19 timloh Add a files argument to vm_tools.cicerone.LaunchContainerApplicationRequest
2018-06-18 benchan shill: remove deprecated kHostNameProperty
2018-06-18 jkardatzke Removed unused protobufs/constants for concierge

Created with:
  roll-dep src/third_party/cros_system_api

Bug:  822513 
Change-Id: Ia857d7d8836ea91b71634c4fa42092be57e9fa19
Reviewed-on: https://chromium-review.googlesource.com/1109316
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569154}
[modify] https://crrev.com/686c6c7e47037dae68a8624da93fff0f94ac2e91/DEPS

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

commit d11a5c2a75d6c0e85227c3c37dd42ccda553e236
Author: Timothy Loh <timloh@chromium.org>
Date: Thu Jun 21 06:21:58 2018

Support launching Crostini apps from the File Manager

This CL adds support for launching Crostini apps from the File Manager.
For the moment, we only support files which are already in the Crostini
container, but in the future we will support external files as well.

Support for display app icons will be added in a separate patch, but as
it will require FindCrostiniTasks to be asynchronous, it is made to take
a callback to run upon completion.

Bug:  822513 
Change-Id: I257b482991ab8cf2e4d0ef5be18d0e33205df4e3
Reviewed-on: https://chromium-review.googlesource.com/1100606
Commit-Queue: Timothy Loh <timloh@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569171}
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/crostini/crostini_manager.cc
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/crostini/crostini_manager.h
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/crostini/crostini_registry_service.h
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/crostini/crostini_registry_service_unittest.cc
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/crostini/crostini_test_helper.cc
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/crostini/crostini_test_helper.h
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/crostini/crostini_util.cc
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/crostini/crostini_util.h
[add] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/file_manager/crostini_file_tasks.cc
[add] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/file_manager/crostini_file_tasks.h
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/file_manager/file_tasks.cc
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/file_manager/file_tasks.h
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/file_manager/file_tasks_unittest.cc
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/file_manager/path_util.cc
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/chrome/browser/chromeos/file_manager/path_util.h
[modify] https://crrev.com/d11a5c2a75d6c0e85227c3c37dd42ccda553e236/tools/metrics/histograms/enums.xml

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/4ff1ae43e54153891ee4e39d4f4241f882082acd

commit 4ff1ae43e54153891ee4e39d4f4241f882082acd
Author: Timothy Loh <timloh@chromium.org>
Date: Thu Jun 21 23:31:11 2018

vm_tools: Support launching apps with file arguments

This CL adds support for the |files| argument in
LaunchContainerApplicationRequest, which will be passed through to the
container and used to launch applications with the given file arguments.
For now, applications which only support launching with a single file
will only use the first file in the list, instead of being launched
multiple times.

BUG= chromium:822513 
TEST=Manually verified this works in combination with
https://crrev.com/c/1100606 and https://crrev.com/c/1103995.
CQ-DEPEND=CL:1103995
Change-Id: I87d7bab6aad8cebe6e14ed29596f1cb413ad08e6
Reviewed-on: https://chromium-review.googlesource.com/1104060
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Timothy Loh <timloh@chromium.org>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>

[modify] https://crrev.com/4ff1ae43e54153891ee4e39d4f4241f882082acd/vm_tools/cicerone/service.cc
[modify] https://crrev.com/4ff1ae43e54153891ee4e39d4f4241f882082acd/vm_tools/cicerone/virtual_machine.cc
[modify] https://crrev.com/4ff1ae43e54153891ee4e39d4f4241f882082acd/vm_tools/garcon/service_impl.cc
[modify] https://crrev.com/4ff1ae43e54153891ee4e39d4f4241f882082acd/vm_tools/proto/container_guest.proto
[modify] https://crrev.com/4ff1ae43e54153891ee4e39d4f4241f882082acd/vm_tools/cicerone/virtual_machine.h

The remaining work here is to display icons for these apps.

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

commit 6f3f1a9e39b22b12d4df90d66e5a4b2f16bf6f11
Author: Timothy Loh <timloh@chromium.org>
Date: Thu Jul 19 06:02:46 2018

Show icons for Crostini Apps in the File Manager

This patch adds support for app icons when used as file handlers in the
file manager. As there is no place to store the CrostiniAppIcon
objects, a IconLoadWaiter is added which manually manages it's lifetime
and deletes itself after an icon is loaded (or fails to load).

BUG= 822513 

Change-Id: Ia31b32806e399df8902ffbf5c953e39dbc414774
Reviewed-on: https://chromium-review.googlesource.com/1127204
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576385}
[modify] https://crrev.com/6f3f1a9e39b22b12d4df90d66e5a4b2f16bf6f11/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[modify] https://crrev.com/6f3f1a9e39b22b12d4df90d66e5a4b2f16bf6f11/chrome/browser/chromeos/crostini/crostini_registry_service.h
[modify] https://crrev.com/6f3f1a9e39b22b12d4df90d66e5a4b2f16bf6f11/chrome/browser/chromeos/crostini/crostini_util.cc
[modify] https://crrev.com/6f3f1a9e39b22b12d4df90d66e5a4b2f16bf6f11/chrome/browser/chromeos/crostini/crostini_util.h
[modify] https://crrev.com/6f3f1a9e39b22b12d4df90d66e5a4b2f16bf6f11/chrome/browser/chromeos/file_manager/crostini_file_tasks.cc
[modify] https://crrev.com/6f3f1a9e39b22b12d4df90d66e5a4b2f16bf6f11/chrome/browser/ui/app_list/crostini/crostini_app_icon.cc
[modify] https://crrev.com/6f3f1a9e39b22b12d4df90d66e5a4b2f16bf6f11/chrome/browser/ui/app_list/crostini/crostini_app_icon.h

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

commit 338ba0e2715f166fc50b6a92e41f865d03cdc3e1
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Thu Jul 19 17:44:46 2018

Revert "Show icons for Crostini Apps in the File Manager"

This reverts commit 6f3f1a9e39b22b12d4df90d66e5a4b2f16bf6f11.

Reason for revert: Seems to cause the following tests to fail due to a memory leak:
FileManagerFileTasksCrostiniTest.BasicFiles
FileManagerFileTasksCrostiniTest.MultipleTypes
FileManagerFileTasksCrostiniTest.MultipleMatches

See https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/28317

Sample output snippet:
Indirect leak of 384 byte(s) in 1 object(s) allocated from:
    #0 0xbe89c2 in operator new(unsigned long) /b/swarming/w/ir/kitchen-workdir/src/third_party/llvm/compiler-rt/lib/asan/asan_new_delete.cc:106:3
    #1 0xec8724f in LoadIcons chrome/browser/chromeos/crostini/crostini_util.cc:141:5
    #2 0xec8724f in LoadIcons(Profile*, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&, int, ui::ScaleFactor, base::TimeDelta, base::OnceCallback<void (std::__1::vector<gfx::ImageSkia, std::__1::allocator<gfx::ImageSkia> > const&)>) chrome/browser/chromeos/crostini/crostini_util.cc:340
    #3 0xed5ecd9 in file_manager::file_tasks::FindCrostiniTasks(Profile*, std::__1::vector<extensions::EntryInfo, std::__1::allocator<extensions::EntryInfo> > const&, std::__1::vector<file_manager::file_tasks::FullTaskDescriptor, std::__1::allocator<file_manager::file_tasks::FullTaskDescriptor> >*, base::OnceCallback<void ()>) chrome/browser/chromeos/file_manager/crostini_file_tasks.cc:123:3
    #4 0xed74e13 in file_manager::file_tasks::FindExtensionAndAppTasks(Profile*, std::__1::vector<extensions::EntryInfo, std::__1::allocator<extensions::EntryInfo> > const&, std::__1::vector<GURL, std::__1::allocator<GURL> > const&, base::RepeatingCallback<void (std::__1::unique_ptr<std::__1::vector<file_manager::file_tasks::FullTaskDescriptor, std::__1::allocator<file_manager::file_tasks::FullTaskDescriptor> >, std::__1::default_delete<std::__1::vector<file_manager::file_tasks::FullTaskDescriptor, std::__1::allocator<file_manager::file_tasks::FullTaskDescriptor> > > >)> const&, std::__1::unique_ptr<std::__1::vector<file_manager::file_tasks::FullTaskDescriptor, std::__1::allocator<file_manager::file_tasks::FullTaskDescriptor> >, std::__1::default_delete<std::__1::vector<file_manager::file_tasks::FullTaskDescriptor, std::__1::allocator<file_manager::file_tasks::FullTaskDescriptor> > > >) chrome/browser/chromeos/file_manager/file_tasks.cc:624:3


Original change's description:
> Show icons for Crostini Apps in the File Manager
> 
> This patch adds support for app icons when used as file handlers in the
> file manager. As there is no place to store the CrostiniAppIcon
> objects, a IconLoadWaiter is added which manually manages it's lifetime
> and deletes itself after an icon is loaded (or fails to load).
> 
> BUG= 822513 
> 
> Change-Id: Ia31b32806e399df8902ffbf5c953e39dbc414774
> Reviewed-on: https://chromium-review.googlesource.com/1127204
> Reviewed-by: Joel Hockey <joelhockey@chromium.org>
> Reviewed-by: Nicholas Verne <nverne@chromium.org>
> Commit-Queue: Timothy Loh <timloh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#576385}

TBR=timloh@chromium.org,joelhockey@chromium.org,nverne@chromium.org

Change-Id: I03ede5af292b3a33fb86ad408f6c4596c876e3ea
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  822513 
Reviewed-on: https://chromium-review.googlesource.com/1144020
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576554}
[modify] https://crrev.com/338ba0e2715f166fc50b6a92e41f865d03cdc3e1/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[modify] https://crrev.com/338ba0e2715f166fc50b6a92e41f865d03cdc3e1/chrome/browser/chromeos/crostini/crostini_registry_service.h
[modify] https://crrev.com/338ba0e2715f166fc50b6a92e41f865d03cdc3e1/chrome/browser/chromeos/crostini/crostini_util.cc
[modify] https://crrev.com/338ba0e2715f166fc50b6a92e41f865d03cdc3e1/chrome/browser/chromeos/crostini/crostini_util.h
[modify] https://crrev.com/338ba0e2715f166fc50b6a92e41f865d03cdc3e1/chrome/browser/chromeos/file_manager/crostini_file_tasks.cc
[modify] https://crrev.com/338ba0e2715f166fc50b6a92e41f865d03cdc3e1/chrome/browser/ui/app_list/crostini/crostini_app_icon.cc
[modify] https://crrev.com/338ba0e2715f166fc50b6a92e41f865d03cdc3e1/chrome/browser/ui/app_list/crostini/crostini_app_icon.h

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

commit db4960bf9310717cc64c5bbad6a399382bcbd309
Author: Timothy Loh <timloh@chromium.org>
Date: Wed Jul 25 08:07:50 2018

Show icons for Crostini Apps in the File Manager

This patch adds support for app icons when used as file handlers in the
file manager. As there is no place to store the CrostiniAppIcon
objects, a IconLoadWaiter is added which manually manages it's lifetime
and deletes itself after an icon is loaded (or fails to load).

This is a re-land of https://crrev.com/c/1127204, but with the longer
deletion time-out removed. It turns out the longer time-out is not
needed, and it was causing LSAN to detect a leak as the objects were
not cleaned up by test completion.

BUG= 822513 

Change-Id: Id04730ee2ea18f6f5004624471e40bcd13e9974b
Reviewed-on: https://chromium-review.googlesource.com/1149432
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577822}
[modify] https://crrev.com/db4960bf9310717cc64c5bbad6a399382bcbd309/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[modify] https://crrev.com/db4960bf9310717cc64c5bbad6a399382bcbd309/chrome/browser/chromeos/crostini/crostini_registry_service.h
[modify] https://crrev.com/db4960bf9310717cc64c5bbad6a399382bcbd309/chrome/browser/chromeos/crostini/crostini_util.cc
[modify] https://crrev.com/db4960bf9310717cc64c5bbad6a399382bcbd309/chrome/browser/chromeos/crostini/crostini_util.h
[modify] https://crrev.com/db4960bf9310717cc64c5bbad6a399382bcbd309/chrome/browser/chromeos/file_manager/crostini_file_tasks.cc
[modify] https://crrev.com/db4960bf9310717cc64c5bbad6a399382bcbd309/chrome/browser/ui/app_list/crostini/crostini_app_icon.cc
[modify] https://crrev.com/db4960bf9310717cc64c5bbad6a399382bcbd309/chrome/browser/ui/app_list/crostini/crostini_app_icon.h

Requesting merge for the CL in comment 24. I tested this on ToT, and nverne@ also checked it independently.

This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Release blockers should be P1 or P0

cindyb@: Ping, any update on the merge request?

Merge approved, M69.

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

commit b116d8c5c330f6c836e7e88cf19bf288170b4327
Author: Timothy Loh <timloh@chromium.org>
Date: Fri Aug 10 05:24:47 2018

Show icons for Crostini Apps in the File Manager

This patch adds support for app icons when used as file handlers in the
file manager. As there is no place to store the CrostiniAppIcon
objects, a IconLoadWaiter is added which manually manages it's lifetime
and deletes itself after an icon is loaded (or fails to load).

This is a re-land of https://crrev.com/c/1127204, but with the longer
deletion time-out removed. It turns out the longer time-out is not
needed, and it was causing LSAN to detect a leak as the objects were
not cleaned up by test completion.

BUG= 822513 

Change-Id: Id04730ee2ea18f6f5004624471e40bcd13e9974b
Reviewed-on: https://chromium-review.googlesource.com/1149432
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577822}(cherry picked from commit db4960bf9310717cc64c5bbad6a399382bcbd309)
Reviewed-on: https://chromium-review.googlesource.com/1170542
Reviewed-by: Renée Wright <rjwright@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#536}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/b116d8c5c330f6c836e7e88cf19bf288170b4327/chrome/browser/chromeos/crostini/crostini_registry_service.cc
[modify] https://crrev.com/b116d8c5c330f6c836e7e88cf19bf288170b4327/chrome/browser/chromeos/crostini/crostini_registry_service.h
[modify] https://crrev.com/b116d8c5c330f6c836e7e88cf19bf288170b4327/chrome/browser/chromeos/crostini/crostini_util.cc
[modify] https://crrev.com/b116d8c5c330f6c836e7e88cf19bf288170b4327/chrome/browser/chromeos/crostini/crostini_util.h
[modify] https://crrev.com/b116d8c5c330f6c836e7e88cf19bf288170b4327/chrome/browser/chromeos/file_manager/crostini_file_tasks.cc
[modify] https://crrev.com/b116d8c5c330f6c836e7e88cf19bf288170b4327/chrome/browser/ui/app_list/crostini/crostini_app_icon.cc
[modify] https://crrev.com/b116d8c5c330f6c836e7e88cf19bf288170b4327/chrome/browser/ui/app_list/crostini/crostini_app_icon.h

Should we mark this as fixed?