@oka - I know we don't do Quick View for the Drive folder, but shouldn't the Media View files be local?

+nya@

I think nya tried to fix this but it somehow didn't work.

Currently, quick view can show previews for only files accessible via file:// scheme (maybe I'm not precise) due to security reasons.

Actually I recently disabled previews except for Downloads, removable medias (and some types of archives). Before this change just broken preview was shown, e.g. for MTP volumes.
https://codereview.chromium.org/2633053003/

But I agree this is not an ideal situation.

Got it, thanks for the update.

I don't think we need to prioritize this now, but it would be more consistent to allow for Quick View on the media folders. Would you be able to provide some additional information around the security challenges and estimates on the work needed to implement for future reference?

Yes, it is very unfortunate that quick view preview does not work for media volumes like MTP and media views. I'm not very familiar with security restriction mechanism around quick view, but if the fix is not security-wise problematic, I agree we want the fix. oka@, do you know background of the problem?

I investigate the issue using Dropbox fsp on Chrome OS on Linux. Dropbox files cannot be rendered in Quick View and I guess the cause is the same as media view.

Inside CreateFileStreamReader, IsAccessAllowed returns false.
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/fileapi/file_system_backend.cc?q=file_system_backend&sq=package:chromium&dr=C&l=383
If I remove this permission check, images in Dropbox are rendered.
Inside IsAccessAllowed, HasAccessPermission returns false, because path_map_ was empty there.
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/fileapi/file_access_permissions.cc?type=cs&l=22

Stacktrace is as below:
[6781:6852:0201/221214.170214:ERROR:file_system_backend.cc(396)] #0 0x7f848d8e556b base::debug::StackTrace::StackTrace()
#1 0x7f848d8e3bec base::debug::StackTrace::StackTrace()
#2 0x7f848f9e59c6 chromeos::FileSystemBackend::CreateFileStreamReader()
#3 0x7f847fead462 storage::FileSystemContext::CreateFileStreamReader()
#4 0x7f847fddc08d storage::(anonymous namespace)::FileStreamReaderProviderImpl::CreateFileStreamReader()
#5 0x7f847fe0c777 storage::BlobReader::CreateFileStreamReader()
#6 0x7f847fe0d466 storage::BlobReader::GetOrCreateFileReaderAtIndex()
#7 0x7f847fe0b9c5 storage::BlobReader::CalculateSizeImpl()
#8 0x7f847fe0b4d3 storage::BlobReader::CalculateSize()
#9 0x7f847fe41b9e storage::BlobURLRequestJob::DidStart()
#10 0x7f847fe450c7 _ZN4base8internal13FunctorTraitsIMN7storage17BlobURLRequestJobEFvvEvE6InvokeIRKNS_7WeakPtrIS3_EEJEEEvS5_OT_DpOT0_
#11 0x7f847fe4501a _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN7storage17BlobURLRequestJobEFvvERKNS_7WeakPtrIS5_EEJEEEvOT_OT0_DpOT1_
#12 0x7f847fe44fa2 _ZN4base8internal7InvokerINS0_9BindStateIMN7storage17BlobURLRequestJobEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE7RunImplIRKS6_RKSt5tupleIJS8_EEJLm0EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#13 0x7f847fe44eec _ZN4base8internal7InvokerINS0_9BindStateIMN7storage17BlobURLRequestJobEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE3RunEPNS0_13BindStateBaseE
#14 0x7f848d8eaad1 _ZNO4base8internal8RunMixinINS_8CallbackIFvvELNS0_8CopyModeE0ELNS0_10RepeatModeE0EEEE3RunEv
#15 0x7f848d8ea4d9 base::debug::TaskAnnotator::RunTask()
#16 0x7f848d971373 base::MessageLoop::RunTask()
#17 0x7f848d9715d4 base::MessageLoop::DeferOrRunPendingTask()
#18 0x7f848d9718be base::MessageLoop::DoWork()
#19 0x7f848d9888dc base::MessagePumpLibevent::Run()
#20 0x7f848d970f62 base::MessageLoop::RunHandler()
#21 0x7f848da11c64 base::RunLoop::Run()
#22 0x7f848daaeee2 base::Thread::Run()
#23 0x7f8487042758 content::BrowserThreadImpl::IOThreadRun()
#24 0x7f84870429f1 content::BrowserThreadImpl::Run()
#25 0x7f848daaf6d3 base::Thread::ThreadMain()
#26 0x7f848da97e4a base::(anonymous namespace)::ThreadFunc()
#27 0x7f848dd13184 start_thread
#28 0x7f847658137d clone

I'll check what happens for files in Download, in which the above error doesn't happen.

Forgot to mention, but in the above experiment I removed some checking code so to deliberately render files provided by FSP in Quick View.

Any update here? It is a weird user experience that it isn't available for the media views

Weifang, do we want to fix this in M57, which is stable from Mar 14?

I think it depends on the solution. If it is going to be a security-related change, maybe we should not rush to M57.

IIUC, it's security related problem, because Files app can show the file if
webview was not used. So I agree not to rush to M57.

Yes, agreed! We should wait for M58 for this update.

 Issue 759826  has been merged into this issue.

I heard oka@ is not actively working on it, so unassigning.

I may work on this later since this issue is more visible in Recent.

Let's make it happen in M64.

+mtomasz@, the owner of fileapi.
I'd like to show ARC++ files in <webview>. Currently it's only possible for local files.
The code looks like this:

url = URL.createObjectURL(file)
webview.contentWindow.postMessage(url)
# webview renders the received url.

This procedure doesn't work due to permission error for ARC++ files, or other FSP, like Dropbox.
Do you have any idea of how to fix it?

Thanks.

+hashimoto who added support for viewing ARC PDF files in Chrome. @hashimoto: Are those files opened by Chrome using a local path, or the contents are piped via File API? If the second, then how did you grant the permissions to Chrome?

> Are those files opened by Chrome using a local path, or the contents are piped via File API?

Chrome converts file: & content: URLs given by Android to externalfile: URLs when being asked to open Android URLs.
https://chromium.googlesource.com/chromium/src/+/51c4bc1252170d459ed0f7122170850dc205cf09/chrome/browser/ui/ash/chrome_shell_delegate.cc#410

It results in creating a new Chrome tab to open the externalfile: URL.
Then ExternalFileURLRequestJob handles the externalfile: URL, and it forwards the request to ArcContentFileSystemFileStreamReader which uses arc::FileSystemIntance::OpenFileToRead() to read the data from Android.


> how did you grant the permissions to Chrome?

If the permission error you are seeing is an Android one, Android's permission grant dialog for storage access should be shown when opening an URL.
Probably, we are not seeing Android's permission grant dialog in your case, because the open action is not triggered by Android, but Chrome.

IIUC we can solve this by granting IntentHelper in the ARC container the same permission as DocumentsUI, but it may result in breaking CTS.
nya@ should be familiar with this documents permission problem.

Talked with hashimoto@ and hirono@ offline.

hirono@ said he is not sure why webview can't access a URL built by URL.createObjectURL(file). However, if we pass an isolated file system URL to webview, instead of blob URL, it might pass permission checks because we don't enforce any permission checks to isolated file system URLs. However currently we don't have a (private) API to build an isolated file system URL from file/blob, so we may need to add one.

newbie question: what is "isolated filesystem URLs"? Any pointers?

Actually the solution can be a bit more complex, because normally JavaScript cannot get isolated filesystem URL and there may be additional checks to prevent from getting data from isolated file urls directly. In this case, we would like to do the followings.

1. We have FileEntry backed by non-native external filesystem (e.g. Arc, MTP).
2. Create FileEntry backed by isolated filesystem from the normal FileEntry. (By using private API which will be added)
3. Get File from FileEntry#file.
4. Obtains blob URL by using URL.createObjectURL(file).

For 2, could you point to the entry point to create an isolated filesystem?

CreateIsolatedURLFromVirtualPath should be useful for this.
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/file_manager/fileapi_util.h?rcl=0b89d8f1cf433f1e2fa0850eb3c234777bf49160&l=166

Re API design, how about

  [nocompile]
  static void createIsolatedEntries(
      [instanceOf=Entry] object[] entries,
      ResolveEntriesCallback callback);

to be consistent with the existing API:

  [nocompile]
  static void resolveIsolatedEntries(
      [instanceOf=Entry] object[] entries,
      ResolveEntriesCallback callback);

hirono@ I made a change to implement #25, but it didn't work. Could you help me to debug?
It fails on converting FileDefinitionList to EntryDefinitionList.
I built and ran https://chromium-review.googlesource.com/c/chromium/src/+/812184/5, and got the following log, when opening an ARC++ image file with Quick View (nothing was displayed).

[17758:17758:1206/163519.859476:ERROR:private_api_file_system.cc(881)] CreateIsolatedEntries A: filesystem:chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/external/arc-documents-provider/com.android.provider
s.media.documents/images_root/Pic Collage/Collage 2017-12-06 15_19_59.jpg (ArcDocumentsProvider@arc-documents-provider:/special/arc-documents-provider/com.android.providers.media.documents/images_root/Pic Collag
e/Collage 2017-12-06 15_19_59.jpg)
[17758:17758:1206/163519.862543:ERROR:private_api_file_system.cc(889)] CreateIsolatedEntries B: filesystem:chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/isolated/4920FA3E2CC44084B4F5B208DA307D44/Collage 20
17-12-06 15_19_59.jpg (ArcDocumentsProvider@arc-documents-provider:/special/arc-documents-provider/com.android.providers.media.documents/images_root/Pic Collage/Collage 2017-12-06 15_19_59.jpg)
[17758:17886:1206/163519.862943:ERROR:file_system_context.cc(285)] NOTREACHED() hit.
[17758:17886:1206/163519.863036:ERROR:file_system_context.cc(286)] GetFileSystemBackend Unknown filesytem type: -1 >>> #0 0x000003d91aec base::debug::StackTrace::StackTrace()
#1 0x000004ef7030 storage::FileSystemContext::GetFileSystemBackend()
#2 0x000004ef74c6 storage::FileSystemContext::ResolveURL()
#3 0x000004ef835a _ZN4base8internal7InvokerINS0_9BindStateIMN7storage17FileSystemContextEFvRKNS3_13FileSystemURLENS_12OnceCallbackIFvNS_4File5ErrorERKNS3_14FileSystemInfoERKNS_8FilePathENS4_17ResolvedEntryTypeEE
EEEJ13scoped_refptrIS4_ES5_SJ_EEEFvvEE7RunOnceEPNS0_13BindStateBaseE
#4 0x000003d920bc base::debug::TaskAnnotator::RunTask()
#5 0x000003da58d0 base::MessageLoop::RunTask()
#6 0x000003da5c3e base::MessageLoop::DoWork()
#7 0x000003da6960 base::MessagePumpLibevent::Run()
#8 0x000003dbdd98 base::RunLoop::Run()
#9 0x000002cd1b80 content::BrowserThreadImpl::IOThreadRun()
#10 0x000002cd1c92 content::BrowserThreadImpl::Run()
#11 0x000003ddd0bc base::Thread::ThreadMain()
#12 0x000003dd90a4 base::(anonymous namespace)::ThreadFunc()
<<<
[17758:17758:1206/163520.553116:ERROR:private_api_file_system.cc(915)] CreateIsolatedEntries C
[17758:17758:1206/163520.553174:ERROR:private_api_file_system.cc(921)] CreateIsolatedEntries C2: definition.error

> [17758:17886:1206/163519.862943:ERROR:file_system_context.cc(285)] NOTREACHED() hit.
> [17758:17886:1206/163519.863036:ERROR:file_system_context.cc(286)] GetFileSystemBackend Unknown filesytem type: -1 >>> #0 

It looks the log contains relevant NOTREACHED(). Does this happen when creating isolated entries, or when resolving entries? What is passed as filesystem type?

Is happens while resolving entries. As B: shows the isolated file URL has
been created, but it fails to be converted to an entry.
The type passed to the method GetFileSystemBackend was -1.

ConvertFileDefinitionListToEntryDefinitionList specifies kFileSystemTypeExternal internally so we cannot use this helper to create isolated entries. Because we have already obtained FileSystemUrl for isolated URL, I guess we can fill EntryDescription from FileSystemUrl somehow.

This is off topic, but we may want to check if we can actually read data in webview via isolated entries first. By using isolated entries, we are probably able to pass the check of IsAccessAllowed mentioned at #7. But there may be other checks which prevents webview from reading data. You can obtain isolated entries for example by creating a test app which receives entries in launch data via onLaunched event.
https://developer.chrome.com/apps/app_runtime#event-onLaunched

Assigning to myself to investigate for M66.

Pushing to M68 when we will have a better story for accessing ARC++ files from the Files App.

Issue 865180 has been merged into this issue.

This is still happening on M69 (10895.18.0/69.0.3497.31) nautilus device.

Thanks for the update :)