Bug 106916 - [V8] Make an Isolate parameter mandatory in SerializedScriptValue::deserialize()
Summary: [V8] Make an Isolate parameter mandatory in SerializedScriptValue::deserialize()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-15 09:34 PST by Kentaro Hara
Modified: 2013-01-16 02:03 PST (History)
3 users (show)

See Also:


Attachments
Patch (6.49 KB, patch)
2013-01-15 09:36 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
patch for landing (7.16 KB, patch)
2013-01-16 00:17 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2013-01-15 09:34:33 PST
This is one of steps to remove optional Isolate parameters.
Comment 1 Kentaro Hara 2013-01-15 09:36:43 PST
Created attachment 182792 [details]
Patch
Comment 2 Adam Barth 2013-01-15 12:35:33 PST
Comment on attachment 182792 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182792&action=review

> Source/WebCore/bindings/v8/SerializedScriptValue.h:88
> +    ScriptValue deserializeForInspector(ScriptState*);
> +    ScriptValue deserializeForInspector(ScriptState*, v8::Isolate*);

We should put the Isolate into ScriptState instead of passing them separately.  Everyone who takes a ScriptState is going to need an isolate.
Comment 3 Kentaro Hara 2013-01-16 00:17:29 PST
Created attachment 182925 [details]
patch for landing
Comment 4 Kentaro Hara 2013-01-16 00:18:08 PST
(In reply to comment #2)
> We should put the Isolate into ScriptState instead of passing them separately.  Everyone who takes a ScriptState is going to need an isolate.

Done. Thanks!
Comment 5 Kentaro Hara 2013-01-16 00:45:58 PST
We might want to implement v8::Context::GetIsolate(). At entry points from WebCore, we have a Frame or a ScriptExecutionContext, which can be converted to a v8::Context. So once v8::Context::GetIsolate() is exposed, we can get an Isolate without calling v8::Isolate::GetCurrent(). I'm implementing v8::Context::GetIsolate().
Comment 6 Kentaro Hara 2013-01-16 01:56:38 PST
(In reply to comment #5)
> I'm implementing v8::Context::GetIsolate().

The V8 patch is here: https://chromiumcodereview.appspot.com/11968011/
Comment 7 WebKit Review Bot 2013-01-16 02:03:38 PST
Comment on attachment 182925 [details]
patch for landing

Clearing flags on attachment: 182925

Committed r139854: <http://trac.webkit.org/changeset/139854>
Comment 8 WebKit Review Bot 2013-01-16 02:03:44 PST
All reviewed patches have been landed.  Closing bug.