Moving the cache to local storage.

Moving the cache to local storage.

Currently the system cache is created inside the project directory in
the 'cache' subdirectory. This is pointless, as it should be stored
in a global location which every project would be able to access.

This patch adds a 'window.requestFileSystem' call to be able to grab
a cache directory. Though running this results in a DOMError that
I don't know how to deal with right now.

[pajamallama, 2014-01-28 05:55:01]

Not working yet, strange bug, very confused.

[tapted, 2014-01-29 02:52:02]

I patched it in and tried it, but didn't see the bug. What's happening?

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
File dart/sdk/lib/_internal/pub/lib/src/path_rep.dart (right):

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/path_rep.dart:8: bool inCache;
nit: leading underscore

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/path_rep.dart:21: // Gets the full path for
this path.
nit: ///

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/path_rep.dart:25: String get name => (inCache
? "cache:" : "") + _filename;
nit: single quotes

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
File dart/sdk/lib/_internal/pub/lib/src/source.dart (right):

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/source.dart:208: });
nit: outdent 2 spaces

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
File dart/sdk/lib/_internal/pub/lib/src/wrap/io_wrap.dart (right):

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/wrap/io_wrap.dart:49: })
nit: outdent 2 spaces

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/wrap/io_wrap.dart:74: log.error("Error
loading cache directory", e);
Is this creating output?

[tapted, 2014-01-29 02:56:07]

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
File dart/sdk/lib/_internal/pub/lib/src/path_rep.dart (right):

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/path_rep.dart:25: String get name => (inCache
? "cache:" : "") + _filename;
On 2014/01/29 02:52:02, tapted wrote:
> nit: single quotes

actually I guess either is fine - I forget that Dart is not as strict with this
as Js.

[pajamallama, 2014-01-29 05:42:57]

Fixed nits.. I'll look at that bug again.

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
File dart/sdk/lib/_internal/pub/lib/src/path_rep.dart (right):

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/path_rep.dart:8: bool inCache;
On 2014/01/29 02:52:02, tapted wrote:
> nit: leading underscore

Done.

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/path_rep.dart:21: // Gets the full path for
this path.
On 2014/01/29 02:52:02, tapted wrote:
> nit: ///

Done.

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/path_rep.dart:25: String get name => (inCache
? "cache:" : "") + _filename;
On 2014/01/29 02:56:07, tapted wrote:
> On 2014/01/29 02:52:02, tapted wrote:
> > nit: single quotes
> 
> actually I guess either is fine - I forget that Dart is not as strict with
this
> as Js.

Done.

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
File dart/sdk/lib/_internal/pub/lib/src/source.dart (right):

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/source.dart:208: });
On 2014/01/29 02:52:02, tapted wrote:
> nit: outdent 2 spaces

Done.

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
File dart/sdk/lib/_internal/pub/lib/src/wrap/io_wrap.dart (right):

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/wrap/io_wrap.dart:49: })
On 2014/01/29 02:52:02, tapted wrote:
> nit: outdent 2 spaces

Done.

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/wrap/io_wrap.dart:74: log.error("Error
loading cache directory", e);
On 2014/01/29 02:52:02, tapted wrote:
> Is this creating output?

Nope..

[pajamallama, 2014-01-30 04:56:53]

Still not working :(

Now the cache is located inside the persistent storage, and writing packages
there does work. When the pub test app opens it recursively lists all the files
stored there.

But the major problem stems from the fact that window.requestFileSystem returns
a html.FileSystem. Everywhere else the chrome.FileSystem is being used (which is
just a thin wrapper around the Javascript objects).

Many parts of the interface are the same, but reading/writing data is vastly
different. The only solution I can see is to write code for both interfaces, but
that overly complicates the design.

Another alternative is to attempt to throw away all the chrome.dart library
calls in io_wrap.dart and simply use the html library. This would be nice but
we'd lose chrome.chooseEntry which is needed just the first time the app is
run..

[tapted, 2014-01-30 07:01:46]

On 2014/01/30 04:56:53, pajamallama wrote:
> Still not working :(
> 
> Now the cache is located inside the persistent storage, and writing packages
> there does work. When the pub test app opens it recursively lists all the
files
> stored there.
> 
> But the major problem stems from the fact that window.requestFileSystem
returns
> a html.FileSystem. Everywhere else the chrome.FileSystem is being used (which
is
> just a thin wrapper around the Javascript objects).
> 
> Many parts of the interface are the same, but reading/writing data is vastly
> different. The only solution I can see is to write code for both interfaces,
but
> that overly complicates the design.
> 
> Another alternative is to attempt to throw away all the chrome.dart library
> calls in io_wrap.dart and simply use the html library. This would be nice but
> we'd lose chrome.chooseEntry which is needed just the first time the app is
> run..

We still need chrome.chooseEntry - I don't think there's a way to create files
in the "actual" filesystem otherwise.

Inheriting from PathRep and overriding functions there to perform the correct
read/write workflows is probably the way to do it. But let's just put this on
the backburner for now.

[pajamallama, 2014-02-05 07:22:05]

IT WORKS!!!!!!!!!!!!!!!!!!!!!

[pajamallama, 2014-02-05 23:59:41]

Okay, I may have submitted the previous patch too early, in a fit of
celebration.

This is the cleaned up version.

[tapted, 2014-02-06 02:05:14]

having a look now. It's a big delta - can you explain/highlight what's actually
changing in io_wrap.dart, or convince the diff to be less scary (even if it
means indenting createFile et al., extractFoo*, etc. in 2 spaces). Usually we
try to isolate big changes that move things around without a lot of logic
changes.

[tapted, 2014-02-06 02:41:30]

I'm really impressed you got this working.

Looking more closely though, I think io_wrap has diverged too much. It is meant
to mirror just what's in dart:io, providing what's needed when dart:io isn't
available. There shouldn't be anything declared there which isn't in dart:io.

More structural changes need to go on PathRep, which can be equipped with
polymorphism to deal with details about whether to load from the dart:io-style
interfaces or from the html5 interfaces. I think you need to leverage what
PathRep can do for you a bit more, but designing it right might still be tricky
-- likely "most" most of what's in pub's own io.dart would need to move to
polymorphic functions under PathRep.

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
File dart/sdk/lib/_internal/pub/lib/src/path_rep.dart (right):

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/path_rep.dart:8: bool inCache;
On 2014/01/29 05:42:57, pajamallama wrote:
> On 2014/01/29 02:52:02, tapted wrote:
> > nit: leading underscore
> 
> Done.

Is it? ;)

https://codereview.appspot.com/57650043/diff/140001/dart/sdk/lib/_internal/pu...
File dart/sdk/lib/_internal/pub/lib/src/io.dart (right):

https://codereview.appspot.com/57650043/diff/140001/dart/sdk/lib/_internal/pu...
dart/sdk/lib/_internal/pub/lib/src/io.dart:91: createDir(symlink)])
nit: indent 1 more space

https://codereview.appspot.com/57650043/diff/140001/dart/sdk/lib/_internal/pu...
dart/sdk/lib/_internal/pub/lib/src/io.dart:102: /// Reads the contents of the
text file [file].
nit: blank line before

https://codereview.appspot.com/57650043/diff/140001/dart/sdk/lib/_internal/pu...
dart/sdk/lib/_internal/pub/lib/src/io.dart:103: Future<String>
readTextFile(PathRep file) {
Since mirrors dart:io's File.readAsString(), I think it belongs on PathRep
(probably called readAsString :).

That is, Pub's own io.dart should't have functionality it used to get dart:io -
that's the job for the wrappers.

https://codereview.appspot.com/57650043/diff/140001/dart/sdk/lib/_internal/pu...
dart/sdk/lib/_internal/pub/lib/src/io.dart:113: Future<Uint8List>
readBinaryFile(PathRep filePath) =>
same here -> PathRep.readAsBytes seems reasonable

https://codereview.appspot.com/57650043/diff/140001/dart/sdk/lib/_internal/pu...
dart/sdk/lib/_internal/pub/lib/src/io.dart:127: return
createFile(file).then((file) => writeText(file, contents));
e.g. here (PathRep)file.writeText(contents) would allow us to continue using
dart:io interfaces (i.e. `createFile` doesn't exist in dart:io -- only
File.create).

https://codereview.appspot.com/57650043/diff/140001/dart/sdk/lib/_internal/pu...
dart/sdk/lib/_internal/pub/lib/src/io.dart:251: }).catchError((_) => []);
maybe log the error?

https://codereview.appspot.com/57650043/diff/140001/dart/sdk/lib/_internal/pu...
File dart/sdk/lib/_internal/pub/lib/src/log.dart (right):

https://codereview.appspot.com/57650043/diff/140001/dart/sdk/lib/_internal/pu...
dart/sdk/lib/_internal/pub/lib/src/log.dart:94: if (error is Exception)
neat!

[pajamallama, 2014-02-06 03:48:12]

Will work on the PathRep changes next.

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
File dart/sdk/lib/_internal/pub/lib/src/path_rep.dart (right):

https://codereview.appspot.com/57650043/diff/1/dart/sdk/lib/_internal/pub/lib...
dart/sdk/lib/_internal/pub/lib/src/path_rep.dart:8: bool inCache;
On 2014/02/06 02:41:30, tapted wrote:
> On 2014/01/29 05:42:57, pajamallama wrote:
> > On 2014/01/29 02:52:02, tapted wrote:
> > > nit: leading underscore
> > 
> > Done.
> 
> Is it? ;)

Well, it was done then I realised I needed to be able to access this info
publicly.

https://codereview.appspot.com/57650043/diff/140001/dart/sdk/lib/_internal/pu...
File dart/sdk/lib/_internal/pub/lib/src/io.dart (right):

https://codereview.appspot.com/57650043/diff/140001/dart/sdk/lib/_internal/pu...
dart/sdk/lib/_internal/pub/lib/src/io.dart:91: createDir(symlink)])
On 2014/02/06 02:41:30, tapted wrote:
> nit: indent 1 more space

Done.

https://codereview.appspot.com/57650043/diff/140001/dart/sdk/lib/_internal/pu...
dart/sdk/lib/_internal/pub/lib/src/io.dart:102: /// Reads the contents of the
text file [file].
On 2014/02/06 02:41:30, tapted wrote:
> nit: blank line before

Done.

https://codereview.appspot.com/57650043/diff/140001/dart/sdk/lib/_internal/pu...
dart/sdk/lib/_internal/pub/lib/src/io.dart:251: }).catchError((_) => []);
On 2014/02/06 02:41:30, tapted wrote:
> maybe log the error?

Done.

[patrick.laub, 2014-03-11 13:24:58]

I think I've narrowed down the problem with this quota business. This code with
the latest rebase works in Dartium, but throws "Failed to execute 'requestQuota'
on 'DeprecatedStorageQuota'" in JavaScript.

My guess is that the Dart2JS compiler should be using the new (?) promises Quota
API http://www.w3.org/TR/2013/WD-quota-api-20131105/#temporary-vs-persistent 
(implemented here:
https://chromium.googlesource.com/chromium/blink/+/8d78e1968ca65489603c77c03d...
) but it isn't.

P.S That CL says it put this API behind a flag but I can't find any flag about
quotas (searching for 'Quota' in chromestatus.com says the Quota API has been
"enabled by default" since chrome 14. That seems way too old?).

[tapted, 2014-03-11 23:58:31]

On 2014/03/11 13:24:58, patrick.laub wrote:
> I think I've narrowed down the problem with this quota business. This code
with
> the latest rebase works in Dartium, but throws "Failed to execute
'requestQuota'
> on 'DeprecatedStorageQuota'" in JavaScript.
> 
> My guess is that the Dart2JS compiler should be using the new (?) promises
Quota
> API http://www.w3.org/TR/2013/WD-quota-api-20131105/#temporary-vs-persistent 
> (implemented here:
>
https://chromium.googlesource.com/chromium/blink/+/8d78e1968ca65489603c77c03d...
> ) but it isn't.
> 
> P.S That CL says it put this API behind a flag but I can't find any flag about
> quotas (searching for 'Quota' in http://chromestatus.com says the Quota API
has been
> "enabled by default" since chrome 14. That seems way too old?).

Not sure if this will help, but does updating to the Dart 1.2 SDK affect things
at all?

[patrick.laub, 2014-03-12 00:02:10]

On 2014/03/11 23:58:31, tapted wrote:
> On 2014/03/11 13:24:58, patrick.laub wrote:
> > I think I've narrowed down the problem with this quota business. This code
> with
> > the latest rebase works in Dartium, but throws "Failed to execute
> 'requestQuota'
> > on 'DeprecatedStorageQuota'" in JavaScript.
> > 
> > My guess is that the Dart2JS compiler should be using the new (?) promises
> Quota
> > API http://www.w3.org/TR/2013/WD-quota-api-20131105/#temporary-vs-persistent

> > (implemented here:
> >
>
https://chromium.googlesource.com/chromium/blink/+/8d78e1968ca65489603c77c03d...
> > ) but it isn't.
> > 
> > P.S That CL says it put this API behind a flag but I can't find any flag
about
> > quotas (searching for 'Quota' in http://chromestatus.com says the Quota API
> has been
> > "enabled by default" since chrome 14. That seems way too old?).
> 
> Not sure if this will help, but does updating to the Dart 1.2 SDK affect
things
> at all?

Na, I tested on 1.2 unfortunately. I'm thinking that I'll pull out the IO
simplifications
from this patch and worry about the persistent storage thing by itself.
and treat the quota issue separately.

[patrick.laub, 2014-03-12 00:02:10]

On 2014/03/11 23:58:31, tapted wrote:
> On 2014/03/11 13:24:58, patrick.laub wrote:
> > I think I've narrowed down the problem with this quota business. This code
> with
> > the latest rebase works in Dartium, but throws "Failed to execute
> 'requestQuota'
> > on 'DeprecatedStorageQuota'" in JavaScript.
> > 
> > My guess is that the Dart2JS compiler should be using the new (?) promises
> Quota
> > API http://www.w3.org/TR/2013/WD-quota-api-20131105/#temporary-vs-persistent

> > (implemented here:
> >
>
https://chromium.googlesource.com/chromium/blink/+/8d78e1968ca65489603c77c03d...
> > ) but it isn't.
> > 
> > P.S That CL says it put this API behind a flag but I can't find any flag
about
> > quotas (searching for 'Quota' in http://chromestatus.com says the Quota API
> has been
> > "enabled by default" since chrome 14. That seems way too old?).
> 
> Not sure if this will help, but does updating to the Dart 1.2 SDK affect
things
> at all?

Na, I tested on 1.2 unfortunately. I'm thinking that I'll pull out the IO
simplifications
from this patch and worry about the persistent storage thing by itself.
and treat the quota issue separately.