WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 83159
[chromium] Only shut down V8 if we actually initialized it
https://bugs.webkit.org/show_bug.cgi?id=83159
Summary
[chromium] Only shut down V8 if we actually initialized it
jochen
Reported
2012-04-04 07:18:42 PDT
[chromium] Only shut down V8 if we actually initialized it
Attachments
Patch
(1.94 KB, patch)
2012-04-04 07:19 PDT
,
jochen
no flags
Details
Formatted Diff
Diff
Patch
(2.31 KB, patch)
2012-04-04 07:57 PDT
,
jochen
no flags
Details
Formatted Diff
Diff
Patch
(2.36 KB, patch)
2012-04-05 00:48 PDT
,
jochen
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
jochen
Comment 1
2012-04-04 07:19:25 PDT
Created
attachment 135585
[details]
Patch
Yury Semikhatsky
Comment 2
2012-04-04 07:40:12 PDT
Comment on
attachment 135585
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=135585&action=review
> Source/WebKit/chromium/src/WebKit.cpp:82 > +static bool s_V8Initialized = false;
Pleas guard it with #ifndef NDEBUG
jochen
Comment 3
2012-04-04 07:57:45 PDT
Created
attachment 135589
[details]
Patch
WebKit Review Bot
Comment 4
2012-04-04 07:59:17 PDT
Please wait for approval from
abarth@webkit.org
,
dglazkov@chromium.org
,
fishd@chromium.org
,
jamesr@chromium.org
or
tkent@chromium.org
before submitting, as this patch contains changes to the Chromium public API. See also
https://trac.webkit.org/wiki/ChromiumWebKitAPI
.
jochen
Comment 5
2012-04-04 07:59:54 PDT
(In reply to
comment #2
)
> (From update of
attachment 135585
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=135585&action=review
> > > Source/WebKit/chromium/src/WebKit.cpp:82 > > +static bool s_V8Initialized = false; > > Pleas guard it with #ifndef NDEBUG
Pavel suggested to introduce shutdownWithoutV8 as a counterpart to initializeWithoutV8 instead
Darin Fisher (:fishd, Google)
Comment 6
2012-04-04 13:09:24 PDT
Hmm... I think the first patch might be nicer because it is less error prone. Embedders will not need to keep track of how they initialized WebKit. I'd probably modify the first patch to name the static variable more specifically, like: "s_addedV8CallCompletedCallback"
Darin Fisher (:fishd, Google)
Comment 7
2012-04-04 13:12:46 PDT
It also seems like we could more tightly couple the call to AddCallCompletedCallback with the existence of s_endOfTaskRunner. Then, we could just have WebKit::shutdown() null-test s_endOfTaskRunner to determine if it should call RemoveCallCompletedCallback. They really do go hand-in-hand.
jochen
Comment 8
2012-04-05 00:48:24 PDT
Created
attachment 135770
[details]
Patch
Pavel Feldman
Comment 9
2012-04-05 00:52:29 PDT
Comment on
attachment 135770
[details]
Patch Rubber stamping since I think you've done what Darin was suggesting.
WebKit Review Bot
Comment 10
2012-04-05 01:53:20 PDT
Comment on
attachment 135770
[details]
Patch Clearing flags on attachment: 135770 Committed
r113301
: <
http://trac.webkit.org/changeset/113301
>
WebKit Review Bot
Comment 11
2012-04-05 01:53:26 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.
Top of Page
Format For Printing
XML
Clone This Bug