Commit 95a8197d authored by alice.liu@apple.com's avatar alice.liu@apple.com

Reviewed by Darin.

        Fixed <rdar://problem/5741440> REGRESSION (r28496): After deactivating JavaScript, scripts embedded in the HTML page continue to run
     
        Before this patch, Frame::scriptProxy() would only return null in the case that javascript was 
        disabled and if the script proxy field wasn't set (which would only be the case if the window 
        hasn't loaded anything yet).  Not all callers of scriptProxy() always check for a non-null return 
        value.  Those that did check would effectively be checking if javascript was enabled before proceeding.
        This fix consists of 2 elements: first, make sure that scriptProxy() will never return null, regardless 
        of whether javascript is disabled.  This will mean that callers who don't check for null won't crash.  
        Second, callers who did check for null now instead check for javascript being disabled.  This means that 
        code paths intended for preventing javascript from being run will be making the correct check.  Another
        minor addition to this patch is that I added a function on Frame to be a shortcut for checking if javascript
        is enabled. 

        * bindings/js/JSCustomSQLStatementCallback.cpp:
        (WebCore::JSCustomSQLStatementCallback::handleEvent):
        * bindings/js/JSCustomSQLStatementErrorCallback.cpp:
        (WebCore::JSCustomSQLStatementErrorCallback::handleEvent):
        * bindings/js/JSCustomSQLTransactionCallback.cpp:
        (WebCore::JSCustomSQLTransactionCallback::handleEvent):
        * bindings/js/JSCustomSQLTransactionErrorCallback.cpp:
        (WebCore::JSCustomSQLTransactionErrorCallback::handleEvent):
        * bindings/js/JSCustomVoidCallback.cpp:
        (WebCore::JSCustomVoidCallback::handleEvent):
        * bindings/js/JSCustomXPathNSResolver.cpp:
        (WebCore::JSCustomXPathNSResolver::lookupNamespaceURI):
        * bindings/js/ScheduledAction.cpp:
        (WebCore::ScheduledAction::execute):
        * bindings/js/kjs_events.cpp:
        (WebCore::JSAbstractEventListener::handleEvent):
        (WebCore::JSLazyEventListener::parseCode):
        * bindings/js/kjs_html.cpp:
        (WebCore::runtimeObjectImplementsCall):
        * bindings/js/kjs_proxy.cpp:
        (WebCore::KJSProxy::isEnabled):
        * bindings/js/kjs_proxy.h:
        * bindings/js/kjs_window.cpp:
        (KJS::Window::retrieveWindow):
        (KJS::Window::retrieve):
        * dom/Document.cpp:
        (WebCore::Document::createHTMLEventListener):
        * dom/EventTarget.cpp:
        (WebCore::EventTarget::dispatchGenericEvent):
        * html/HTMLPlugInElement.cpp:
        (WebCore::HTMLPlugInElement::createNPObject):
        * html/HTMLScriptElement.cpp:
        (WebCore::HTMLScriptElement::evaluateScript):
        * html/HTMLTokenizer.cpp:
        (WebCore::HTMLTokenizer::parseTag):
        (WebCore::HTMLTokenizer::processToken):
        * loader/FrameLoader.cpp:
        (WebCore::FrameLoader::executeScript):
        (WebCore::FrameLoader::userGestureHint):
        (WebCore::FrameLoader::open):
        (WebCore::FrameLoader::dispatchWindowObjectAvailable):
        (WebCore::FrameLoader::switchOutLowBandwidthDisplayIfReady):
        * manual-tests/disable-javascript-reload.html: Added.
        * page/Frame.cpp:
        (WebCore::Frame::scriptProxy):
        (WebCore::Frame::bindingRootObject):
        (WebCore::Frame::windowScriptNPObject):
        * page/Frame.h:
        * page/InspectorController.cpp:
        (WebCore::canPassNodeToJavaScript):
        * page/mac/FrameMac.mm:
        (WebCore::Frame::windowScriptObject):
        * svg/SVGDocumentExtensions.cpp:
        (WebCore::SVGDocumentExtensions::createSVGEventListener):



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@30325 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 7f60f6b7
2008-02-15 Alice Liu <alice.liu@apple.com>
Reviewed by Darin.
Fixed <rdar://problem/5741440> REGRESSION (r28496): After deactivating JavaScript, scripts embedded in the HTML page continue to run
Before this patch, Frame::scriptProxy() would only return null in the case that javascript was
disabled and if the script proxy field wasn't set (which would only be the case if the window
hasn't loaded anything yet). Not all callers of scriptProxy() always check for a non-null return
value. Those that did check would effectively be checking if javascript was enabled before proceeding.
This fix consists of 2 elements: first, make sure that scriptProxy() will never return null, regardless
of whether javascript is disabled. This will mean that callers who don't check for null won't crash.
Second, callers who did check for null now instead check for javascript being disabled. This means that
code paths intended for preventing javascript from being run will be making the correct check. Another
minor addition to this patch is that I added a function on Frame to be a shortcut for checking if javascript
is enabled.
* bindings/js/JSCustomSQLStatementCallback.cpp:
(WebCore::JSCustomSQLStatementCallback::handleEvent):
* bindings/js/JSCustomSQLStatementErrorCallback.cpp:
(WebCore::JSCustomSQLStatementErrorCallback::handleEvent):
* bindings/js/JSCustomSQLTransactionCallback.cpp:
(WebCore::JSCustomSQLTransactionCallback::handleEvent):
* bindings/js/JSCustomSQLTransactionErrorCallback.cpp:
(WebCore::JSCustomSQLTransactionErrorCallback::handleEvent):
* bindings/js/JSCustomVoidCallback.cpp:
(WebCore::JSCustomVoidCallback::handleEvent):
* bindings/js/JSCustomXPathNSResolver.cpp:
(WebCore::JSCustomXPathNSResolver::lookupNamespaceURI):
* bindings/js/ScheduledAction.cpp:
(WebCore::ScheduledAction::execute):
* bindings/js/kjs_events.cpp:
(WebCore::JSAbstractEventListener::handleEvent):
(WebCore::JSLazyEventListener::parseCode):
* bindings/js/kjs_html.cpp:
(WebCore::runtimeObjectImplementsCall):
* bindings/js/kjs_proxy.cpp:
(WebCore::KJSProxy::isEnabled):
* bindings/js/kjs_proxy.h:
* bindings/js/kjs_window.cpp:
(KJS::Window::retrieveWindow):
(KJS::Window::retrieve):
* dom/Document.cpp:
(WebCore::Document::createHTMLEventListener):
* dom/EventTarget.cpp:
(WebCore::EventTarget::dispatchGenericEvent):
* html/HTMLPlugInElement.cpp:
(WebCore::HTMLPlugInElement::createNPObject):
* html/HTMLScriptElement.cpp:
(WebCore::HTMLScriptElement::evaluateScript):
* html/HTMLTokenizer.cpp:
(WebCore::HTMLTokenizer::parseTag):
(WebCore::HTMLTokenizer::processToken):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::executeScript):
(WebCore::FrameLoader::userGestureHint):
(WebCore::FrameLoader::open):
(WebCore::FrameLoader::dispatchWindowObjectAvailable):
(WebCore::FrameLoader::switchOutLowBandwidthDisplayIfReady):
* manual-tests/disable-javascript-reload.html: Added.
* page/Frame.cpp:
(WebCore::Frame::scriptProxy):
(WebCore::Frame::bindingRootObject):
(WebCore::Frame::windowScriptNPObject):
* page/Frame.h:
* page/InspectorController.cpp:
(WebCore::canPassNodeToJavaScript):
* page/mac/FrameMac.mm:
(WebCore::Frame::windowScriptObject):
* svg/SVGDocumentExtensions.cpp:
(WebCore::SVGDocumentExtensions::createSVGEventListener):
2008-02-15 Dan Bernstein <mitz@apple.com>
Reviewed by Alexey Proskuryakov.
......@@ -51,11 +51,10 @@ void JSCustomSQLStatementCallback::handleEvent(SQLTransaction* transaction, SQLR
ASSERT(m_callback);
ASSERT(m_frame);
KJSProxy* proxy = m_frame->scriptProxy();
if (!proxy)
if (!m_frame->scriptProxy()->isEnabled())
return;
JSGlobalObject* globalObject = proxy->globalObject();
JSGlobalObject* globalObject = m_frame->scriptProxy()->globalObject();
ExecState* exec = globalObject->globalExec();
KJS::JSLock lock;
......
......@@ -51,11 +51,10 @@ bool JSCustomSQLStatementErrorCallback::handleEvent(SQLTransaction* transaction,
ASSERT(m_callback);
ASSERT(m_frame);
KJSProxy* proxy = m_frame->scriptProxy();
if (!proxy)
if (!m_frame->scriptProxy()->isEnabled())
return true;
JSGlobalObject* globalObject = proxy->globalObject();
JSGlobalObject* globalObject = m_frame->scriptProxy()->globalObject();
ExecState* exec = globalObject->globalExec();
KJS::JSLock lock;
......
......@@ -103,11 +103,10 @@ void JSCustomSQLTransactionCallback::handleEvent(SQLTransaction* transaction, bo
ASSERT(m_data->callback());
ASSERT(m_data->frame());
KJSProxy* proxy = m_data->frame()->scriptProxy();
if (!proxy)
if (!m_data->frame()->scriptProxy()->isEnabled())
return;
JSGlobalObject* globalObject = proxy->globalObject();
JSGlobalObject* globalObject = m_data->frame()->scriptProxy()->globalObject();
ExecState* exec = globalObject->globalExec();
KJS::JSLock lock;
......
......@@ -50,11 +50,10 @@ bool JSCustomSQLTransactionErrorCallback::handleEvent(SQLError* error)
ASSERT(m_callback);
ASSERT(m_frame);
KJSProxy* proxy = m_frame->scriptProxy();
if (!proxy)
if (!m_frame->scriptProxy()->isEnabled())
return true;
JSGlobalObject* globalObject = proxy->globalObject();
JSGlobalObject* globalObject = m_frame->scriptProxy()->globalObject();
ExecState* exec = globalObject->globalExec();
KJS::JSLock lock;
......
......@@ -52,11 +52,10 @@ void JSCustomVoidCallback::handleEvent()
ASSERT(m_callback);
ASSERT(m_frame);
KJSProxy* proxy = m_frame->scriptProxy();
if (!proxy)
if (!m_frame->scriptProxy()->isEnabled())
return;
JSGlobalObject* globalObject = proxy->globalObject();
JSGlobalObject* globalObject = m_frame->scriptProxy()->globalObject();
ExecState* exec = globalObject->globalExec();
KJS::JSLock lock;
......
......@@ -73,13 +73,12 @@ String JSCustomXPathNSResolver::lookupNamespaceURI(const String& prefix)
if (!m_frame)
return String();
KJSProxy* proxy = m_frame->scriptProxy();
if (!proxy)
if (!m_frame->scriptProxy()->isEnabled())
return String();
JSLock lock;
JSGlobalObject* globalObject = proxy->globalObject();
JSGlobalObject* globalObject = m_frame->scriptProxy()->globalObject();
ExecState* exec = globalObject->globalExec();
JSValue* lookupNamespaceURIFuncValue = m_customResolver->get(exec, "lookupNamespaceURI");
......
......@@ -52,10 +52,10 @@ void ScheduledAction::execute(Window* window)
if (!frame)
return;
KJSProxy* scriptProxy = frame->scriptProxy();
if (!scriptProxy)
if (!frame->scriptProxy()->isEnabled())
return;
KJSProxy* scriptProxy = frame->scriptProxy();
Window* globalObject = scriptProxy->globalObject();
scriptProxy->setProcessingTimerCallback(true);
......
......@@ -74,13 +74,12 @@ void JSAbstractEventListener::handleEvent(Event* ele, bool isWindowEvent)
Frame *frame = window->impl()->frame();
if (!frame)
return;
KJSProxy* proxy = frame->scriptProxy();
if (!proxy)
if (!frame->scriptProxy()->isEnabled())
return;
JSLock lock;
JSGlobalObject* globalObject = proxy->globalObject();
JSGlobalObject* globalObject = frame->scriptProxy()->globalObject();
ExecState* exec = globalObject->globalExec();
JSValue* handleEventFuncValue = listener->get(exec, "handleEvent");
......@@ -288,15 +287,11 @@ void JSLazyEventListener::parseCode() const
m_parsed = true;
Frame* frame = windowObj()->impl()->frame();
KJSProxy* proxy = 0;
if (frame)
proxy = frame->scriptProxy();
if (proxy) {
ExecState* exec = proxy->globalObject()->globalExec();
if (frame && frame->scriptProxy()->isEnabled()) {
ExecState* exec = frame->scriptProxy()->globalObject()->globalExec();
JSLock lock;
JSObject* constr = proxy->globalObject()->functionConstructor();
JSObject* constr = frame->scriptProxy()->globalObject()->functionConstructor();
List args;
UString sourceURL(frame->loader()->url().string());
......
......@@ -130,8 +130,7 @@ bool runtimeObjectImplementsCall(HTMLElement* thisImp)
Frame* frame = thisImp->document()->frame();
if (!frame)
return false;
KJSProxy* proxy = frame->scriptProxy();
ExecState* exec = proxy->globalObject()->globalExec();
ExecState* exec = frame->scriptProxy()->globalObject()->globalExec();
if (JSValue* runtimeObject = getRuntimeObject(exec, thisImp))
return static_cast<JSObject*>(runtimeObject)->implementsCall();
......
......@@ -197,4 +197,10 @@ bool KJSProxy::processingUserGesture() const
return false;
}
bool KJSProxy::isEnabled()
{
Settings* settings = m_frame->settings();
return (settings && settings->isJavaScriptEnabled());
}
} // namespace WebCore
......@@ -66,6 +66,8 @@ public:
void setProcessingTimerCallback(bool b) { m_processingTimerCallback = b; }
bool processingUserGesture() const;
bool isEnabled();
private:
void initScriptIfNeeded()
{
......
......@@ -242,7 +242,7 @@ Window* Window::retrieveWindow(Frame* frame)
{
JSObject* o = retrieve(frame)->getObject();
ASSERT(o || !frame->settings() || !frame->settings()->isJavaScriptEnabled());
ASSERT(o || !frame->scriptProxy()->isEnabled());
return static_cast<Window*>(o);
}
......@@ -256,8 +256,8 @@ Window* Window::retrieveActive(ExecState* exec)
JSValue* Window::retrieve(Frame* frame)
{
ASSERT(frame);
if (KJSProxy* proxy = frame->scriptProxy())
return proxy->globalObject(); // the Global object is the "window"
if (frame->scriptProxy()->isEnabled())
return frame->scriptProxy()->globalObject(); // the Global object is the "window"
return jsUndefined(); // This can happen with JS disabled on the domain of that window
}
......
......@@ -2530,8 +2530,8 @@ bool Document::hasWindowEventListener(const AtomicString &eventType)
PassRefPtr<EventListener> Document::createHTMLEventListener(const String& functionName, const String& code, Node *node)
{
if (Frame* frm = frame())
if (KJSProxy* proxy = frm->scriptProxy())
return proxy->createHTMLEventHandler(functionName, code, node);
if (frm->scriptProxy()->isEnabled())
return frm->scriptProxy()->createHTMLEventHandler(functionName, code, node);
return 0;
}
......
......@@ -269,7 +269,7 @@ bool EventTarget::dispatchGenericEvent(EventTargetNode* referenceNode, PassRefPt
// have a reference to it in a variable. So there is no need for
// the interpreter to keep the event in it's cache
Frame* frame = referenceNode->document()->frame();
if (tempEvent && frame && frame->scriptProxy())
if (tempEvent && frame && frame->scriptProxy()->isEnabled())
frame->scriptProxy()->finishedWithEvent(evt.get());
return !evt->defaultPrevented(); // ### what if defaultPrevented was called before dispatchEvent?
......
......@@ -180,7 +180,7 @@ NPObject* HTMLPlugInElement::createNPObject()
}
// Can't create NPObjects when JavaScript is disabled
if (!settings->isJavaScriptEnabled())
if (!frame->scriptProxy()->isEnabled())
return _NPN_CreateNoScriptObject();
// Create a JSObject bound to this element
......
......@@ -233,10 +233,9 @@ void HTMLScriptElement::evaluateScript(const String& url, const String& script)
Frame* frame = document()->frame();
if (frame) {
KJSProxy* proxy = frame->scriptProxy();
if (proxy) {
if (frame->scriptProxy()->isEnabled()) {
m_evaluated = true;
proxy->evaluate(url, 0, script);
frame->scriptProxy()->evaluate(url, 0, script);
Document::updateDocumentsRendering();
}
}
......
......@@ -1209,8 +1209,7 @@ HTMLTokenizer::State HTMLTokenizer::parseTag(SegmentedString &src, State state)
scriptSrc = String();
scriptSrcCharset = String();
if (currToken.attrs && !m_fragment) {
Settings* settings = m_doc->settings();
if (settings && settings->isJavaScriptEnabled()) {
if (m_doc->frame()->scriptProxy()->isEnabled()) {
if ((a = currToken.attrs->getAttributeItem(srcAttr)))
scriptSrc = m_doc->completeURL(parseURL(a->value())).string();
if ((a = currToken.attrs->getAttributeItem(charsetAttr)))
......@@ -1603,7 +1602,7 @@ void HTMLTokenizer::finish()
PassRefPtr<Node> HTMLTokenizer::processToken()
{
KJSProxy* jsProxy = (!m_fragment && m_doc->frame()) ? m_doc->frame()->scriptProxy() : 0;
if (jsProxy)
if (jsProxy && m_doc->frame()->scriptProxy()->isEnabled())
jsProxy->setEventHandlerLineno(tagStartLineno);
if (dest > buffer) {
currToken.text = StringImpl::createStrippingNullCharacters(buffer, dest - buffer);
......
......@@ -747,14 +747,13 @@ JSValue* FrameLoader::executeScript(const String& script, bool forceUserGesture)
JSValue* FrameLoader::executeScript(const String& url, int baseLine, const String& script)
{
KJSProxy* proxy = m_frame->scriptProxy();
if (!proxy)
if (!m_frame->scriptProxy()->isEnabled())
return 0;
bool wasRunningScript = m_isRunningScript;
m_isRunningScript = true;
JSValue* result = proxy->evaluate(url, baseLine, script);
JSValue* result = m_frame->scriptProxy()->evaluate(url, baseLine, script);
if (!wasRunningScript) {
m_isRunningScript = false;
......@@ -1712,7 +1711,7 @@ bool FrameLoader::userGestureHint()
while (rootFrame->tree()->parent())
rootFrame = rootFrame->tree()->parent();
if (rootFrame->scriptProxy())
if (rootFrame->scriptProxy()->isEnabled())
return rootFrame->scriptProxy()->processingUserGesture();
return true; // If JavaScript is disabled, a user gesture must have initiated the navigation
......@@ -2736,8 +2735,7 @@ void FrameLoader::open(CachedPage& cachedPage)
m_didCallImplicitClose = true;
// Delete old status bar messages (if it _was_ activated on last URL).
Settings* settings = m_frame->settings();
if (settings && settings->isJavaScriptEnabled()) {
if (m_frame->scriptProxy()->isEnabled()) {
m_frame->setJSStatusBarText(String());
m_frame->setJSDefaultStatusBarText(String());
}
......@@ -4547,8 +4545,7 @@ String FrameLoader::referrer() const
void FrameLoader::dispatchWindowObjectAvailable()
{
Settings* settings = m_frame->settings();
if (!settings || !settings->isJavaScriptEnabled() || !m_frame->scriptProxy()->haveGlobalObject())
if (!m_frame->scriptProxy()->isEnabled() || !m_frame->scriptProxy()->haveGlobalObject())
return;
m_client->windowObjectCleared();
......@@ -4779,7 +4776,7 @@ void FrameLoader::switchOutLowBandwidthDisplayIfReady()
// similar to clear(), should be refactored to share more code
oldDoc->cancelParsing();
oldDoc->detach();
if (m_frame->scriptProxy())
if (m_frame->scriptProxy()->isEnabled())
m_frame->scriptProxy()->clear();
if (m_frame->view())
m_frame->view()->clear();
......
<html>
<head>
<script>
function test()
{
document.getElementById("manualDirections").setAttribute("style","display:block;");
document.getElementById("console").innerHTML = "FAIL - disabled javascript hasn't been applied to refreshed webpage";
}
</script>
</head>
<body onload="test();">
<div id="manualDirections" style="display:none;">
To run this test manually, disable javascript and reload the page.
<br><br>
</div>
<div id="console">
PASS
</div>
</body
</html>
......@@ -230,11 +230,8 @@ void Frame::setView(FrameView* view)
KJSProxy *Frame::scriptProxy()
{
if (!d->m_jscript) {
Settings* settings = this->settings();
if (settings && settings->isJavaScriptEnabled())
d->m_jscript = new KJSProxy(this);
}
if (!d->m_jscript)
d->m_jscript = new KJSProxy(this);
return d->m_jscript;
}
......@@ -1034,8 +1031,7 @@ void Frame::lifeSupportTimerFired(Timer<Frame>*)
KJS::Bindings::RootObject* Frame::bindingRootObject()
{
Settings* settings = this->settings();
if (!settings || !settings->isJavaScriptEnabled())
if (!scriptProxy()->isEnabled())
return 0;
if (!d->m_bindingRootObject) {
......@@ -1061,8 +1057,7 @@ PassRefPtr<KJS::Bindings::RootObject> Frame::createRootObject(void* nativeHandle
NPObject* Frame::windowScriptNPObject()
{
if (!d->m_windowScriptNPObject) {
Settings* settings = this->settings();
if (settings && settings->isJavaScriptEnabled()) {
if (scriptProxy()->isEnabled()) {
// JavaScript is enabled, so there is a JavaScript window object. Return an NPObject bound to the window
// object.
KJS::JSLock lock;
......
......@@ -630,7 +630,7 @@ static bool canPassNodeToJavaScript(Node* node)
if (!node)
return false;
Frame* frame = node->document()->frame();
return frame && frame->scriptProxy();
return frame && frame->scriptProxy()->isEnabled();
}
void InspectorController::inspect(Node* node)
......
......@@ -638,8 +638,7 @@ KJS::Bindings::Instance* Frame::createScriptInstanceForWidget(WebCore::Widget* w
WebScriptObject* Frame::windowScriptObject()
{
Settings* settings = this->settings();
if (!settings || !settings->isJavaScriptEnabled())
if (!scriptProxy()->isEnabled())
return 0;
if (!d->m_windowScriptObject) {
......
......@@ -54,8 +54,8 @@ SVGDocumentExtensions::~SVGDocumentExtensions()
PassRefPtr<EventListener> SVGDocumentExtensions::createSVGEventListener(const String& functionName, const String& code, Node *node)
{
if (Frame* frame = m_doc->frame())
if (KJSProxy* proxy = frame->scriptProxy())
return proxy->createSVGEventHandler(functionName, code, node);
if (frame->scriptProxy()->isEnabled())
return frame->scriptProxy()->createSVGEventHandler(functionName, code, node);
return 0;
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment