- Do not keep the environment alive past the process spawn.

- Do not keep the environment alive past the process spawn.
- Avoid creation of many temp objects while creating the process
  environment.
- "if (identical(true, value))" makes no sense.

R=asiva@google.com

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

[siva, 2014-04-10 00:11:04]

lgtm

[Ivan Posva, 2014-04-10 00:15:49]

Committed patchset #1 manually as r34907 (presubmit successful).

[Anders Johnsen, 2014-04-10 07:06:44]

https://codereview.chromium.org/232223004/diff/1/runtime/bin/process_patch.dart
File runtime/bin/process_patch.dart (right):

https://codereview.chromium.org/232223004/diff/1/runtime/bin/process_patch.da...
runtime/bin/process_patch.dart:195: var environmentEntryHandler = (key, value) {
void environmentEntryHandler(key, value) {
 ...
}

?

https://codereview.chromium.org/232223004/diff/1/runtime/bin/process_patch.da...
runtime/bin/process_patch.dart:198: "Environment key or value is not a string:
($key, $value)");
indentation

https://codereview.chromium.org/232223004/diff/1/runtime/bin/process_patch.da...
runtime/bin/process_patch.dart:209:
Platform.environment.forEach(environmentEntryHandler);
This is wrong. Now parent environment will override child.

[Ivan Posva, 2014-04-10 07:27:38]

Thanks,
-Ivan

https://codereview.chromium.org/232223004/diff/1/runtime/bin/process_patch.dart
File runtime/bin/process_patch.dart (right):

https://codereview.chromium.org/232223004/diff/1/runtime/bin/process_patch.da...
runtime/bin/process_patch.dart:195: var environmentEntryHandler = (key, value) {
On 2014/04/10 07:06:44, Anders Johnsen wrote:
> void environmentEntryHandler(key, value) {
>  ...
> }
> 
> ?

They are equivalent.

https://codereview.chromium.org/232223004/diff/1/runtime/bin/process_patch.da...
runtime/bin/process_patch.dart:209:
Platform.environment.forEach(environmentEntryHandler);
On 2014/04/10 07:06:44, Anders Johnsen wrote:
> This is wrong. Now parent environment will override child.

I see it now. It was not obvious in the original code. I'll fix this tomorrow.

[Anders Johnsen, 2014-04-10 08:08:12]

https://codereview.chromium.org/232223004/diff/1/runtime/bin/process_patch.dart
File runtime/bin/process_patch.dart (right):

https://codereview.chromium.org/232223004/diff/1/runtime/bin/process_patch.da...
runtime/bin/process_patch.dart:195: var environmentEntryHandler = (key, value) {
On 2014/04/10 07:27:38, Ivan Posva wrote:
> On 2014/04/10 07:06:44, Anders Johnsen wrote:
> > void environmentEntryHandler(key, value) {
> >  ...
> > }
> > 
> > ?
> 
> They are equivalent.

Yes, but very odd style (not what we usually use in dart:io).

https://codereview.chromium.org/232223004/diff/1/runtime/bin/process_patch.da...
runtime/bin/process_patch.dart:209:
Platform.environment.forEach(environmentEntryHandler);
On 2014/04/10 07:27:38, Ivan Posva wrote:
> On 2014/04/10 07:06:44, Anders Johnsen wrote:
> > This is wrong. Now parent environment will override child.
> 
> I see it now. It was not obvious in the original code. I'll fix this tomorrow.

I'll start cooking on a test.