Redo LocationManager to use HTML5 History API

Redo LocationManager to use HTML5 History API

Make a Class ServiceObject
Introduce application Panes
Intercept all "application link" click events and manually dispatch them to LocationManager.
Add <nav-control> that adds back and forward buttons

Rolled in:

* https://codereview.chromium.org/306723002
* Revert to HttpVM

R=turnidge@google.com

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

[turnidge, 2014-06-04 18:02:24]

lgtm overall, some relatively minor suggestions below.

Also, what do you think of sneaking the WebSocketVm -> HttpVm diff into your CL?

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/service_html.dart (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/service_html.dart:42: 'observatory':
'observatory'
Header names are usually Dashed-With-Capital-Letters.  Instead of using
'observatory' as the payload, how about a version string, e.g:

'Observatory-Version': '1.0'

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/app/location_manager.dart (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/app/location_manager.dart:23: void
_onLocationChange(_);
Do you know what the _ parameter is supposed to be?  The subclasses know that
they do not use the parameter, but it might be good for the parent class to
declare it.

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/app/location_manager.dart:31: _lastUrl =
url;
Could move this line inside above "if".

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/class_tree.dart (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/class_tree.dart:104: // Subtract 1
to get 0 based indexing.
Move the above comment inside the "try"

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/class_tree.html (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/class_tree.html:19: .table tfoot >
tr > td {
I wonder if you could get away with just...

.table tr > th {

instead of the 6 selectors above.

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/class_tree.html:105: <tr template
repeat="{{row in tree.rows }}" style="{{}}">
What is the...

style="{{}}"

...about?  Could you just drop the style tag?  Or use style=""?

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/class_view.html (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/class_view.html:27: <template
if="{{ cls.isAbstract }}">
Thanks for doing this.

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/code_view.html (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/code_view.html:60:
<nav-control></nav-control>
Did you make sure to add nav-control everywhere?

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/instance_ref.html (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/instance_ref.html:41: <a
on-click="{{ goto }}" href="{{ url }}">{{ ref['user_name'] }}</a>
It's a shame we have to do this with all of our links.  Maybe we should make our
own custom link element that does some of this for us.  (Not for this change).

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/nav_bar.html (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/nav_bar.html:185: <button
class="black" on-click="{{ forward }}">&#9654;</button>
Which symbols did you end up using for these?

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/service/object.dart (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:577: Future
getClassHierarchy() {
Can we add a type param to this Future?   I think it is Future<Class>.  Also a
short comment on the function?

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:581: Future
_processClasses(ServiceMap classList) {
Maybe name this _loadClasses?

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:598:
rootClasses.add(cls);
I thought that Object was the root class.  Am I being confused?  Which other
classes have a null superClass?

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:996: }
Instead of empty body, just use a ';'?

[Cutch, 2014-06-04 20:28:57]

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/service_html.dart (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/service_html.dart:42: 'observatory':
'observatory'
On 2014/06/04 18:02:24, turnidge wrote:
> Header names are usually Dashed-With-Capital-Letters.  Instead of using
> 'observatory' as the payload, how about a version string, e.g:
> 
> 'Observatory-Version': '1.0'

Done.

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/app/location_manager.dart (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/app/location_manager.dart:23: void
_onLocationChange(_);
On 2014/06/04 18:02:24, turnidge wrote:
> Do you know what the _ parameter is supposed to be?  The subclasses know that
> they do not use the parameter, but it might be good for the parent class to
> declare it.

Done.

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/app/location_manager.dart:31: _lastUrl =
url;
On 2014/06/04 18:02:24, turnidge wrote:
> Could move this line inside above "if".

Done.

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/class_tree.dart (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/class_tree.dart:104: // Subtract 1
to get 0 based indexing.
On 2014/06/04 18:02:24, turnidge wrote:
> Move the above comment inside the "try"

Done.

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/class_tree.html (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/class_tree.html:105: <tr template
repeat="{{row in tree.rows }}" style="{{}}">
On 2014/06/04 18:02:24, turnidge wrote:
> What is the...
> 
> style="{{}}"
> 
> ...about?  Could you just drop the style tag?  Or use style=""?

I just removed it.

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/code_view.html (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/code_view.html:60:
<nav-control></nav-control>
On 2014/06/04 18:02:24, turnidge wrote:
> Did you make sure to add nav-control everywhere?

Just double checked. I had missed the error pages, added it there.

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/instance_ref.html (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/instance_ref.html:41: <a
on-click="{{ goto }}" href="{{ url }}">{{ ref['user_name'] }}</a>
On 2014/06/04 18:02:24, turnidge wrote:
> It's a shame we have to do this with all of our links.  Maybe we should make
our
> own custom link element that does some of this for us.  (Not for this change).

Agreed. We should have an <app-link> tag or something.

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/elements/nav_bar.html (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/elements/nav_bar.html:185: <button
class="black" on-click="{{ forward }}">&#9654;</button>
On 2014/06/04 18:02:24, turnidge wrote:
> Which symbols did you end up using for these?

These are both large black triangles.

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
File runtime/bin/vmservice/client/lib/src/service/object.dart (right):

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:577: Future
getClassHierarchy() {
On 2014/06/04 18:02:24, turnidge wrote:
> Can we add a type param to this Future?   I think it is Future<Class>.  Also a
> short comment on the function?

Done.

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:581: Future
_processClasses(ServiceMap classList) {
On 2014/06/04 18:02:24, turnidge wrote:
> Maybe name this _loadClasses?

Woah! I made that rename before reading this comment. Names, names, names.

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:598:
rootClasses.add(cls);
On 2014/06/04 18:02:24, turnidge wrote:
> I thought that Object was the root class.  Am I being confused?  Which other
> classes have a null superClass?

Internal VM classes (Field, Function, etc).

Note, I renamed this to "_buildClassHiearchy".

https://codereview.chromium.org/308673012/diff/60001/runtime/bin/vmservice/cl...
runtime/bin/vmservice/client/lib/src/service/object.dart:996: }
On 2014/06/04 18:02:24, turnidge wrote:
> Instead of empty body, just use a ';'?

Done.

[Cutch, 2014-06-04 21:01:38]

Committed patchset #5 manually as r37014 (presubmit successful).