Reland: [Parse] ParseInfo owns the parsing Zone.

Reland: [Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203, v8:5215
Review-Url: https://codereview.chromium.org/2632123006
Cr-Original-Commit-Position: refs/heads/master@{#42993}
Committed: https://chromium.googlesource.com/v8/v8/+/14fb337200d5da09c77438ddd40bea935b1dc823
Review-Url: https://codereview.chromium.org/2632123006
Cr-Commit-Position: refs/heads/master@{#42996}
Committed: https://chromium.googlesource.com/v8/v8/+/9e7d5a6065470ca03411d4c8dbc61d1be5c3f84a

[rmcilroy, 2017-01-17 15:18:17]

rmcilroy@chromium.org changed reviewers:
+ ahaas@chromium.org, marja@chromium.org, mstarzinger@chromium.org,
verwaest@chromium.org

[rmcilroy, 2017-01-17 15:18:17]

This is the first step in enabling eager inner function CompilerDispatcherJobs
to keep the parse zone alive after the outer function compilation is complete. 

marja@: Please review changes in parser/
mstarzinger@: Please review changes in compiler/ and compiler.cc
ahaas@c: Please review changes in asmjs/wasm
verwaest@: Please review changes in crankshaft/

[marja, 2017-01-17 15:39:08]

I was worried about

1) coupling the Zone lifetime with ParseInfo's lifetime, but it looks like
they're (almost) always (where relevant) created together anyway. (Or ParseInfo
used an existing Zone but it can as well use a new Zone I guess.)

2) (and I still am worried about this): there's no way to assure that no place
calls ParseInfo::zone() and saves it (or passes it somewhere which saves it) and
it's very hard to verify that this CL is correct. Plus it's fragile, somebody
could easily introduce a bug like that in the future. Renaming zone() to
zone_unsafe() or sth would make it less probable (and also force this CL touch
all call cites) but it's probably not feasible :(

What's your take on 2)?

https://codereview.chromium.org/2632123006/diff/1/src/asmjs/asm-wasm-builder.cc
File src/asmjs/asm-wasm-builder.cc (right):

https://codereview.chromium.org/2632123006/diff/1/src/asmjs/asm-wasm-builder....
src/asmjs/asm-wasm-builder.cc:146: std::unique_ptr<ParseInfo> info;
Hmm, now the Zone cannot outlive ParseInfo? That sounds worrying... but I'll
read on!

https://codereview.chromium.org/2632123006/diff/1/src/parsing/parse-info.h
File src/parsing/parse-info.h (right):

https://codereview.chromium.org/2632123006/diff/1/src/parsing/parse-info.h#ne...
src/parsing/parse-info.h:44: Zone* zone() const { return zone_.get(); }
Based on this CL, it's not possible to see whether the changes are correct or
not. I'd need to go and check every zone() caller to see whether any of them is
saving this pointer and using it after ParseInfo is gone. :/

https://codereview.chromium.org/2632123006/diff/1/test/cctest/asmjs/test-asm-...
File test/cctest/asmjs/test-asm-typer.cc (right):

https://codereview.chromium.org/2632123006/diff/1/test/cctest/asmjs/test-asm-...
test/cctest/asmjs/test-asm-typer.cc:56: ParseInfo info(script_);
This is a bit weird. We'll create a Zone here, and ParseInfo will create another
one. And we give ParseInfo an AstValueFactory which is from a different Zone...

But that should work anyway, right?

[marja, 2017-01-17 15:44:54]

Hmm, looking at the call sites of ParseInfo::zone(), it's not very worrying;
looks like all are very temporal.

And we already have the similarly "unsafe" structure:

Zone zone;
ParseInfo p_info(&zone);
CompilationInfo c_info(&zone); // The Zone from here shouldn't escape.

lgtm

[ahaas, 2017-01-17 15:51:00]

On 2017/01/17 at 15:44:54, marja wrote:
> Hmm, looking at the call sites of ParseInfo::zone(), it's not very worrying;
looks like all are very temporal.
> 
> And we already have the similarly "unsafe" structure:
> 
> Zone zone;
> ParseInfo p_info(&zone);
> CompilationInfo c_info(&zone); // The Zone from here shouldn't escape.
> 
> lgtm

asmjs lgtm

[Toon Verwaest, 2017-01-18 09:00:51]

Actually seems reasonable to me. LGTM

[Michael Starzinger, 2017-01-18 09:12:20]

LGTM on the compiler plumbing. My biggest worry is point (1) that Marja raised
in the first comment. Currently it seems that coupling lifetime of the parser
zone to {ParseInfo} is what we want. But it removes the freedom to change our
minds about that. If we don't plan to embed the parsing result into an existing
zone in the near future, then I am fine with this change.

[rmcilroy, 2017-01-19 12:38:52]

The CQ bit was checked by rmcilroy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 12:38:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-19 12:44:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 12:44:46]

Dry run: Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)

[rmcilroy, 2017-01-19 12:48:29]

Thanks all.

re 2): see comment on parse-info.h.

https://codereview.chromium.org/2632123006/diff/1/src/asmjs/asm-wasm-builder.cc
File src/asmjs/asm-wasm-builder.cc (right):

https://codereview.chromium.org/2632123006/diff/1/src/asmjs/asm-wasm-builder....
src/asmjs/asm-wasm-builder.cc:146: std::unique_ptr<ParseInfo> info;
On 2017/01/17 15:39:08, marja wrote:
> Hmm, now the Zone cannot outlive ParseInfo? That sounds worrying... but I'll
> read on!

Acknowledged.

https://codereview.chromium.org/2632123006/diff/1/src/parsing/parse-info.h
File src/parsing/parse-info.h (right):

https://codereview.chromium.org/2632123006/diff/1/src/parsing/parse-info.h#ne...
src/parsing/parse-info.h:44: Zone* zone() const { return zone_.get(); }
On 2017/01/17 15:39:08, marja wrote:
> Based on this CL, it's not possible to see whether the changes are correct or
> not. I'd need to go and check every zone() caller to see whether any of them
is
> saving this pointer and using it after ParseInfo is gone. :/

Given that the existing zone which was created always had the same lifetime as
the parseinfo I think this is fine, as you say all the uses of this are
temporary (and they should be since AFAIK the expectation in the style guide is
that you never stash away raw pointers you receive, only std::unique_ptr or
std::shared_ptr where you are explicilty transferring ownership).

https://codereview.chromium.org/2632123006/diff/1/test/cctest/asmjs/test-asm-...
File test/cctest/asmjs/test-asm-typer.cc (right):

https://codereview.chromium.org/2632123006/diff/1/test/cctest/asmjs/test-asm-...
test/cctest/asmjs/test-asm-typer.cc:56: ParseInfo info(script_);
On 2017/01/17 15:39:08, marja wrote:
> This is a bit weird. We'll create a Zone here, and ParseInfo will create
another
> one. And we give ParseInfo an AstValueFactory which is from a different
Zone...
> 
> But that should work anyway, right?

You are right and this was actually an issue. I reworked this test code so that
we use the zone from ParseInfo to create the ast-value-factory.

[rmcilroy, 2017-01-19 12:48:39]

The CQ bit was checked by rmcilroy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 12:48:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[marja, 2017-01-19 12:54:48]

still lgtm

[commit-bot: I haz the power, 2017-01-19 13:19:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 13:19:03]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2017-01-19 16:31:06]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2017-01-19 16:31:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, ahaas@chromium.org, verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/2632123006/#ps40001
(title: "Remove unused variable")

[commit-bot: I haz the power, 2017-01-19 16:31:19]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-19 17:14:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 17:14:51]

Failed to apply patch for src/compiler.cc:
While running git apply --index -p1;
  error: patch failed: src/compiler.cc:1575
  error: src/compiler.cc: patch does not apply

Patch:       src/compiler.cc
Index: src/compiler.cc
diff --git a/src/compiler.cc b/src/compiler.cc
index
6f123ea2b3e6ad5db98cd7ac1d048fed70ab0e6a..1972fd10b6e64463cf5ddb872bbad5b451efa231
100644
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -507,8 +507,7 @@ bool CompileUnoptimizedInnerFunctionsRecursively(
       shared->set_is_toplevel(false);
     }
 
-    Zone zone(isolate->allocator(), ZONE_NAME);
-    ParseInfo parse_info(&zone, script);
+    ParseInfo parse_info(script);
     parse_info.set_literal(literal);
     parse_info.set_shared_info(shared);
     parse_info.set_function_literal_id(shared->function_literal_id());
@@ -892,8 +891,7 @@ MaybeHandle<Code> GetBaselineCode(Handle<JSFunction>
function) {
   Isolate* isolate = function->GetIsolate();
   VMState<COMPILER> state(isolate);
   PostponeInterruptsScope postpone(isolate);
-  Zone zone(isolate->allocator(), ZONE_NAME);
-  ParseInfo parse_info(&zone, handle(function->shared()));
+  ParseInfo parse_info(handle(function->shared()));
   CompilationInfo info(&parse_info, function);
 
   DCHECK(function->shared()->is_compiled());
@@ -1026,8 +1024,7 @@ MaybeHandle<Code> GetLazyCode(Handle<JSFunction> function)
{
     return entry;
   }
 
-  Zone zone(isolate->allocator(), ZONE_NAME);
-  ParseInfo parse_info(&zone, handle(function->shared()));
+  ParseInfo parse_info(handle(function->shared()));
   CompilationInfo info(&parse_info, function);
   Handle<Code> result;
   ASSIGN_RETURN_ON_EXCEPTION(isolate, result, GetUnoptimizedCode(&info), Code);
@@ -1206,8 +1203,7 @@ bool Compiler::CompileOptimized(Handle<JSFunction>
function,
       code = isolate->builtins()->InterpreterEntryTrampoline();
       function->shared()->ReplaceCode(*code);
     } else {
-      Zone zone(isolate->allocator(), ZONE_NAME);
-      ParseInfo parse_info(&zone, handle(function->shared()));
+      ParseInfo parse_info(handle(function->shared()));
       CompilationInfo info(&parse_info, function);
       if (!GetUnoptimizedCode(&info).ToHandle(&code)) {
         return false;
@@ -1231,8 +1227,7 @@ bool Compiler::CompileDebugCode(Handle<SharedFunctionInfo>
shared) {
   DCHECK(AllowCompilation::IsAllowed(isolate));
 
   // Start a compilation.
-  Zone zone(isolate->allocator(), ZONE_NAME);
-  ParseInfo parse_info(&zone, shared);
+  ParseInfo parse_info(shared);
   CompilationInfo info(&parse_info, Handle<JSFunction>::null());
   info.MarkAsDebug();
   if (GetUnoptimizedCode(&info).is_null()) {
@@ -1259,8 +1254,7 @@ MaybeHandle<JSArray>
Compiler::CompileForLiveEdit(Handle<Script> script) {
   script->set_shared_function_infos(isolate->heap()->empty_fixed_array());
 
   // Start a compilation.
-  Zone zone(isolate->allocator(), ZONE_NAME);
-  ParseInfo parse_info(&zone, script);
+  ParseInfo parse_info(script);
   CompilationInfo info(&parse_info, Handle<JSFunction>::null());
   info.MarkAsDebug();
 
@@ -1271,7 +1265,7 @@ MaybeHandle<JSArray>
Compiler::CompileForLiveEdit(Handle<Script> script) {
     // Check postconditions on success.
     DCHECK(!isolate->has_pending_exception());
     infos = LiveEditFunctionTracker::Collect(parse_info.literal(), script,
-                                             &zone, isolate);
+                                             parse_info.zone(), isolate);
   }
 
   // Restore the original function info list in order to remain side-effect
@@ -1395,8 +1389,7 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
     script->set_compilation_type(Script::COMPILATION_TYPE_EVAL);
     Script::SetEvalOrigin(script, outer_info, eval_position);
 
-    Zone zone(isolate->allocator(), ZONE_NAME);
-    ParseInfo parse_info(&zone, script);
+    ParseInfo parse_info(script);
     CompilationInfo info(&parse_info, Handle<JSFunction>::null());
     parse_info.set_eval();
     parse_info.set_language_mode(language_mode);
@@ -1575,8 +1568,7 @@ Handle<SharedFunctionInfo>
Compiler::GetSharedFunctionInfoForScript(
     }
 
     // Compile the function and add it to the cache.
-    Zone zone(isolate->allocator(), ZONE_NAME);
-    ParseInfo parse_info(&zone, script);
+    ParseInfo parse_info(script);
     CompilationInfo info(&parse_info, Handle<JSFunction>::null());
     if (is_module) parse_info.set_module();
     if (compile_options != ScriptCompiler::kNoCompileOptions) {

[rmcilroy, 2017-01-19 18:15:17]

The CQ bit was checked by rmcilroy@chromium.org

[commit-bot: I haz the power, 2017-01-19 18:15:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-19 18:17:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 18:17:49]

Failed to apply patch for src/compiler.cc:
While running git apply --index -p1;
  error: patch failed: src/compiler.cc:1575
  error: src/compiler.cc: patch does not apply

Patch:       src/compiler.cc
Index: src/compiler.cc
diff --git a/src/compiler.cc b/src/compiler.cc
index
6f123ea2b3e6ad5db98cd7ac1d048fed70ab0e6a..1972fd10b6e64463cf5ddb872bbad5b451efa231
100644
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -507,8 +507,7 @@ bool CompileUnoptimizedInnerFunctionsRecursively(
       shared->set_is_toplevel(false);
     }
 
-    Zone zone(isolate->allocator(), ZONE_NAME);
-    ParseInfo parse_info(&zone, script);
+    ParseInfo parse_info(script);
     parse_info.set_literal(literal);
     parse_info.set_shared_info(shared);
     parse_info.set_function_literal_id(shared->function_literal_id());
@@ -892,8 +891,7 @@ MaybeHandle<Code> GetBaselineCode(Handle<JSFunction>
function) {
   Isolate* isolate = function->GetIsolate();
   VMState<COMPILER> state(isolate);
   PostponeInterruptsScope postpone(isolate);
-  Zone zone(isolate->allocator(), ZONE_NAME);
-  ParseInfo parse_info(&zone, handle(function->shared()));
+  ParseInfo parse_info(handle(function->shared()));
   CompilationInfo info(&parse_info, function);
 
   DCHECK(function->shared()->is_compiled());
@@ -1026,8 +1024,7 @@ MaybeHandle<Code> GetLazyCode(Handle<JSFunction> function)
{
     return entry;
   }
 
-  Zone zone(isolate->allocator(), ZONE_NAME);
-  ParseInfo parse_info(&zone, handle(function->shared()));
+  ParseInfo parse_info(handle(function->shared()));
   CompilationInfo info(&parse_info, function);
   Handle<Code> result;
   ASSIGN_RETURN_ON_EXCEPTION(isolate, result, GetUnoptimizedCode(&info), Code);
@@ -1206,8 +1203,7 @@ bool Compiler::CompileOptimized(Handle<JSFunction>
function,
       code = isolate->builtins()->InterpreterEntryTrampoline();
       function->shared()->ReplaceCode(*code);
     } else {
-      Zone zone(isolate->allocator(), ZONE_NAME);
-      ParseInfo parse_info(&zone, handle(function->shared()));
+      ParseInfo parse_info(handle(function->shared()));
       CompilationInfo info(&parse_info, function);
       if (!GetUnoptimizedCode(&info).ToHandle(&code)) {
         return false;
@@ -1231,8 +1227,7 @@ bool Compiler::CompileDebugCode(Handle<SharedFunctionInfo>
shared) {
   DCHECK(AllowCompilation::IsAllowed(isolate));
 
   // Start a compilation.
-  Zone zone(isolate->allocator(), ZONE_NAME);
-  ParseInfo parse_info(&zone, shared);
+  ParseInfo parse_info(shared);
   CompilationInfo info(&parse_info, Handle<JSFunction>::null());
   info.MarkAsDebug();
   if (GetUnoptimizedCode(&info).is_null()) {
@@ -1259,8 +1254,7 @@ MaybeHandle<JSArray>
Compiler::CompileForLiveEdit(Handle<Script> script) {
   script->set_shared_function_infos(isolate->heap()->empty_fixed_array());
 
   // Start a compilation.
-  Zone zone(isolate->allocator(), ZONE_NAME);
-  ParseInfo parse_info(&zone, script);
+  ParseInfo parse_info(script);
   CompilationInfo info(&parse_info, Handle<JSFunction>::null());
   info.MarkAsDebug();
 
@@ -1271,7 +1265,7 @@ MaybeHandle<JSArray>
Compiler::CompileForLiveEdit(Handle<Script> script) {
     // Check postconditions on success.
     DCHECK(!isolate->has_pending_exception());
     infos = LiveEditFunctionTracker::Collect(parse_info.literal(), script,
-                                             &zone, isolate);
+                                             parse_info.zone(), isolate);
   }
 
   // Restore the original function info list in order to remain side-effect
@@ -1395,8 +1389,7 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
     script->set_compilation_type(Script::COMPILATION_TYPE_EVAL);
     Script::SetEvalOrigin(script, outer_info, eval_position);
 
-    Zone zone(isolate->allocator(), ZONE_NAME);
-    ParseInfo parse_info(&zone, script);
+    ParseInfo parse_info(script);
     CompilationInfo info(&parse_info, Handle<JSFunction>::null());
     parse_info.set_eval();
     parse_info.set_language_mode(language_mode);
@@ -1575,8 +1568,7 @@ Handle<SharedFunctionInfo>
Compiler::GetSharedFunctionInfoForScript(
     }
 
     // Compile the function and add it to the cache.
-    Zone zone(isolate->allocator(), ZONE_NAME);
-    ParseInfo parse_info(&zone, script);
+    ParseInfo parse_info(script);
     CompilationInfo info(&parse_info, Handle<JSFunction>::null());
     if (is_module) parse_info.set_module();
     if (compile_options != ScriptCompiler::kNoCompileOptions) {

[rmcilroy, 2017-01-19 23:06:14]

The CQ bit was checked by rmcilroy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 23:06:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-19 23:42:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 23:42:28]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2017-01-20 09:25:10]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2017-01-20 09:25:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, ahaas@chromium.org, verwaest@chromium.org,
marja@chromium.org
Link to the patchset: https://codereview.chromium.org/2632123006/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2017-01-20 09:25:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-20 09:26:58]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484904310972110,
"parent_rev": "a0230f635949711e92c48d95ea4f9847dc6c62f6", "commit_rev":
"839b06b64fcaaaa9cceb2c3fd8fd5429e2932095"}

[commit-bot: I haz the power, 2017-01-20 09:27:02]

Description was changed from

==========
[Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215
==========

to

==========
[Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215

Review-Url: https://codereview.chromium.org/2632123006
Cr-Commit-Position: refs/heads/master@{#42539}
Committed:
https://chromium.googlesource.com/v8/v8/+/839b06b64fcaaaa9cceb2c3fd8fd5429e29...
==========

[commit-bot: I haz the power, 2017-01-20 09:27:03]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/v8/v8/+/839b06b64fcaaaa9cceb2c3fd8fd5429e29...

[rmcilroy, 2017-01-20 10:38:36]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/2645613008/ by rmcilroy@chromium.org.

The reason for reverting is: Crashes on Windows in:
 CompilerDispatcherJobTest.CompileFailureToFinalize
 CompilerDispatcherJobTest.ScopeChain.

[rmcilroy, 2017-01-20 10:47:58]

Description was changed from

==========
[Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215

Review-Url: https://codereview.chromium.org/2632123006
Cr-Commit-Position: refs/heads/master@{#42539}
Committed:
https://chromium.googlesource.com/v8/v8/+/839b06b64fcaaaa9cceb2c3fd8fd5429e29...
==========

to

==========
[Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215

Review-Url: https://codereview.chromium.org/2632123006
Cr-Commit-Position: refs/heads/master@{#42539}
Committed:
https://chromium.googlesource.com/v8/v8/+/839b06b64fcaaaa9cceb2c3fd8fd5429e29...
==========

[rmcilroy, 2017-01-20 12:30:28]

Patchset #5 (id:80001) has been deleted

[rmcilroy, 2017-01-20 12:31:06]

The issue was destructor order in CompilerJobDispatcher::ResetOnMainThread.
Should be fixed now - relanding.

[rmcilroy, 2017-01-20 12:31:44]

The CQ bit was checked by rmcilroy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-20 12:31:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-20 13:08:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-20 13:08:37]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2017-01-20 14:13:41]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2017-01-20 14:13:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, ahaas@chromium.org, verwaest@chromium.org,
marja@chromium.org
Link to the patchset: https://codereview.chromium.org/2632123006/#ps100001
(title: "Fix destructor order")

[commit-bot: I haz the power, 2017-01-20 14:13:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-20 14:15:29]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1484921621380870,
"parent_rev": "ee9c709190b1a5a8c32fbcffb33f8ebe5cb20673", "commit_rev":
"4b0101d369af121a6daf5e5c731cb939c026d526"}

[commit-bot: I haz the power, 2017-01-20 14:15:33]

Description was changed from

==========
[Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215

Review-Url: https://codereview.chromium.org/2632123006
Cr-Commit-Position: refs/heads/master@{#42539}
Committed:
https://chromium.googlesource.com/v8/v8/+/839b06b64fcaaaa9cceb2c3fd8fd5429e29...
==========

to

==========
[Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215

Review-Url: https://codereview.chromium.org/2632123006
Cr-Original-Commit-Position: refs/heads/master@{#42539}
Committed:
https://chromium.googlesource.com/v8/v8/+/839b06b64fcaaaa9cceb2c3fd8fd5429e29...
Review-Url: https://codereview.chromium.org/2632123006
Cr-Commit-Position: refs/heads/master@{#42562}
Committed:
https://chromium.googlesource.com/v8/v8/+/4b0101d369af121a6daf5e5c731cb939c02...
==========

[commit-bot: I haz the power, 2017-01-20 14:15:34]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/v8/v8/+/4b0101d369af121a6daf5e5c731cb939c02...

[rmcilroy, 2017-02-07 11:29:01]

Description was changed from

==========
[Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215

Review-Url: https://codereview.chromium.org/2632123006
Cr-Original-Commit-Position: refs/heads/master@{#42539}
Committed:
https://chromium.googlesource.com/v8/v8/+/839b06b64fcaaaa9cceb2c3fd8fd5429e29...
Review-Url: https://codereview.chromium.org/2632123006
Cr-Commit-Position: refs/heads/master@{#42562}
Committed:
https://chromium.googlesource.com/v8/v8/+/4b0101d369af121a6daf5e5c731cb939c02...
==========

to

==========
[Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215

Review-Url: https://codereview.chromium.org/2632123006
Cr-Original-Commit-Position: refs/heads/master@{#42539}
Committed:
https://chromium.googlesource.com/v8/v8/+/839b06b64fcaaaa9cceb2c3fd8fd5429e29...
Review-Url: https://codereview.chromium.org/2632123006
Cr-Commit-Position: refs/heads/master@{#42562}
Committed:
https://chromium.googlesource.com/v8/v8/+/4b0101d369af121a6daf5e5c731cb939c02...
==========

[rmcilroy, 2017-02-07 11:35:29]

The CQ bit was checked by rmcilroy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-07 11:35:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-07 11:59:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-07 11:59:04]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2017-02-07 12:31:56]

Description was changed from

==========
[Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215

Review-Url: https://codereview.chromium.org/2632123006
Cr-Original-Commit-Position: refs/heads/master@{#42539}
Committed:
https://chromium.googlesource.com/v8/v8/+/839b06b64fcaaaa9cceb2c3fd8fd5429e29...
Review-Url: https://codereview.chromium.org/2632123006
Cr-Commit-Position: refs/heads/master@{#42562}
Committed:
https://chromium.googlesource.com/v8/v8/+/4b0101d369af121a6daf5e5c731cb939c02...
==========

to

==========
Reland: [Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215
==========

[rmcilroy, 2017-02-07 12:32:58]

Relanding this CL since it turned out not to be the cause of chromium:684481

[rmcilroy, 2017-02-07 12:33:05]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2017-02-07 12:33:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, ahaas@chromium.org, verwaest@chromium.org,
marja@chromium.org
Link to the patchset: https://codereview.chromium.org/2632123006/#ps120001
(title: "Rebase")

[commit-bot: I haz the power, 2017-02-07 12:33:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-07 12:34:57]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1486470785315130,
"parent_rev": "47f858d217eebf3b498458efe550a1b19ec773bd", "commit_rev":
"14fb337200d5da09c77438ddd40bea935b1dc823"}

[commit-bot: I haz the power, 2017-02-07 12:35:02]

Description was changed from

==========
Reland: [Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215
==========

to

==========
Reland: [Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215

Review-Url: https://codereview.chromium.org/2632123006
Cr-Commit-Position: refs/heads/master@{#42993}
Committed:
https://chromium.googlesource.com/v8/v8/+/14fb337200d5da09c77438ddd40bea935b1...
==========

[commit-bot: I haz the power, 2017-02-07 12:35:03]

Committed patchset #6 (id:120001) as
https://chromium.googlesource.com/v8/v8/+/14fb337200d5da09c77438ddd40bea935b1...

[jochen (gone - plz use gerrit), 2017-02-07 13:02:29]

A revert of this CL (patchset #6 id:120001) has been created in
https://codereview.chromium.org/2685543003/ by jochen@chromium.org.

The reason for reverting is: doesn't compile on ToT .

[Michael Achenbach, 2017-02-07 13:03:19]

A revert of this CL (patchset #6 id:120001) has been created in
https://codereview.chromium.org/2678393002/ by machenbach@chromium.org.

The reason for reverting is: Makes the tree red. Maybe outdated compile tryjobs?

E.g.:
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20builder/buil....

[rmcilroy, 2017-02-07 13:41:03]

On 2017/02/07 13:03:19, Michael Achenbach wrote:
> A revert of this CL (patchset #6 id:120001) has been created in
> https://codereview.chromium.org/2678393002/ by mailto:machenbach@chromium.org.
> 
> The reason for reverting is: Makes the tree red. Maybe outdated compile
tryjobs?
> 
> E.g.:
>
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20builder/buil....

Looks like "[parsing] Fix maybe-assigned for loop variables" landed in the tree
after the try-jobs passed, but before this CL landed. Trying again.

[rmcilroy, 2017-02-07 13:41:05]

On 2017/02/07 13:03:19, Michael Achenbach wrote:
> A revert of this CL (patchset #6 id:120001) has been created in
> https://codereview.chromium.org/2678393002/ by mailto:machenbach@chromium.org.
> 
> The reason for reverting is: Makes the tree red. Maybe outdated compile
tryjobs?
> 
> E.g.:
>
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20builder/buil....

Looks like "[parsing] Fix maybe-assigned for loop variables" landed in the tree
after the try-jobs passed, but before this CL landed. Trying again.

[rmcilroy, 2017-02-07 13:41:43]

Description was changed from

==========
Reland: [Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215

Review-Url: https://codereview.chromium.org/2632123006
Cr-Commit-Position: refs/heads/master@{#42993}
Committed:
https://chromium.googlesource.com/v8/v8/+/14fb337200d5da09c77438ddd40bea935b1...
==========

to

==========
Reland: [Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215

Review-Url: https://codereview.chromium.org/2632123006
Cr-Commit-Position: refs/heads/master@{#42993}
Committed:
https://chromium.googlesource.com/v8/v8/+/14fb337200d5da09c77438ddd40bea935b1...
==========

[rmcilroy, 2017-02-07 13:41:49]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2017-02-07 13:41:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org, ahaas@chromium.org, verwaest@chromium.org,
marja@chromium.org
Link to the patchset: https://codereview.chromium.org/2632123006/#ps140001
(title: "Rebase")

[commit-bot: I haz the power, 2017-02-07 13:42:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[marja, 2017-02-07 13:44:09]

Right, commented (confused) on the revert but commenting here too: so it's a
legit in-flight conflict.

Have you rebased on top of https://codereview.chromium.org/2683573002 ? If not,
this try will fail too :(

[commit-bot: I haz the power, 2017-02-07 14:04:45]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1486474909118050,
"parent_rev": "6159c5779d479b16d919b781f4d55abe504b023e", "commit_rev":
"9e7d5a6065470ca03411d4c8dbc61d1be5c3f84a"}

[commit-bot: I haz the power, 2017-02-07 14:04:50]

Description was changed from

==========
Reland: [Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215

Review-Url: https://codereview.chromium.org/2632123006
Cr-Commit-Position: refs/heads/master@{#42993}
Committed:
https://chromium.googlesource.com/v8/v8/+/14fb337200d5da09c77438ddd40bea935b1...
==========

to

==========
Reland: [Parse] ParseInfo owns the parsing Zone.

Moves ownership of the parsing Zone to ParseInfo with a shared_ptr. This is
in preperation for enabling background compilation jobs for inner functions
share the AST in the outer-function's parse zone memory (read-only), with the
and zone being released when all compilation jobs have completed.

BUG=v8:5203,v8:5215

Review-Url: https://codereview.chromium.org/2632123006
Cr-Original-Commit-Position: refs/heads/master@{#42993}
Committed:
https://chromium.googlesource.com/v8/v8/+/14fb337200d5da09c77438ddd40bea935b1...
Review-Url: https://codereview.chromium.org/2632123006
Cr-Commit-Position: refs/heads/master@{#42996}
Committed:
https://chromium.googlesource.com/v8/v8/+/9e7d5a6065470ca03411d4c8dbc61d1be5c...
==========

[commit-bot: I haz the power, 2017-02-07 14:04:51]

Committed patchset #7 (id:140001) as
https://chromium.googlesource.com/v8/v8/+/9e7d5a6065470ca03411d4c8dbc61d1be5c...

[Michael Hablich, 2017-02-07 19:35:32]

A revert of this CL (patchset #7 id:140001) has been created in
https://codereview.chromium.org/2683733002/ by hablich@chromium.org.

The reason for reverting is: Speculative revert because of revert needed for
https://codereview.chromium.org/2632123006.

[rmcilroy, 2017-02-07 20:15:57]

On 2017/02/07 19:35:32, Michael Hablich wrote:
> A revert of this CL (patchset #7 id:140001) has been created in
> https://codereview.chromium.org/2683733002/ by mailto:hablich@chromium.org.
> 
> The reason for reverting is: Speculative revert because of revert needed for
> https://codereview.chromium.org/2632123006.

Why was this revert needed? Description seems recursive (mentioning revert
needed for <this CL>).