Initial GUI for review

Initial GUI for VM observatory, some notes:

*) Minimal feature set: isolate list and stack trace for a given isolate.
*) This CL only includes the Polymer application.

Looking for review Polymer application structure and style.

[justinfagnani, 2013-09-24 23:20:16]

Please add more documentation, it's hard to see how everything is wired
together.

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
File runtime/bin/vmservice/client/observatory/index.html (right):

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/index.html:11:
<observatory-application></observatory-application>
indent 2 spaces

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
File runtime/bin/vmservice/client/observatory/observatory_main.dart (right):

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/observatory_main.dart:1: library
observatory_main;
copyright notices on all source files

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/observatory_main.dart:7: import
'package:observatory/observatory_elements.dart';
all of these appear to be unused imports. if they're needed to keep something
from breaking, please comment that (and file a bug?)

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
File
runtime/bin/vmservice/client/observatory/packages/observatory/observatory.dart
(right):

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/observatory.dart:1:
library observatory;
why do we have files checked in under packages/ ?

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
File
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory/anchor.dart
(right):

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory/anchor.dart:3:
class Anchor extends Object with ObservableMixin {
What does this class do? Is it an HTML <a> tag of some sort? Docs please :)

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
File
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory/request_manager.dart
(right):

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory/request_manager.dart:3:
typedef void RequestResponseInterceptor();
what is this?

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory/request_manager.dart:37:
throw new UnimplementedError('RequestManager is abstract.');
unnecessary. just declare RequestManager as an abstract class and get as an
abstract method by leaving off the body.

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory/request_manager.dart:43:
Future get(String url) {
rather than making it the responsibility of get() to do the right thing and call
parseResponses or setResponseError, just have the implementation return a
Future<String> and have the caller handle it. This would mean that get() is
implemented in RequestManager and has the basic structure of this method here,
but instead of calling HttpRequest.getString() either calls an abstract instance
method that a subclass implements or invokes a closure that's provided in the
constructor.

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
File
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/isolate_summary.html
(right):

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/isolate_summary.html:9:
<div class="row">
2 space indents

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/isolate_summary.html:9:
<div class="row">
looks like you're emulating a table, why not use a <table>?

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
File
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/message_viewer.dart
(right):

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/message_viewer.dart:13:
notifyProperty(this, const Symbol('messageType'));
I believe we have symbol literals in the VM now

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
File
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/stack_trace.html
(right):

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/stack_trace.html:3:
<html>
you don't need this boilerplate unless you have multiple polymer elements per
files. Just use <polymer-element> as your root tag

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/stack_trace.html:21:
<td>{{$index}}</td>
where does $index come from? I'm having a hard time seeing where trace is
populated in StackTrace

[Cutch, 2013-09-25 18:10:19]

I will update the CL shortly with a more commented version which incorporates
all of your feedback so far.

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
File runtime/bin/vmservice/client/observatory/index.html (right):

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/index.html:11:
<observatory-application></observatory-application>
On 2013/09/24 23:20:16, justinfagnani wrote:
> indent 2 spaces

Done.

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
File runtime/bin/vmservice/client/observatory/observatory_main.dart (right):

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/observatory_main.dart:7: import
'package:observatory/observatory_elements.dart';
On 2013/09/24 23:20:16, justinfagnani wrote:
> all of these appear to be unused imports. if they're needed to keep something
> from breaking, please comment that (and file a bug?)

Done.

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
File
runtime/bin/vmservice/client/observatory/packages/observatory/observatory.dart
(right):

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/observatory.dart:1:
library observatory;
On 2013/09/24 23:20:16, justinfagnani wrote:
> why do we have files checked in under packages/ ?

These aren't checked in and this CL doesn't have the final directory structure.

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
File
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory/request_manager.dart
(right):

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory/request_manager.dart:37:
throw new UnimplementedError('RequestManager is abstract.');
On 2013/09/24 23:20:16, justinfagnani wrote:
> unnecessary. just declare RequestManager as an abstract class and get as an
> abstract method by leaving off the body. 

Done.

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory/request_manager.dart:43:
Future get(String url) {
On 2013/09/24 23:20:16, justinfagnani wrote:
> rather than making it the responsibility of get() to do the right thing and
call
> parseResponses or setResponseError, just have the implementation return a
> Future<String> and have the caller handle it. This would mean that get() is
> implemented in RequestManager and has the basic structure of this method here,
> but instead of calling HttpRequest.getString() either calls an abstract
instance
> method that a subclass implements or invokes a closure that's provided in the
> constructor.

Done.

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
File
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/isolate_summary.html
(right):

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/isolate_summary.html:9:
<div class="row">
On 2013/09/24 23:20:16, justinfagnani wrote:
> 2 space indents

The Editor has a bug which inserts tabs when editing .html files:
https://code.google.com/p/dart/issues/detail?id=13557

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/isolate_summary.html:9:
<div class="row">
On 2013/09/24 23:20:16, justinfagnani wrote:
> looks like you're emulating a table, why not use a <table>?

Mostly aesthetics. I'll play with a table and see if I like it.

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
File
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/message_viewer.dart
(right):

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/message_viewer.dart:13:
notifyProperty(this, const Symbol('messageType'));
On 2013/09/24 23:20:16, justinfagnani wrote:
> I believe we have symbol literals in the VM now

I will switch to symbol literals in the future.

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
File
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/stack_trace.html
(right):

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/stack_trace.html:3:
<html>
On 2013/09/24 23:20:16, justinfagnani wrote:
> you don't need this boilerplate unless you have multiple polymer elements per
> files. Just use <polymer-element> as your root tag

Thanks! Done for all custom elements.

https://codereview.chromium.org/24471002/diff/1/runtime/bin/vmservice/client/...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory_elements/stack_trace.html:21:
<td>{{$index}}</td>
On 2013/09/24 23:20:16, justinfagnani wrote:
> where does $index come from? I'm having a hard time seeing where trace is
> populated in StackTrace

trace is populated in <message-viewer> which is the main switch point for
instantiating message viewers. $index is actually a left over from web_ui which
I thought polymer_expressions was going to support.

[justinfagnani, 2013-09-26 01:46:23]

lgtm

Still need copyright notices on all files

https://codereview.chromium.org/24471002/diff/16001/runtime/bin/vmservice/cli...
File
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory/anchor.dart
(right):

https://codereview.chromium.org/24471002/diff/16001/runtime/bin/vmservice/cli...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory/anchor.dart:6:
class Anchor extends Object with ObservableMixin {
I'd rename this to something like Links or Uris

[Cutch, 2013-09-26 20:34:21]

Added copyright notices.

https://codereview.chromium.org/24471002/diff/16001/runtime/bin/vmservice/cli...
File
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory/anchor.dart
(right):

https://codereview.chromium.org/24471002/diff/16001/runtime/bin/vmservice/cli...
runtime/bin/vmservice/client/observatory/packages/observatory/src/observatory/anchor.dart:6:
class Anchor extends Object with ObservableMixin {
On 2013/09/26 01:46:24, justinfagnani wrote:
> I'd rename this to something like Links or Uris

Done.