Show progress on upload and resolve.

Show progress on upload and resolve.

BUG=https://code.google.com/p/dart/issues/detail?id=7397
R=nweiz@google.com

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

[nweiz, 2013-05-28 18:46:41]

A couple nits, otherwise LGTM.

https://codereview.chromium.org/16159003/diff/2001/sdk/lib/_internal/pub/lib/...
File sdk/lib/_internal/pub/lib/src/log.dart (right):

https://codereview.chromium.org/16159003/diff/2001/sdk/lib/_internal/pub/lib/...
sdk/lib/_internal/pub/lib/src/log.dart:192: if (_progressTimer != null) throw
new StateError("Already in progress.");
It seems weird that most of the time when a different log call happens, the old
progress indicator stops, but when [progress] in particular is called, it throws
an error.

https://codereview.chromium.org/16159003/diff/2001/sdk/lib/_internal/pub/lib/...
sdk/lib/_internal/pub/lib/src/log.dart:205: _stopProgress() {
A brief docstring would be nice here.

https://codereview.chromium.org/16159003/diff/2001/sdk/lib/_internal/pub/lib/...
sdk/lib/_internal/pub/lib/src/log.dart:217: }
It feels like we should probably null out _progressMessage here.

[Bob Nystrom, 2013-05-28 19:55:12]

Thanks!

https://codereview.chromium.org/16159003/diff/2001/sdk/lib/_internal/pub/lib/...
File sdk/lib/_internal/pub/lib/src/log.dart (right):

https://codereview.chromium.org/16159003/diff/2001/sdk/lib/_internal/pub/lib/...
sdk/lib/_internal/pub/lib/src/log.dart:192: if (_progressTimer != null) throw
new StateError("Already in progress.");
On 2013/05/28 18:46:41, nweiz wrote:
> It seems weird that most of the time when a different log call happens, the
old
> progress indicator stops, but when [progress] in particular is called, it
throws
> an error.

At least right now, the progress indicator isn't designed to nest or be
pre-empted by itself. This just asserts that.

Note that the API for using this is explicitly scoped, so you couldn't hit this
error when you're trying to have one progress indicator *followed* by another.
It only happens if you try to nest them.

https://codereview.chromium.org/16159003/diff/2001/sdk/lib/_internal/pub/lib/...
sdk/lib/_internal/pub/lib/src/log.dart:205: _stopProgress() {
On 2013/05/28 18:46:41, nweiz wrote:
> A brief docstring would be nice here.

Done.

https://codereview.chromium.org/16159003/diff/2001/sdk/lib/_internal/pub/lib/...
sdk/lib/_internal/pub/lib/src/log.dart:217: }
On 2013/05/28 18:46:41, nweiz wrote:
> It feels like we should probably null out _progressMessage here.

Done.

[Bob Nystrom, 2013-05-28 19:55:46]

Committed patchset #3 manually as r23306 (presubmit successful).

[nweiz, 2013-05-28 20:04:39]

https://codereview.chromium.org/16159003/diff/2001/sdk/lib/_internal/pub/lib/...
File sdk/lib/_internal/pub/lib/src/log.dart (right):

https://codereview.chromium.org/16159003/diff/2001/sdk/lib/_internal/pub/lib/...
sdk/lib/_internal/pub/lib/src/log.dart:192: if (_progressTimer != null) throw
new StateError("Already in progress.");
On 2013/05/28 19:55:12, Bob Nystrom wrote:
> On 2013/05/28 18:46:41, nweiz wrote:
> > It seems weird that most of the time when a different log call happens, the
> old
> > progress indicator stops, but when [progress] in particular is called, it
> throws
> > an error.
> 
> At least right now, the progress indicator isn't designed to nest or be
> pre-empted by itself. This just asserts that.
> 
> Note that the API for using this is explicitly scoped, so you couldn't hit
this
> error when you're trying to have one progress indicator *followed* by another.
> It only happens if you try to nest them.

It just seems weird that in this block of code there could be some functions
you're not allowed to call because of their logging behavior.