WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
58096
Re-land
http://trac.webkit.org/changeset/83007
. Needed to lock v8 before using.
https://bugs.webkit.org/show_bug.cgi?id=58096
Summary
Re-land http://trac.webkit.org/changeset/83007. Needed to lock v8 before using.
Aaron Boodman
Reported
2011-04-07 16:21:54 PDT
Re-land
http://trac.webkit.org/changeset/83007
. Needed to lock v8 before using.
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 attachment
proposed patch, testcase, etc.
Aaron Boodman
Comment 1
2011-04-07 16:22:31 PDT
Created
attachment 88726
[details]
Patch
Aaron Boodman
Comment 2
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.
Aaron Boodman
Comment 3
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.
Adam Barth
Comment 4
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.
Dmitry Titov
Comment 5
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.
Aaron Boodman
Comment 6
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.
Eric Seidel (no email)
Comment 7
2011-04-08 15:39:13 PDT
Attachment 88883
[details]
did not build on chromium: Build output:
http://queues.webkit.org/results/8377161
Adam Barth
Comment 8
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.
WebKit Review Bot
Comment 9
2011-04-08 16:00:25 PDT
Attachment 88883
[details]
did not build on chromium: Build output:
http://queues.webkit.org/results/8377172
Aaron Boodman
Comment 10
2011-04-08 16:35:45 PDT
Created
attachment 88893
[details]
Patch fffff...
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug