Bug 58096 - Re-land http://trac.webkit.org/changeset/83007. Needed to lock v8 before using.
Summary: Re-land http://trac.webkit.org/changeset/83007. Needed to lock v8 before using.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Aaron Boodman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-07 16:21 PDT by Aaron Boodman
Modified: 2011-04-08 17:55 PDT (History)
6 users (show)

See Also:

Attachments
Patch (8.62 KB, patch)
2011-04-07 16:22 PDT, Aaron Boodman 		no flags Details | Formatted Diff | Diff
Patch (8.47 KB, patch)
2011-04-08 15:29 PDT, Aaron Boodman 		no flags Details | Formatted Diff | Diff
Patch (8.56 KB, patch)
2011-04-08 16:35 PDT, Aaron Boodman 		dimich: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Aaron Boodman 2011-04-07 16:21:54 PDT 
Re-land http://trac.webkit.org/changeset/83007. Needed to lock v8 before using.
Comment 1 Aaron Boodman 2011-04-07 16:22:31 PDT 
Created attachment 88726 [details]
Patch
Comment 2 Aaron Boodman 2011-04-07 16:36:20 PDT 
Adam, Darin: The only change from the previous patch you reviewed is the addition of v8::Locker in WebFrameTest.cpp > FrameForEnteredContext.
Comment 3 Aaron Boodman 2011-04-07 16:37:17 PDT 
Oh, and the reason I didn't see this earlier is that the build bots randomize the order of the test runs and in order for this CHECK to be hit, v8 has to be used with a locker once before my code ran. There is one test that does that.
Comment 4 Adam Barth 2011-04-07 16:41:08 PDT 
Comment on attachment 88726 [details]
Patch

Yep.  Sorry I didn't catch the Locker issue.  It's bene a while since I worked on code that enters V8 from an empty stack.
Comment 5 Dmitry Titov 2011-04-08 13:14:14 PDT 
With bug 58110 resolved, the Lockers are no longer needed.
I've verified Aaron's patch w/o lockers does not fail on:
webkit_unit_tests --gtest_repeat=-1 --gtest_shuffle
which was the case before. So the patch, sans the Locker line, can be landed now.
Comment 6 Aaron Boodman 2011-04-08 15:29:22 PDT 
Created attachment 88883 [details]
Patch

New pach with removed references to v8.h and v8::Locker. This is the same patch that was previously reviewed and landed.
Comment 7 Eric Seidel (no email) 2011-04-08 15:39:13 PDT 
Attachment 88883 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8377161
Comment 8 Adam Barth 2011-04-08 15:56:35 PDT 
/Projects/CrMacEWS/Source/WebKit/chromium/tests/WebFrameTest.cpp:136: error: 'HandleScope' is not a member of 'v8'

Looks like we still need v8.h.
Comment 9 WebKit Review Bot 2011-04-08 16:00:25 PDT 
Attachment 88883 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8377172
Comment 10 Aaron Boodman 2011-04-08 16:35:45 PDT 
Created attachment 88893 [details]
Patch

fffff...