Bug 126805 - [iOS] Fix GCController::releaseExecutableMemory
Summary: [iOS] Fix GCController::releaseExecutableMemory
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-10 21:07 PST by Joseph Pecoraro
Modified: 2014-01-10 22:53 PST (History)
4 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (2.02 KB, patch)
2014-01-10 21:10 PST, Joseph Pecoraro
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2014-01-10 21:07:16 PST
GCController::releaseExecutableMemory uses JSDOMWindow::commonVM()->dynamicGlobalObject which no longer exists. It moved to VMEntryScope->globalObject. Can the same trick still be applied?
Comment 1 Joseph Pecoraro 2014-01-10 21:10:01 PST
Created attachment 220919 [details]
[PATCH] Proposed Fix

JSC folks, does this look correct? Should I be null checking JSDOMWindow::commonVM()->entryScope now instead of JSDOMWindow::commonVM()->entryScope->globalObject()?

This is one of the iOS in OpenSource build errors.
Comment 2 Sam Weinig 2014-01-10 21:43:55 PST
Comment on attachment 220919 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=220919&action=review

> Source/WebCore/bindings/js/GCController.cpp:37
>  
> +#if PLATFORM(IOS)
> +#include <runtime/VMEntryScope.h>
> +#endif

Seems like while you are here, you might as well make this function not iOS specific as it seems generally useful (and you can remove the #ifdefs)!
Comment 3 Mark Lam 2014-01-10 21:56:39 PST
Comment on attachment 220919 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=220919&action=review

r=me with the needed fixes.

> Source/WebCore/bindings/js/GCController.cpp:120
> +    ASSERT(!JSDOMWindow::commonVM()->entryScope->globalObject());

This is not right.  The entryScope will be NULL if we haven’t entered the VM to run any JS code.  Hence, the assertion you want is:
    ASSERT(!JSDOMWindow::commonVM()->entryScope);

> Source/WebCore/bindings/js/GCController.cpp:123
> +    if (JSDOMWindow::commonVM()->entryScope->globalObject())

Similarly here, what you want is:
    if (JSDOMDOMWindow::commonVM()->entryScope)
Comment 4 Joseph Pecoraro 2014-01-10 22:53:47 PST
Landed in <http://trac.webkit.org/changeset/161747>.
And follow fix to in <http://trac.webkit.org/changeset/161748> cause even though I thought I tested building Mac, my terminal tricked me into thinking it compiled!