Bug 95156

Summary: [V8] ScriptController::matchesCurrentContext duplicates code from ScriptController::currentWorldContext
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, haraken, japhet, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Adam Barth
Reported 2012-08-27 17:19:35 PDT
[V8] ScriptController::matchesCurrentContext duplicates code from ScriptController::currentWorldContext
Attachments
Patch (22.56 KB, patch)
2012-08-27 17:21 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (9.85 KB, patch)
2012-08-27 17:47 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch for landing (9.73 KB, patch)
2012-08-29 09:46 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Adam Barth
Comment 1 2012-08-27 17:21:20 PDT
Created attachment 160865 [details] Patch
Kentaro Hara
Comment 2 2012-08-27 17:24:19 PDT
Comment on attachment 160865 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160865&action=review > Source/WebCore/bindings/v8/ScriptController.cpp:447 > + return v8::Context::GetCurrent() == contextForCurrentWorldPrivate(); CodeGeneratorV8.pm is the only caller site of matchesCurrentContext(). You might want to write this in CodeGeneratorV8.pm and remove matchesCurrentContext().
Adam Barth
Comment 3 2012-08-27 17:31:13 PDT
(In reply to comment #2) > (From update of attachment 160865 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=160865&action=review > > > Source/WebCore/bindings/v8/ScriptController.cpp:447 > > + return v8::Context::GetCurrent() == contextForCurrentWorldPrivate(); > > CodeGeneratorV8.pm is the only caller site of matchesCurrentContext(). You might want to write this in CodeGeneratorV8.pm and remove matchesCurrentContext(). Good point... The downside is that then I'd need to make contextForCurrentWorldPrivate non-private. Eric prefers the old names for these functions, so I'm going to try another iteration of the patch.
Adam Barth
Comment 4 2012-08-27 17:47:33 PDT
Created attachment 160869 [details] Patch
Eric Seidel (no email)
Comment 5 2012-08-27 17:49:31 PDT
Comment on attachment 160869 [details] Patch Maybe unsafeHandleToCurrentWorldContext()? Otherwise looks fine.
Adam Barth
Comment 6 2012-08-29 09:46:12 PDT
Created attachment 161250 [details] Patch for landing
WebKit Review Bot
Comment 7 2012-08-29 23:24:14 PDT
Comment on attachment 161250 [details] Patch for landing Clearing flags on attachment: 161250 Committed r127106: <http://trac.webkit.org/changeset/127106>
WebKit Review Bot
Comment 8 2012-08-29 23:24:18 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.