Parsing: Make Parser not know about Isolate during background parsing.

Parsing: Make Parser not know about Isolate during background parsing.

Parser must be able to operate independent of Isolate and the V8 heap during
parsing. After the heap-independent phase, there is a heap dependent phase,
during which we internalize strings, handle errors, etc.

This makes Isolate (also via CompilationInfo) unaccessible during parsing, and
thus decreases the probability of accidental code changes which would add
heap-dependent operations into the heap-independent phase.

Since Isolate is also accessible via CompilationInfo, now CompilationInfo is
only passed to the entry points of parsing, and not stored in Parser.

R=rossberg@chromium.org
BUG=

Committed: https://crrev.com/df0cb9999ff0574c6476b29f0610c7ca69d3ae1f
Cr-Commit-Position: refs/heads/master@{#26612}

[marja, 2015-02-10 16:10:38]

rossberg, ptal. The second part of "make isolate unaccessible" changes.
Should've done this ages ago.

[marja, 2015-02-10 16:24:39]

whoops, something's wrong, tests are failing.. I'll ping this CL again when I've
fixed them.

[marja, 2015-02-10 17:26:59]

lulz, function naming is hard, function resolution is hard too.

I'm not happy w/ the names, now that we cannot have static Parser::Parse and
non-static Parser::Parse any more, how should we call them? This is my best
guess but...

[marja, 2015-02-10 17:27:14]

(and that is to say, the latest patch set fixes the problems, pls review)

[titzer, 2015-02-12 10:06:33]

titzer@chromium.org changed reviewers:
+ titzer@chromium.org

[titzer, 2015-02-12 10:06:34]

I like getting rid of the isolate in the parser.

As discussed in person, there will be some conflict with splitting up
CompilationInfo into a separate ParserInfo. The idea there being that the
ParseInfo data structure contains both the inputs to parsing and the output of
parsing (potentially include errors), and nothing more. Then some of the
instance fields of Parser and Preparser can be factored out into that ParseInfo.
WDYT?

https://codereview.chromium.org/908173003/diff/40001/src/parser.h
File src/parser.h (right):

https://codereview.chromium.org/908173003/diff/40001/src/parser.h#newcode670
src/parser.h:670: bool Parse(CompilationInfo* info);
What about renaming this ParseSynchronous() and then keeping the name for the
other?

[marja, 2015-02-12 10:13:26]

... concluded that I'll rebase this stuff on top of
https://codereview.chromium.org/913183005/ after it has landed, and then land
this (assuming rossberg@ thinks this is lg, too), and then Ben will do the next
step of the CompilationInfo split.

[rossberg, 2015-02-12 10:31:30]

LGTM

https://codereview.chromium.org/908173003/diff/1/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/908173003/diff/1/src/parser.cc#newcode917
src/parser.cc:917: // created. This kind of parsing can only be done in the main
thread.
Is there a way to DCHECK that here?

[marja, 2015-02-12 12:31:01]

New patchsets have been uploaded after l-g-t-m from rossberg@chromium.org

[marja, 2015-02-12 12:37:31]

thx for review.

The naming is not that great still, but idk... ParseSynchronous doesn't transmit
the meaning either, since ParseOnBackground is also synchronous (it doesn't
launch a background task or anything, it's just a func meant to be called on a
background thread).

https://codereview.chromium.org/908173003/diff/1/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/908173003/diff/1/src/parser.cc#newcode917
src/parser.cc:917: // created. This kind of parsing can only be done in the main
thread.
On 2015/02/12 10:31:29, rossberg wrote:
> Is there a way to DCHECK that here?

I added a bool parsing_on_main_thread_ which ParseOnBackground sets to false and
which can then be checked.

[marja, 2015-02-12 12:37:37]

The CQ bit was checked by marja@chromium.org

[commit-bot: I haz the power, 2015-02-12 12:37:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/908173003/80001

[titzer, 2015-02-12 12:42:42]

On 2015/02/12 12:37:31, marja wrote:
> thx for review.
> 
> The naming is not that great still, but idk... ParseSynchronous doesn't
transmit
> the meaning either, since ParseOnBackground is also synchronous (it doesn't
> launch a background task or anything, it's just a func meant to be called on a
> background thread).

I was just trying to avoid the renaming spreading to too many files, since
Parse() is called in several places, for good or for worse.

> 
> https://codereview.chromium.org/908173003/diff/1/src/parser.cc
> File src/parser.cc (right):
> 
> https://codereview.chromium.org/908173003/diff/1/src/parser.cc#newcode917
> src/parser.cc:917: // created. This kind of parsing can only be done in the
main
> thread.
> On 2015/02/12 10:31:29, rossberg wrote:
> > Is there a way to DCHECK that here?
> 
> I added a bool parsing_on_main_thread_ which ParseOnBackground sets to false
and
> which can then be checked.

[commit-bot: I haz the power, 2015-02-12 13:02:36]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-02-12 13:02:57]

Patchset 5 (id:??) landed as
https://crrev.com/df0cb9999ff0574c6476b29f0610c7ca69d3ae1f
Cr-Commit-Position: refs/heads/master@{#26612}