add support for data:* URIs

[sigmundch]

The default FileSystem only supports file:* URIs, we should also support data:* URIs

[keertip]

This is blocking internal testing of the VM+FE.

[sigmundch]

Peter and I had a quick discussion about this - we agreed that the support for data:* should be added at the file-system abstraction layer, but it will not be done in the physical file system. Instead there will be an additional file system added for this, which is only enabled by the VM's kernel-service (this is to keep dart2js/ddc as they work today without support for these URIs)

[kmillikin]

From @sigmundch over chat:

It's a fairly simple change, basically the compiler doesn't need any changes, we just need to add a new FileSystem implementation to kernel-service. The compiler when it reads an URI it delegates to the file system to do the read, so the new file-system internally will distinguish the scheme and do UriData.decode(...) on it if it is a data URI.

[peter-ahe-google]

Aske and I discussed this at lunch today.

We'll extend PhysicalFileSystem to handle data-URIs.

dart:core provides support for data-URIs, both creating them and decoding them. The latter is done via Uri.data.

[sigmundch]

@peter-ahe-google & @askeksa-google - just a point that Peter and I discussed a while back (not sure if you covered this during your conversation): we prefer not to put the logic together in the same PhysicalFileSystem implementation so we don't add support for data:* uris to ddc/dart2js as a result (see also the earlier comment above from a month ago).

My thinking was that we could compose the file system with something among these lines:

• PhysicalFileSystem - like today, it just reads "file:" URIs
• DataFileSystem - a file system that only reads "data:*" URIs
• MixedSchemeFileSystem - a file system that delegates to other file systems based on the URI scheme. So the kernel-service would create one like: MixedSchemeFileSystem({'data': dataFS, 'file': physicalFS});

[askeksa-google]

What would be a good way to test that such a mixed file system works in the context of Fasta, i.e. for compiling a Dart program containing data: URIs? Is there a convenient way of specifying that this other file system should be used for a particular test case?

[sigmundch]

That might depend on how much test coverage we want and how we want to test it.

To test it with the package:testing-chain, we can change the file system definition here:

sdk/pkg/front_end/test/fasta/testing/suite.dart

Line 297 in 391955f

PhysicalFileSystem.instance, false, dillTarget, uriTranslator);

However, since we are anyways going to be updating the vm kernel-service to use this, I'd be inclined to leverage the existing testing from VM tests. In particular:

• After we change the default FS in kernel-service here:
  

  sdk/pkg/vm/bin/kernel_service.dart

  Line 159 in 391955f

  final FileSystem fileSystem = sourceFiles == null

• We could add test files that use data URIs in a VM-only suite like tests/standalone_2

You may notice in that link above that the VM's uses a custom file system also for their own unit tests set up. This is to support adding in-memory source-files that they define in the tests. We use the HybridFileSystem for that today. Honestly, if the VM unit tests can be changed to use a custom URI scheme, then we could potentially get rid of the use of the hybrid file system there and instead use the MixedSchemeFileSystem for both purposes. It would now have 3 schemes, one for tests (which points to a memory file system), one for data-uris, and one for file-uris.

[askeksa-google]

Peter and I discussed this some more. :)

As the FileSystem abstraction is used currently, its function is to specify how URIs are handled in general, rather than to specify handling of specific schemes. Based on this observation, it makes most sense to replace PhysicalFileSystem by a StandardFileSystem (for the normal handling of standard URIs, including data) and then to replace/wrap that when special handling is required (as we already do with the MemoryFileSystem/HybridFileSystem).

[askeksa-google]

Missing piece of the argument: It seems we do want universal data URI support, contrary to some comments earlier in this issue.