add server.status notification

add server.status notification

BUG=
R=brianwilkerson@google.com, paulberry@google.com

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

[Brian Wilkerson, 2014-05-23 13:19:26]

Let's talk about the structure of the status notification, then LGTM.

[Paul Berry, 2014-05-23 15:56:45]

lgtm as a starting point, but I think we should probably have a discussion about
the throttling issue before we get too much further along in analysis server
development.

https://codereview.chromium.org/298823007/diff/1/pkg/analysis_server/lib/src/...
File pkg/analysis_server/lib/src/analysis_server.dart (right):

https://codereview.chromium.org/298823007/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/analysis_server.dart:237: return;
This technique for throttling status notifications breaks what is IMO one of the
primary use case for having status notifications in the first place: to report
the current state of analysis to the user so that if analysis is slow or hangs,
they know what it is currently trying to do.  As written, we drop any status
notifications that occur within 500ms of the previously reported status.  That's
fine if all analysis is quick, but if we analyze file A in 100ms, and then
analyzing file B takes 30s, the status seen by the user will indicate that we're
analyzing file A that entire time.

That could lead to some really confusing bug reports if a user ever encounters a
case where analysis hangs.

Unfortunately due to the single-threaded nature of dart, this isn't going to be
trivial to fix--we probably have to create a new isolate.  Alternatively, we
could hold off on throttling until we have enough of an implementation working
that we can measure performance and confirm that it's really needed.

https://codereview.chromium.org/298823007/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/analysis_server.dart:244: shortMessage = 'Analyzing
${notice.source.shortName}';
Similar concern here.  As you've arranged things, sendStatusNotification() won't
be called until analysis for the given file has completed.  So any time we say
"analyzing foo" we have actually already finished analyzing foo and moved on to
analyze some other file.  Again, not a problem if analysis is quick, but if a
bug makes analysis slow for a certain file, the status messages aren't going to
properly reflect which file is the slow one.

https://codereview.chromium.org/298823007/diff/1/pkg/analysis_server/test/ana...
File pkg/analysis_server/test/analysis_server_test.dart (right):

https://codereview.chromium.org/298823007/diff/1/pkg/analysis_server/test/ana...
pkg/analysis_server/test/analysis_server_test.dart:96:
assertStatusNotification(notifications[3], false);
Not a problem with this test, but a heads up for later: at some point we are
probably going to need a way to disable throttling for unit testing purposes (or
put it in a mode where it uses some sort of mock instead of a timer).  Otherwise
our tests will be nondeterministic because their behavior will depend on how
much analysis gets done in 500ms.

[danrubel, 2014-05-27 12:31:42]

Per discussion, reworked to send notifications on analysis start/stop.
PTAL

[Paul Berry, 2014-05-27 16:09:26]

lgtm

https://codereview.chromium.org/298823007/diff/40001/pkg/analysis_server/lib/...
File pkg/analysis_server/lib/src/analysis_server.dart (right):

https://codereview.chromium.org/298823007/diff/40001/pkg/analysis_server/lib/...
pkg/analysis_server/lib/src/analysis_server.dart:190: AnalysisResult result =
context.performAnalysisTask();
I think the call to sendStatusNotification(contextId) needs to go here, just
before the call to performAnalysisTask().  That way if the first analysis task
freezes (or takes a long time to execute) the user will get an accurate picture
of what's happening.

[danrubel, 2014-05-28 02:02:00]

https://codereview.chromium.org/298823007/diff/40001/pkg/analysis_server/lib/...
File pkg/analysis_server/lib/src/analysis_server.dart (right):

https://codereview.chromium.org/298823007/diff/40001/pkg/analysis_server/lib/...
pkg/analysis_server/lib/src/analysis_server.dart:190: AnalysisResult result =
context.performAnalysisTask();
On 2014/05/27 16:09:26, Paul Berry wrote:
> I think the call to sendStatusNotification(contextId) needs to go here, just
> before the call to performAnalysisTask().  That way if the first analysis task
> freezes (or takes a long time to execute) the user will get an accurate
picture
> of what's happening.

Good suggestion. Done.

[danrubel, 2014-05-28 02:25:25]

Committed patchset #6 manually as r36727 (presubmit successful).