Bug 83159 – [chromium] Only shut down V8 if we actually initialized it

• Component: New Bugs
• Status: RESOLVED FIXED

[chromium] Only shut down V8 if we actually initialized it

Created attachment 135585 [details]
Patch

[Yury Semikhatsky]

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

Created attachment 135589 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

(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)]

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)]

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.

Created attachment 135770 [details]
Patch

[Pavel Feldman]

Comment on attachment 135770 [details]
Patch

Rubber stamping since I think you've done what Darin was suggesting.

[WebKit Review Bot]

Comment on attachment 135770 [details]
Patch

Clearing flags on attachment: 135770

Committed r113301: <http://trac.webkit.org/changeset/113301>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.