WIP enum / V8PerContextData solution

WIP Convert embedded debug data to enums/int in V8PerContextData

Create enums characterizing the origin of JS source code (CompilationOriginCategory), the context where that source will be compiled (CompilationContextCategory), and restrictions on the result (CompilationRestrictions). Add enum values and the debugId integer in V8PerContextData.

Add functions in V8PerContextDebugData to set initial CompilationOriginCategory and debugId. Create V8ScopedCompilation to mark and automatically unmark a context for use by the browser, eg debugging or extension API injection. 

(WIP) Use the new enums to set debug data into the V8 embedders data.




BUG=226476

[johnjbarton, 2013-04-23 17:00:56]

This patch crashes with
STDERR:  [0x000002635f18] base::debug::StackTrace::StackTrace()
STDERR:  [0x00000263581f] base::debug::(anonymous
namespace)::StackDumpSignalHandler()
STDERR:  [0x7f9cc5a41cb0] <unknown>
STDERR:  [0x00000144cf48] v8::internal::Map::instance_type()
STDERR:  [0x0000015f41a2] v8::internal::ShortCircuitConsString()
STDERR:  [0x0000015ff56a]
v8::internal::RootMarkingVisitor::MarkObjectByPointer()
STDERR:  [0x0000015ff51d] v8::internal::RootMarkingVisitor::VisitPointers()
STDERR:  [0x000001531d22] v8::internal::Heap::IterateStrongRoots()
STDERR:  [0x0000015f52d4] v8::internal::MarkCompactCollector::MarkRoots()
STDERR:  [0x0000015f5d52] v8::internal::MarkCompactCollector::MarkLiveObjects()
STDERR:  [0x0000015f1c92] v8::internal::MarkCompactCollector::CollectGarbage()
STDERR:  [0x000001522a08] v8::internal::Heap::MarkCompact()
STDERR:  [0x00000152233d] v8::internal::Heap::PerformGarbageCollection()
STDERR:  [0x00000152194e] v8::internal::Heap::CollectGarbage()
STDERR:  [0x00000148ab8c] v8::internal::Heap::CollectGarbage()
STDERR:  [0x00000161f0b6] v8::internal::JSObject::NormalizeProperties()
STDERR:  [0x00000168e656] v8::internal::CreateObjectLiteralBoilerplate()
STDERR:  [0x00000168f91c] v8::internal::Runtime_CreateObjectLiteralShallow()

[johnjbarton, 2013-04-23 18:03:48]

Crash solved. Makes me wonder if using V8PerContextData is such a great idea
after all...

https://codereview.chromium.org/14362015/diff/1001/Source/bindings/v8/V8DOMWi...
File Source/bindings/v8/V8DOMWindowShell.cpp (right):

https://codereview.chromium.org/14362015/diff/1001/Source/bindings/v8/V8DOMWi...
Source/bindings/v8/V8DOMWindowShell.cpp:81:
V8PerContextDebugData::setDebugDataForContentScript(targetContext, debugId);
This will crash because the V8PerContextData object has not been initialized
when we are called by initializedIfNeeded()

[Jakob Kummerow, 2013-04-24 12:20:04]

I've noticed one more method that looks unsafe to me (see below).

Generally, I'm not an expert on how to use the V8 API (jochen@, marja@, and
dcarney@ are much more familiar with the pitfalls), and I have no idea how the
bindings/inspector system is supposed to work, so I'm not the right reviewer for
this CL.

https://codereview.chromium.org/14362015/diff/1001/Source/bindings/v8/custom/...
File Source/bindings/v8/custom/V8InjectedScriptHostCustom.cpp (right):

https://codereview.chromium.org/14362015/diff/1001/Source/bindings/v8/custom/...
Source/bindings/v8/custom/V8InjectedScriptHostCustom.cpp:324: return
script->Run();
This is another handle that's returned from a HandleScope that's about to get
zapped.

[pfeldman, 2013-04-24 12:28:33]

Is there anything other than preprocessor that would benefit of this change?
Sounds like it adds new aspects/complexities to the code.

[johnjbarton, 2013-04-24 15:00:28]

On 2013/04/24 12:28:33, pfeldman wrote:
> Is there anything other than preprocessor that would benefit of this change?

Yes, this change should help us improve both existing JS debugging, eg avoiding
pause on errors in browser system code,  and create new opportunities to apply
JS debugging to the browser system itself. 

> Sounds like it adds new aspects/complexities to the code.

Well I hope not! The current CL may appear more complex because it centralizes
the complexity that is currently distributed and because it is still a work in
progress.

Goals:
  Gather the "context category" settings like 'page' and 'worker' into a list so
we know all possible value (CompilationContextCategory).
  Decouple the debugId from the context category so we don't need to encode them
in a string on the C++ side then decode the string on the JS side (debugId).
  Replace use of the zero slot number distributed through out the code with
calls in to one module that uses the V8 embedding slot.
  Move the transfer of blink debugging information from context onto script from
a mysterious couple of lines in V8 compiler.cc to blink binding code for
debugging.

If these goals can be met, extending the system to, for example, fix the
preprocessor would be much easier.  The preprocessor requires two new markers in
addition to the current context-category and debugId: who is sending JS code to
the compiler (origin-category) and how is the compilation restricted
(restriction). Adding these to the current system would result in encoded
strings like 'page, 12, debugger, oneFunction'. These strings would have to be
decoded and re-encoded, and if the current pattern were to be followed, these
operations would be spread out over the code. 

I added the marker for context-category in
https://codereview.chromium.org/13575004/ but when I realized that more
information would be needed I started to look into this new direction. By
improving the binding system we can make it clearer and easier to extend.

If you agree with the goals perhaps we can find some better way to implement
them.  I think the enum names are ObnoxiouslyLongAndTedious but for all the
common cases they are limited to two files.  Call sites into the DebugData
settings are just a function call or a declaration of a V8ScopedCompilation.

[johnjbarton, 2013-04-24 23:21:35]

I found a different way to implement the preprocessor filter so I won't need
this CL.  Thanks for the feedback.