Add property base to the Uri class

Add property base to the Uri class

Uri.base returns the natural base URI for the current platform.

R=ahe@google.com, floitsch@google.com, iposva@google.com, rnystrom@google.com
BUG=

Committed: https://code.google.com/p/dart/source/detail?r=27877

[Søren Gjesse, 2013-09-17 11:18:12]

Ivan,

I am including bin/directory.h and bin/utils.h in lib/uri.cc (using Directory
and OSError) - which is not good. When the change is otherwise approved I will
look into that issue.

[floitsch, 2013-09-17 11:49:10]

LGTM. Please wait for the OK from dart2js and VM.

https://codereview.chromium.org/23686021/diff/3001/runtime/vm/bootstrap_nativ...
File runtime/vm/bootstrap_natives.h (right):

https://codereview.chromium.org/23686021/diff/3001/runtime/vm/bootstrap_nativ...
runtime/vm/bootstrap_natives.h:306: V(Uri_currentDirectory, 0)                  
                               \
nit: one missing space.

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/_internal/lib/core...
File sdk/lib/_internal/lib/core_patch.dart (right):

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/_internal/lib/core...
sdk/lib/_internal/lib/core_patch.dart:7: import 'dart:_foreign_helper' show JS;
I think the dart2js doesn't want JS except in the _foreign_helper.

[Søren Gjesse, 2013-09-17 11:59:53]

https://codereview.chromium.org/23686021/diff/3001/runtime/vm/bootstrap_nativ...
File runtime/vm/bootstrap_natives.h (right):

https://codereview.chromium.org/23686021/diff/3001/runtime/vm/bootstrap_nativ...
runtime/vm/bootstrap_natives.h:306: V(Uri_currentDirectory, 0)                  
                               \
On 2013/09/17 11:49:11, floitsch wrote:
> nit: one missing space.

Done.

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/_internal/lib/core...
File sdk/lib/_internal/lib/core_patch.dart (right):

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/_internal/lib/core...
sdk/lib/_internal/lib/core_patch.dart:7: import 'dart:_foreign_helper' show JS;
On 2013/09/17 11:49:11, floitsch wrote:
> I think the dart2js doesn't want JS except in the _foreign_helper.

I don't understand this comment.

[ahe, 2013-09-17 12:23:48]

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/_internal/lib/core...
File sdk/lib/_internal/lib/core_patch.dart (right):

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/_internal/lib/core...
sdk/lib/_internal/lib/core_patch.dart:7: import 'dart:_foreign_helper' show JS;
On 2013/09/17 11:59:53, Søren Gjesse wrote:
> On 2013/09/17 11:49:11, floitsch wrote:
> > I think the dart2js doesn't want JS except in the _foreign_helper.
> 
> I don't understand this comment.

It would be better to add a "primitive" to the class Primitives in
js_helper.dart.

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/core/uri.dart
File sdk/lib/core/uri.dart (right):

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/core/uri.dart#newc...
sdk/lib/core/uri.dart:364: * When running in the browser this will be the
current URL (from
the browser -> a browser

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/core/uri.dart#newc...
sdk/lib/core/uri.dart:367: * When running in the standalone VM this will be a
file URI
When not running in a browser...

[Søren Gjesse, 2013-09-17 14:12:10]

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/_internal/lib/core...
File sdk/lib/_internal/lib/core_patch.dart (right):

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/_internal/lib/core...
sdk/lib/_internal/lib/core_patch.dart:7: import 'dart:_foreign_helper' show JS;
On 2013/09/17 12:23:48, ahe wrote:
> On 2013/09/17 11:59:53, Søren Gjesse wrote:
> > On 2013/09/17 11:49:11, floitsch wrote:
> > > I think the dart2js doesn't want JS except in the _foreign_helper.
> > 
> > I don't understand this comment.
> 
> It would be better to add a "primitive" to the class Primitives in
> js_helper.dart.

OK, that was also what floitsch@ ment - I just saw the direct use of JS in quite
a few places. Added currentUri to Primitives.

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/core/uri.dart
File sdk/lib/core/uri.dart (right):

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/core/uri.dart#newc...
sdk/lib/core/uri.dart:364: * When running in the browser this will be the
current URL (from
On 2013/09/17 12:23:48, ahe wrote:
> the browser -> a browser

Done.

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/core/uri.dart#newc...
sdk/lib/core/uri.dart:367: * When running in the standalone VM this will be a
file URI
On 2013/09/17 12:23:48, ahe wrote:
> When not running in a browser...

Done.

[ahe, 2013-09-17 14:17:39]

LGTM!

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/core/uri.dart
File sdk/lib/core/uri.dart (right):

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/core/uri.dart#newc...
sdk/lib/core/uri.dart:364: * When running in the browser this will be the
current URL (from
On 2013/09/17 14:12:10, Søren Gjesse wrote:
> On 2013/09/17 12:23:48, ahe wrote:
> > the browser -> a browser
> 
> Done.

I don't see this change.

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/core/uri.dart#newc...
sdk/lib/core/uri.dart:367: * When running in the standalone VM this will be a
file URI
On 2013/09/17 14:12:10, Søren Gjesse wrote:
> On 2013/09/17 12:23:48, ahe wrote:
> > When not running in a browser...
> 
> Done.

Ditto.

[Søren Gjesse, 2013-09-17 15:57:35]

Vijay,

For the Uri.base to return "window.location.href" in Dartium (and not the file
URI for the current directory which I assume it will try to do right now) I
assume some additional Dartium bindings are needed. Can you shed some light on
this?

[vsm, 2013-09-17 16:13:00]

On 2013/09/17 15:57:35, Søren Gjesse wrote:
> Vijay,
> 
> For the Uri.base to return "window.location.href" in Dartium (and not the file
> URI for the current directory which I assume it will try to do right now) I
> assume some additional Dartium bindings are needed. Can you shed some light on
> this?

Yes, we'll need to something similar to how print is forwarded to console.log in
Dartium.  E.g., see:

runtime/lib/print_patch.dart

Dartium (from C++) sets _printClosure to console.log.  If you provide a similar
hook, we can set that in the Dartium bindings.  See line 158 for the Dartium
side of it:

http://src.chromium.org/viewvc/multivm/trunk/webkit/Source/bindings/dart/Dart...

[Ivan Posva, 2013-09-17 21:28:04]

Please provide a hook that the embedder (Dartium or standalone) can use to
provide the correct implementation. That should also enable you to just use
dart:io based code in the standalone case.

Thanks,
-Ivan

https://codereview.chromium.org/23686021/diff/12001/runtime/lib/uri.cc
File runtime/lib/uri.cc (right):

https://codereview.chromium.org/23686021/diff/12001/runtime/lib/uri.cc#newcode6
runtime/lib/uri.cc:6: #include "bin/utils.h"
Yes, this needs to be fixed.

https://codereview.chromium.org/23686021/diff/12001/runtime/lib/uri_patch.dart
File runtime/lib/uri_patch.dart (right):

https://codereview.chromium.org/23686021/diff/12001/runtime/lib/uri_patch.dar...
runtime/lib/uri_patch.dart:14: static _currentDirectory() native
"Uri_currentDirectory";
Why does this have to be a native method? Can't this just call into the dart:io
code to get the current directory?

[Søren Gjesse, 2013-09-18 08:37:02]

PTAL

Added embedder hook and some support for JavaScript shells.

[floitsch, 2013-09-18 09:06:26]

DBC. (letting VM and dart2js give the LGTM).

If some things don't work on windows, shouldn't there be status-file entries?

https://codereview.chromium.org/23686021/diff/24001/sdk/lib/core/uri.dart
File sdk/lib/core/uri.dart (right):

https://codereview.chromium.org/23686021/diff/24001/sdk/lib/core/uri.dart#new...
sdk/lib/core/uri.dart:364: * When running in a browser this will be the current
URL (from
is the current URL

https://codereview.chromium.org/23686021/diff/24001/sdk/lib/core/uri.dart#new...
sdk/lib/core/uri.dart:367: * When not running in a browser this will be a file
URI referencing
is the file URI ...

[Ivan Posva, 2013-09-24 07:37:41]

LGTM

-Ivan

[Ivan Posva, 2013-09-24 07:38:17]

On 2013/09/24 07:37:41, Ivan Posva wrote:
> LGTM
> 
> -Ivan

And please work with Vijay and Siva to get this properly hooked into Dartium
bindings.

Thanks,
-Ivan

[Søren Gjesse, 2013-09-25 09:17:07]

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/core/uri.dart
File sdk/lib/core/uri.dart (right):

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/core/uri.dart#newc...
sdk/lib/core/uri.dart:364: * When running in the browser this will be the
current URL (from
On 2013/09/17 14:17:39, ahe wrote:
> On 2013/09/17 14:12:10, Søren Gjesse wrote:
> > On 2013/09/17 12:23:48, ahe wrote:
> > > the browser -> a browser
> > 
> > Done.
> 
> I don't see this change.

Now it's there.

https://codereview.chromium.org/23686021/diff/3001/sdk/lib/core/uri.dart#newc...
sdk/lib/core/uri.dart:367: * When running in the standalone VM this will be a
file URI
On 2013/09/17 14:17:39, ahe wrote:
> On 2013/09/17 14:12:10, Søren Gjesse wrote:
> > On 2013/09/17 12:23:48, ahe wrote:
> > > When not running in a browser...
> > 
> > Done.
> 
> Ditto.

Now it's there.

https://codereview.chromium.org/23686021/diff/12001/runtime/lib/uri.cc
File runtime/lib/uri.cc (right):

https://codereview.chromium.org/23686021/diff/12001/runtime/lib/uri.cc#newcode6
runtime/lib/uri.cc:6: #include "bin/utils.h"
On 2013/09/17 21:28:04, Ivan Posva wrote:
> Yes, this needs to be fixed.

Done.

https://codereview.chromium.org/23686021/diff/12001/runtime/lib/uri_patch.dart
File runtime/lib/uri_patch.dart (right):

https://codereview.chromium.org/23686021/diff/12001/runtime/lib/uri_patch.dar...
runtime/lib/uri_patch.dart:14: static _currentDirectory() native
"Uri_currentDirectory";
On 2013/09/17 21:28:04, Ivan Posva wrote:
> Why does this have to be a native method? Can't this just call into the
dart:io
> code to get the current directory?

Done.

https://codereview.chromium.org/23686021/diff/24001/sdk/lib/core/uri.dart
File sdk/lib/core/uri.dart (right):

https://codereview.chromium.org/23686021/diff/24001/sdk/lib/core/uri.dart#new...
sdk/lib/core/uri.dart:364: * When running in a browser this will be the current
URL (from
On 2013/09/18 09:06:26, floitsch wrote:
> is the current URL

Done.

https://codereview.chromium.org/23686021/diff/24001/sdk/lib/core/uri.dart#new...
sdk/lib/core/uri.dart:367: * When not running in a browser this will be a file
URI referencing
On 2013/09/18 09:06:26, floitsch wrote:
> is the file URI ...

Done.

[Søren Thygesen Gjesse, 2013-09-25 13:52:34]

Committed patchset #7 manually as r27877 (presubmit successful).