[webkit-reviews] review denied: [Bug 14437] CRASH: RTÃ video crashes Safari : [Attachment 15346] alternate fix
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jul 3 11:24:52 PDT 2007
Anders Carlsson <andersca at apple.com> has denied Maxime BRITTO
<mbritto at pleyo.com>'s request for review:
Bug 14437: CRASH: RTÃ video crashes Safari
http://bugs.webkit.org/show_bug.cgi?id=14437
Attachment 15346: alternate fix
http://bugs.webkit.org/attachment.cgi?id=15346&action=edit
------- Additional Comments from Anders Carlsson <andersca at apple.com>
>Index: WebCore/page/mac/WebCoreFrameBridge.mm
>===================================================================
>--- WebCore/page/mac/WebCoreFrameBridge.mm (revision 23926)
>+++ WebCore/page/mac/WebCoreFrameBridge.mm (working copy)
>@@ -682,20 +682,32 @@ static HTMLFormElement *formElementFromD
> - (NSString *)stringByEvaluatingJavaScriptFromString:(NSString *)string
forceUserGesture:(BOOL)forceUserGesture
> {
> ASSERT(m_frame->document());
>+
>+ //The main frame cannot be destroyed so we can backup the global exec
state
>+ ExecState *ex = m_frame->scriptProxy()->interpreter()->globalExec();
The above comment isn't really correct here. While it is true that a script
can't cause the main frame to be destroyed,
there's no guarantee that the main frame actually is passed in here.
>+ //the script may destroy the frame if it's not the main frame
> JSValue* result = m_frame->loader()->executeScript(0, string,
forceUserGesture);
Which means that m_frame could point to garbage memory here
> JSLock lock;
>- return String(result ?
result->toString(m_frame->scriptProxy()->interpreter()->globalExec()) : "");
>+ return String(result ? result->toString(ex) : "");
And ex could point to garbage memory here too.
> - (NSAppleEventDescriptor *)aeDescByEvaluatingJavaScriptFromString:(NSString
*)
> {
>From code inspection we do however know that the only place this function is
called from is
- (NSAppleEventDescriptor *)aeDescByEvaluatingJavaScriptFromString:(NSString
*)script
{
return [[[self mainFrame] _bridge]
aeDescByEvaluatingJavaScriptFromString:script];
}
Which means that m_frame will be valid even after running the script.
> ASSERT(m_frame->document());
>+
>+ //The main frame cannot be destroyed so we can backup the global exec
state
>+ ExecState *ex = m_frame->scriptProxy()->interpreter()->globalExec();
>+
>+ //the script may destroy the frame if it's not the main frame
> JSValue* result = m_frame->loader()->executeScript(0, string, true);
>+
> if (!result) // FIXME: pass errors
> return 0;
>+
> JSLock lock;
>- return
aeDescFromJSValue(m_frame->scriptProxy()->interpreter()->globalExec(), result);
>+ return aeDescFromJSValue(ex, result);
So this code does not need to change.
More information about the webkit-reviews
mailing list