Address shortcomings in documentation on Isolate class.

Address shortcomings in documentation on Isolate class.

Addresses issued #26597 and #26627.

BUG= http://dartbug.com/26597 http://dartbug.com/26627
R=eernst@google.com, floitsch@google.com

Committed: https://github.com/dart-lang/sdk/commit/9487e107bb53a18eb05c663202dffd935c24d29a

[Lasse Reichstein Nielsen, 2016-06-13 05:37:46]

lrn@google.com changed reviewers:
+ eernst@google.com, floitsch@google.com

[eernst, 2016-06-14 11:36:47]

LGTM, with a couple of minor comments.

https://codereview.chromium.org/2062683002/diff/1/sdk/lib/isolate/isolate.dart
File sdk/lib/isolate/isolate.dart (right):

https://codereview.chromium.org/2062683002/diff/1/sdk/lib/isolate/isolate.dar...
sdk/lib/isolate/isolate.dart:171: * that can be called with a single argument -
Might as well use ',' because the single hyphen is so different from the proper
em-dash, and a comma would not be unnatural here.

https://codereview.chromium.org/2062683002/diff/1/sdk/lib/isolate/isolate.dar...
sdk/lib/isolate/isolate.dart:176: * as long as they *can* be called with just a
single argument.
There's a singular/plural issue here, and I also think that the emphasis is
slightly too informal for long time use (this whole sentence is just there for
emphasis), so I'd suggest: 'as long as it can'.

https://codereview.chromium.org/2062683002/diff/1/sdk/lib/isolate/isolate.dar...
sdk/lib/isolate/isolate.dart:178: * allowed.
A bit of confusion here, too. Maybe it would work to say 'It is not allowed to
pass a function expression nor an instance method tear-off.'

https://codereview.chromium.org/2062683002/diff/1/sdk/lib/isolate/isolate.dar...
sdk/lib/isolate/isolate.dart:313: * but before already enqueued event loop
events that hasn't been processed
'after any previously sent command messages, but before any enqueued event loop
events.'

If event loop events can indeed be enqueued even though they have already been
processed then we can add 'that have not yet been processed'. Otherwise we might
as well put a period here, and then, possibly, add a separate sentence
explaining that processed events cannot exist in the queue.

[floitsch, 2016-06-16 08:36:35]

Dbc

https://codereview.chromium.org/2062683002/diff/1/sdk/lib/isolate/isolate.dart
File sdk/lib/isolate/isolate.dart (right):

https://codereview.chromium.org/2062683002/diff/1/sdk/lib/isolate/isolate.dar...
sdk/lib/isolate/isolate.dart:305: * When the isolate receives the pause command,
it will stop
Stops

https://codereview.chromium.org/2062683002/diff/1/sdk/lib/isolate/isolate.dar...
sdk/lib/isolate/isolate.dart:308: * or receive port messages. When the isolate
is resumed, it will handle
Maybe "handles" but here it really is in the future.

[Lasse Reichstein Nielsen, 2016-07-04 11:47:33]

https://codereview.chromium.org/2062683002/diff/1/sdk/lib/isolate/isolate.dart
File sdk/lib/isolate/isolate.dart (right):

https://codereview.chromium.org/2062683002/diff/1/sdk/lib/isolate/isolate.dar...
sdk/lib/isolate/isolate.dart:171: * that can be called with a single argument -
On 2016/06/14 11:36:47, eernst wrote:
> Might as well use ',' because the single hyphen is so different from the
proper
> em-dash, and a comma would not be unnatural here.

Done.

https://codereview.chromium.org/2062683002/diff/1/sdk/lib/isolate/isolate.dar...
sdk/lib/isolate/isolate.dart:176: * as long as they *can* be called with just a
single argument.
On 2016/06/14 11:36:47, eernst wrote:
> There's a singular/plural issue here, and I also think that the emphasis is
> slightly too informal for long time use (this whole sentence is just there for
> emphasis), so I'd suggest: 'as long as it can'.

Done.

https://codereview.chromium.org/2062683002/diff/1/sdk/lib/isolate/isolate.dar...
sdk/lib/isolate/isolate.dart:178: * allowed.
You can't pass an expression, only its value.
Reworded to: 
The function must not be the value of a function expression or an instance
method tear-off.

(We don't have a convenient shorthand for those two, but it would probably be
"closure" if we did)

https://codereview.chromium.org/2062683002/diff/1/sdk/lib/isolate/isolate.dar...
sdk/lib/isolate/isolate.dart:305: * When the isolate receives the pause command,
it will stop
On 2016/06/16 08:36:34, floitsch wrote:
> Stops

Done.

https://codereview.chromium.org/2062683002/diff/1/sdk/lib/isolate/isolate.dar...
sdk/lib/isolate/isolate.dart:308: * or receive port messages. When the isolate
is resumed, it will handle
Used "handles" and improved the second half of the sentence.

https://codereview.chromium.org/2062683002/diff/1/sdk/lib/isolate/isolate.dar...
sdk/lib/isolate/isolate.dart:313: * but before already enqueued event loop
events that hasn't been processed
On 2016/06/14 11:36:47, eernst wrote:
> 'after any previously sent command messages, but before any enqueued event
loop
> events.'
> 
> If event loop events can indeed be enqueued even though they have already been
> processed 

Event loop events are enqueued until they are processed, and that removes them
from the queue. 
So, no, they can't. 

> then we can add 'that have not yet been processed'. Otherwise we might
> as well put a period here, and then, possibly, add a separate sentence
> explaining that processed events cannot exist in the queue.

I think a larger rewrite is in order.
The "pause" command (like any command) takes effect before the next event loop
event is processed, but after the current one is done.
(I don't think you pause the microtask queue).

[floitsch, 2016-07-04 14:38:28]

LGTM.

https://codereview.chromium.org/2062683002/diff/20001/sdk/lib/isolate/isolate...
File sdk/lib/isolate/isolate.dart (right):

https://codereview.chromium.org/2062683002/diff/20001/sdk/lib/isolate/isolate...
sdk/lib/isolate/isolate.dart:308: * or receive port messages. When the isolate
is resumed, it handles
receive-port

https://codereview.chromium.org/2062683002/diff/20001/sdk/lib/isolate/isolate...
sdk/lib/isolate/isolate.dart:309: * the already enqueued events in their
existing order.
I would drop the 'in their existing order'.
It reads like "in order", but that's not true.

If I'm not wrong, the order is actually different than if the system hadn't been
paused, because high-priority events are executed earlier.

https://codereview.chromium.org/2062683002/diff/20001/sdk/lib/isolate/isolate...
sdk/lib/isolate/isolate.dart:324: * each pause must be individully ended before
the isolate resumes.
individually

[Lasse Reichstein Nielsen, 2016-09-28 08:42:03]

https://codereview.chromium.org/2062683002/diff/20001/sdk/lib/isolate/isolate...
File sdk/lib/isolate/isolate.dart (right):

https://codereview.chromium.org/2062683002/diff/20001/sdk/lib/isolate/isolate...
sdk/lib/isolate/isolate.dart:308: * or receive port messages. When the isolate
is resumed, it handles
On 2016/07/04 14:38:27, floitsch wrote:
> receive-port

Done.

https://codereview.chromium.org/2062683002/diff/20001/sdk/lib/isolate/isolate...
sdk/lib/isolate/isolate.dart:309: * the already enqueued events in their
existing order.
Dropped.
I do think the events are executed in the order they were enqueued, which is the
order they would have been executed in if the isolate hadn't been paused. Unless
timers are handled differently.
I'm not sure what high-priority events are.

https://codereview.chromium.org/2062683002/diff/20001/sdk/lib/isolate/isolate...
sdk/lib/isolate/isolate.dart:324: * each pause must be individully ended before
the isolate resumes.
On 2016/07/04 14:38:27, floitsch wrote:
> individually

Done.

[Lasse Reichstein Nielsen, 2016-09-28 08:42:10]

Description was changed from

==========
Address shortcomings in documentation on Isolate class.

Addresses issued #26597 and #26627.

BUG= http://dartbug.com/26597 http://dartbug.com/26627
==========

to

==========
Address shortcomings in documentation on Isolate class.

Addresses issued #26597 and #26627.

BUG= http://dartbug.com/26597 http://dartbug.com/26627
R=eernst@google.com, floitsch@google.com

Committed:
https://github.com/dart-lang/sdk/commit/9487e107bb53a18eb05c663202dffd935c24d29a
==========

[Lasse Reichstein Nielsen, 2016-09-28 08:42:11]

Committed patchset #3 (id:40001) manually as
9487e107bb53a18eb05c663202dffd935c24d29a (presubmit successful).