Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[Chromium API] Disambiguate allowJavaScript from didNotAllowScript
https://bugs.webkit.org/show_bug.cgi?id=35205

Patch by Adam Barth <abarth@webkit.org> on 2010-02-24
Reviewed by Darin Fisher.

WebCore:

For clients that want to show a user interface element when JavaScript
was blocked on a page, we need to disambiguate between querying the
client for whether JavaScript is enabled from actually failing to
execute some script.

This patch adds a new FrameLoaderClient callback for when WebCore would
like to execute JavaScript but fails to because JavaScript is disabled.

This patch also touches every client of canExecuteScripts so they can
indicate whether we should make this callback.  I was hoping there was
a better choke point, but my first two attempts were wrong in subtle
ways.  pkasting points out that this will be easy to screw up in the
future, so it's better to make all the clients be explicit.

* WebCore.PluginHostProcess.exp:
* bindings/ScriptControllerBase.cpp:
(WebCore::ScriptController::canExecuteScripts):
(WebCore::ScriptController::executeScript):
* bindings/js/JSEventListener.cpp:
(WebCore::JSEventListener::handleEvent):
* bindings/js/JSLazyEventListener.cpp:
(WebCore::JSLazyEventListener::initializeJSFunction):
* bindings/js/ScheduledAction.cpp:
(WebCore::ScheduledAction::execute):
* bindings/js/ScriptController.cpp:
(WebCore::ScriptController::bindingRootObject):
(WebCore::ScriptController::windowScriptNPObject):
(WebCore::ScriptController::jsObjectForPluginElement):
(WebCore::ScriptController::executeScriptInWorld):
* bindings/js/ScriptController.h:
(WebCore::):
* bindings/js/ScriptControllerMac.mm:
(WebCore::ScriptController::windowScriptObject):
* bindings/js/ScriptDebugServer.cpp:
(WebCore::ScriptDebugServer::setJavaScriptPaused):
* bindings/js/ScriptEventListener.cpp:
(WebCore::createAttributeEventListener):
* bindings/js/ScriptState.cpp:
(WebCore::scriptStateFromNode):
* bindings/v8/ScriptController.cpp:
(WebCore::ScriptController::windowScriptNPObject):
(WebCore::ScriptController::createScriptObjectForPluginElement):
* bindings/v8/ScriptController.h:
(WebCore::):
* bindings/v8/ScriptEventListener.cpp:
(WebCore::createAttributeEventListener):
* bindings/v8/V8Proxy.cpp:
(WebCore::V8Proxy::retrieve):
* dom/ScriptElement.cpp:
(WebCore::ScriptElementData::evaluateScript):
* dom/XMLTokenizerLibxml2.cpp:
(WebCore::XMLTokenizer::startElementNs):
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::controls):
* html/HTMLTokenizer.cpp:
(WebCore::HTMLTokenizer::parseTag):
(WebCore::HTMLTokenizer::processToken):
* inspector/InspectorController.cpp:
(WebCore::canPassNodeToJavaScript):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::isProcessingUserGesture):
(WebCore::FrameLoader::open):
(WebCore::FrameLoader::dispatchDidClearWindowObjectsInAllWorlds):
(WebCore::FrameLoader::dispatchDidClearWindowObjectInWorld):
* loader/FrameLoaderClient.h:
(WebCore::FrameLoaderClient::didNotAllowScript):

WebKit/chromium:

Plumb didNotAllowScript through Chromium's WebKit API.

* public/WebFrameClient.h:
(WebKit::WebFrameClient::didNotAllowScript):
* src/DebuggerAgentImpl.cpp:
(WebKit::DebuggerAgentImpl::createUtilityContext):
* src/FrameLoaderClientImpl.cpp:
(WebKit::FrameLoaderClientImpl::didNotAllowScript):
* src/FrameLoaderClientImpl.h:
* src/WebFrameImpl.cpp:
(WebKit::WebFrameImpl::bindToWindowObject):

WebKit/mac:

Make these two callsites explicit about not running script immediately.

* Plugins/Hosted/NetscapePluginInstanceProxy.mm:
(WebKit::NetscapePluginInstanceProxy::getWindowNPObject):
(WebKit::NetscapePluginInstanceProxy::demarshalValueFromArray):



Canonical link: https://commits.webkit.org/46509@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@55207 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Adam Barth authored and pkasting committed Feb 25, 2010
1 parent e97f2af commit f3ca3bd
Show file tree
Hide file tree
Showing 31 changed files with 174 additions and 38 deletions.
74 changes: 74 additions & 0 deletions WebCore/ChangeLog
@@ -1,3 +1,77 @@
2010-02-24 Adam Barth <abarth@webkit.org>

Reviewed by Darin Fisher.

[Chromium API] Disambiguate allowJavaScript from didNotAllowScript
https://bugs.webkit.org/show_bug.cgi?id=35205

For clients that want to show a user interface element when JavaScript
was blocked on a page, we need to disambiguate between querying the
client for whether JavaScript is enabled from actually failing to
execute some script.

This patch adds a new FrameLoaderClient callback for when WebCore would
like to execute JavaScript but fails to because JavaScript is disabled.

This patch also touches every client of canExecuteScripts so they can
indicate whether we should make this callback. I was hoping there was
a better choke point, but my first two attempts were wrong in subtle
ways. pkasting points out that this will be easy to screw up in the
future, so it's better to make all the clients be explicit.

* WebCore.PluginHostProcess.exp:
* bindings/ScriptControllerBase.cpp:
(WebCore::ScriptController::canExecuteScripts):
(WebCore::ScriptController::executeScript):
* bindings/js/JSEventListener.cpp:
(WebCore::JSEventListener::handleEvent):
* bindings/js/JSLazyEventListener.cpp:
(WebCore::JSLazyEventListener::initializeJSFunction):
* bindings/js/ScheduledAction.cpp:
(WebCore::ScheduledAction::execute):
* bindings/js/ScriptController.cpp:
(WebCore::ScriptController::bindingRootObject):
(WebCore::ScriptController::windowScriptNPObject):
(WebCore::ScriptController::jsObjectForPluginElement):
(WebCore::ScriptController::executeScriptInWorld):
* bindings/js/ScriptController.h:
(WebCore::):
* bindings/js/ScriptControllerMac.mm:
(WebCore::ScriptController::windowScriptObject):
* bindings/js/ScriptDebugServer.cpp:
(WebCore::ScriptDebugServer::setJavaScriptPaused):
* bindings/js/ScriptEventListener.cpp:
(WebCore::createAttributeEventListener):
* bindings/js/ScriptState.cpp:
(WebCore::scriptStateFromNode):
* bindings/v8/ScriptController.cpp:
(WebCore::ScriptController::windowScriptNPObject):
(WebCore::ScriptController::createScriptObjectForPluginElement):
* bindings/v8/ScriptController.h:
(WebCore::):
* bindings/v8/ScriptEventListener.cpp:
(WebCore::createAttributeEventListener):
* bindings/v8/V8Proxy.cpp:
(WebCore::V8Proxy::retrieve):
* dom/ScriptElement.cpp:
(WebCore::ScriptElementData::evaluateScript):
* dom/XMLTokenizerLibxml2.cpp:
(WebCore::XMLTokenizer::startElementNs):
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::controls):
* html/HTMLTokenizer.cpp:
(WebCore::HTMLTokenizer::parseTag):
(WebCore::HTMLTokenizer::processToken):
* inspector/InspectorController.cpp:
(WebCore::canPassNodeToJavaScript):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::isProcessingUserGesture):
(WebCore::FrameLoader::open):
(WebCore::FrameLoader::dispatchDidClearWindowObjectsInAllWorlds):
(WebCore::FrameLoader::dispatchDidClearWindowObjectInWorld):
* loader/FrameLoaderClient.h:
(WebCore::FrameLoaderClient::didNotAllowScript):

2010-02-24 Adam Barth <abarth@webkit.org>

Reviewed by Darin Fisher.
Expand Down
2 changes: 1 addition & 1 deletion WebCore/WebCore.PluginHostProcess.exp
Expand Up @@ -9,7 +9,7 @@ __ZN7WebCore13IdentifierRep3getEi
__ZN7WebCore13IdentifierRep3getEPKc
__ZN7WebCore13IdentifierRep7isValidEPS0_
__ZN7WebCore16ScriptController16createRootObjectEPv
__ZN7WebCore16ScriptController17canExecuteScriptsEv
__ZN7WebCore16ScriptController17canExecuteScriptsENS_33ReasonForCallingCanExecuteScriptsE
__ZN7WebCore16ScriptController24jsObjectForPluginElementEPNS_17HTMLPlugInElementE
__ZN7WebCore6String26fromUTF8WithLatin1FallbackEPKcm
__ZN7WebCore6String8fromUTF8EPKcm
9 changes: 6 additions & 3 deletions WebCore/bindings/ScriptControllerBase.cpp
Expand Up @@ -31,14 +31,17 @@

namespace WebCore {

bool ScriptController::canExecuteScripts()
bool ScriptController::canExecuteScripts(ReasonForCallingCanExecuteScripts reason)
{
// FIXME: We should get this information from the document instead of the frame.
if (m_frame->loader()->isSandboxed(SandboxScripts))
return false;

Settings* settings = m_frame->settings();
return m_frame->loader()->client()->allowJavaScript(settings && settings->isJavaScriptEnabled());
const bool allowed = m_frame->loader()->client()->allowJavaScript(settings && settings->isJavaScriptEnabled());
if (!allowed && reason == AboutToExecuteScript)
m_frame->loader()->client()->didNotAllowScript();
return allowed;
}

ScriptValue ScriptController::executeScript(const String& script, bool forceUserGesture)
Expand All @@ -48,7 +51,7 @@ ScriptValue ScriptController::executeScript(const String& script, bool forceUser

ScriptValue ScriptController::executeScript(const ScriptSourceCode& sourceCode)
{
if (!canExecuteScripts() || isPaused())
if (!canExecuteScripts(AboutToExecuteScript) || isPaused())
return ScriptValue();

bool wasInExecuteScript = m_inExecuteScript;
Expand Down
2 changes: 1 addition & 1 deletion WebCore/bindings/js/JSEventListener.cpp
Expand Up @@ -83,7 +83,7 @@ void JSEventListener::handleEvent(ScriptExecutionContext* scriptExecutionContext
return;
// FIXME: Is this check needed for other contexts?
ScriptController* script = frame->script();
if (!script->canExecuteScripts() || script->isPaused())
if (!script->canExecuteScripts(AboutToExecuteScript) || script->isPaused())
return;
}

Expand Down
4 changes: 2 additions & 2 deletions WebCore/bindings/js/JSLazyEventListener.cpp
Expand Up @@ -79,7 +79,7 @@ JSObject* JSLazyEventListener::initializeJSFunction(ScriptExecutionContext* exec
return 0;

ScriptController* scriptController = frame->script();
if (!scriptController->canExecuteScripts())
if (!scriptController->canExecuteScripts(AboutToExecuteScript))
return 0;

JSDOMGlobalObject* globalObject = toJSDOMGlobalObject(executionContext, isolatedWorld());
Expand All @@ -93,7 +93,7 @@ JSObject* JSLazyEventListener::initializeJSFunction(ScriptExecutionContext* exec
return 0;
// FIXME: Is this check needed for non-Document contexts?
ScriptController* script = frame->script();
if (!script->canExecuteScripts() || script->isPaused())
if (!script->canExecuteScripts(AboutToExecuteScript) || script->isPaused())
return 0;
}

Expand Down
2 changes: 1 addition & 1 deletion WebCore/bindings/js/ScheduledAction.cpp
Expand Up @@ -117,7 +117,7 @@ void ScheduledAction::execute(Document* document)
return;

RefPtr<Frame> frame = window->impl()->frame();
if (!frame || !frame->script()->canExecuteScripts())
if (!frame || !frame->script()->canExecuteScripts(AboutToExecuteScript))
return;

frame->script()->setProcessingTimerCallback(true);
Expand Down
8 changes: 4 additions & 4 deletions WebCore/bindings/js/ScriptController.cpp
Expand Up @@ -317,7 +317,7 @@ void ScriptController::updateSecurityOrigin()

Bindings::RootObject* ScriptController::bindingRootObject()
{
if (!canExecuteScripts())
if (!canExecuteScripts(NotAboutToExecuteScript))
return 0;

if (!m_bindingRootObject) {
Expand All @@ -344,7 +344,7 @@ PassRefPtr<Bindings::RootObject> ScriptController::createRootObject(void* native
NPObject* ScriptController::windowScriptNPObject()
{
if (!m_windowScriptNPObject) {
if (canExecuteScripts()) {
if (canExecuteScripts(NotAboutToExecuteScript)) {
// JavaScript is enabled, so there is a JavaScript window object.
// Return an NPObject bound to the window object.
JSC::JSLock lock(SilenceAssertionsOnly);
Expand Down Expand Up @@ -377,7 +377,7 @@ NPObject* ScriptController::createScriptObjectForPluginElement(HTMLPlugInElement
JSObject* ScriptController::jsObjectForPluginElement(HTMLPlugInElement* plugin)
{
// Can't create JSObjects when JavaScript is disabled
if (!canExecuteScripts())
if (!canExecuteScripts(NotAboutToExecuteScript))
return 0;

// Create a JSObject bound to this element
Expand Down Expand Up @@ -444,7 +444,7 @@ ScriptValue ScriptController::executeScriptInWorld(DOMWrapperWorld* world, const
{
ScriptSourceCode sourceCode(script, forceUserGesture ? KURL() : m_frame->loader()->url());

if (!canExecuteScripts() || isPaused())
if (!canExecuteScripts(AboutToExecuteScript) || isPaused())
return ScriptValue();

bool wasInExecuteScript = m_inExecuteScript;
Expand Down
7 changes: 6 additions & 1 deletion WebCore/bindings/js/ScriptController.h
Expand Up @@ -62,6 +62,11 @@ class XSSAuditor;

typedef HashMap<void*, RefPtr<JSC::Bindings::RootObject> > RootObjectMap;

enum ReasonForCallingCanExecuteScripts {
AboutToExecuteScript,
NotAboutToExecuteScript
};

class ScriptController {
friend class ScriptCachedFrameData;
typedef WTF::HashMap< RefPtr<DOMWrapperWorld>, JSC::ProtectedPtr<JSDOMWindowShell> > ShellMap;
Expand Down Expand Up @@ -110,7 +115,7 @@ class ScriptController {
bool processingUserGesture(DOMWrapperWorld*) const;
bool anyPageIsProcessingUserGesture() const;

bool canExecuteScripts();
bool canExecuteScripts(ReasonForCallingCanExecuteScripts);

// Debugger can be 0 to detach any existing Debugger.
void attachDebugger(JSC::Debugger*); // Attaches/detaches in all worlds/window shells.
Expand Down
2 changes: 1 addition & 1 deletion WebCore/bindings/js/ScriptControllerMac.mm
Expand Up @@ -107,7 +107,7 @@ - (NPObject *)createPluginScriptableObject;

WebScriptObject* ScriptController::windowScriptObject()
{
if (!canExecuteScripts())
if (!canExecuteScripts(NotAboutToExecuteScript))
return 0;

if (!m_windowScriptObject) {
Expand Down
2 changes: 1 addition & 1 deletion WebCore/bindings/js/ScriptDebugServer.cpp
Expand Up @@ -378,7 +378,7 @@ void ScriptDebugServer::setJavaScriptPaused(Frame* frame, bool paused)
{
ASSERT_ARG(frame, frame);

if (!frame->script()->canExecuteScripts())
if (!frame->script()->canExecuteScripts(NotAboutToExecuteScript))
return;

frame->script()->setPaused(paused);
Expand Down
4 changes: 2 additions & 2 deletions WebCore/bindings/js/ScriptEventListener.cpp
Expand Up @@ -64,7 +64,7 @@ PassRefPtr<JSLazyEventListener> createAttributeEventListener(Node* node, Attribu
// FIXME: We should be able to provide accurate source information for frameless documents, too (e.g. for importing nodes from XMLHttpRequest.responseXML).
if (Frame* frame = node->document()->frame()) {
ScriptController* scriptController = frame->script();
if (!scriptController->canExecuteScripts())
if (!scriptController->canExecuteScripts(AboutToExecuteScript))
return 0;

if (!scriptController->xssAuditor()->canCreateInlineEventListener(attr->localName().string(), attr->value())) {
Expand Down Expand Up @@ -96,7 +96,7 @@ PassRefPtr<JSLazyEventListener> createAttributeEventListener(Frame* frame, Attri
String sourceURL;

ScriptController* scriptController = frame->script();
if (!scriptController->canExecuteScripts())
if (!scriptController->canExecuteScripts(AboutToExecuteScript))
return 0;

if (!scriptController->xssAuditor()->canCreateInlineEventListener(attr->localName().string(), attr->value())) {
Expand Down
2 changes: 1 addition & 1 deletion WebCore/bindings/js/ScriptState.cpp
Expand Up @@ -54,7 +54,7 @@ ScriptState* scriptStateFromNode(DOMWrapperWorld* world, Node* node)
Frame* frame = document->frame();
if (!frame)
return 0;
if (!frame->script()->canExecuteScripts())
if (!frame->script()->canExecuteScripts(NotAboutToExecuteScript))
return 0;
return frame->script()->globalObject(world)->globalExec();
}
Expand Down
4 changes: 2 additions & 2 deletions WebCore/bindings/v8/ScriptController.cpp
Expand Up @@ -389,7 +389,7 @@ NPObject* ScriptController::windowScriptNPObject()
if (m_windowScriptNPObject)
return m_windowScriptNPObject;

if (canExecuteScripts()) {
if (canExecuteScripts(NotAboutToExecuteScript)) {
// JavaScript is enabled, so there is a JavaScript window object.
// Return an NPObject bound to the window object.
m_windowScriptNPObject = createScriptObject(m_frame);
Expand All @@ -406,7 +406,7 @@ NPObject* ScriptController::windowScriptNPObject()
NPObject* ScriptController::createScriptObjectForPluginElement(HTMLPlugInElement* plugin)
{
// Can't create NPObjects when JavaScript is disabled.
if (!canExecuteScripts())
if (!canExecuteScripts(NotAboutToExecuteScript))
return createNoScriptObject();

v8::HandleScope handleScope;
Expand Down
7 changes: 6 additions & 1 deletion WebCore/bindings/v8/ScriptController.h
Expand Up @@ -55,6 +55,11 @@ class String;
class Widget;
class XSSAuditor;

enum ReasonForCallingCanExecuteScripts {
AboutToExecuteScript,
NotAboutToExecuteScript
};

class ScriptController {
public:
ScriptController(Frame*);
Expand Down Expand Up @@ -113,7 +118,7 @@ class ScriptController {
// Check if the javascript engine has been initialized.
bool haveInterpreter() const;

bool canExecuteScripts();
bool canExecuteScripts(ReasonForCallingCanExecuteScripts);

// FIXME: void* is a compile hack.
void attachDebugger(void*);
Expand Down
4 changes: 2 additions & 2 deletions WebCore/bindings/v8/ScriptEventListener.cpp
Expand Up @@ -56,7 +56,7 @@ PassRefPtr<V8LazyEventListener> createAttributeEventListener(Node* node, Attribu

if (Frame* frame = node->document()->frame()) {
ScriptController* scriptController = frame->script();
if (!scriptController->canExecuteScripts())
if (!scriptController->canExecuteScripts(AboutToExecuteScript))
return 0;

if (!scriptController->xssAuditor()->canCreateInlineEventListener(attr->localName().string(), attr->value())) {
Expand Down Expand Up @@ -89,7 +89,7 @@ PassRefPtr<V8LazyEventListener> createAttributeEventListener(Frame* frame, Attri
String sourceURL;

ScriptController* scriptController = frame->script();
if (!scriptController->canExecuteScripts())
if (!scriptController->canExecuteScripts(AboutToExecuteScript))
return 0;

if (!scriptController->xssAuditor()->canCreateInlineEventListener(attr->localName().string(), attr->value())) {
Expand Down
2 changes: 1 addition & 1 deletion WebCore/bindings/v8/V8Proxy.cpp
Expand Up @@ -560,7 +560,7 @@ V8Proxy* V8Proxy::retrieve(Frame* frame)
{
if (!frame)
return 0;
return frame->script()->canExecuteScripts() ? frame->script()->proxy() : 0;
return frame->script()->canExecuteScripts(NotAboutToExecuteScript) ? frame->script()->proxy() : 0;
}

V8Proxy* V8Proxy::retrieve(ScriptExecutionContext* context)
Expand Down
2 changes: 1 addition & 1 deletion WebCore/dom/ScriptElement.cpp
Expand Up @@ -177,7 +177,7 @@ void ScriptElementData::evaluateScript(const ScriptSourceCode& sourceCode)
return;

if (Frame* frame = m_element->document()->frame()) {
if (!frame->script()->canExecuteScripts())
if (!frame->script()->canExecuteScripts(AboutToExecuteScript))
return;

m_evaluated = true;
Expand Down
2 changes: 1 addition & 1 deletion WebCore/dom/XMLTokenizerLibxml2.cpp
Expand Up @@ -786,7 +786,7 @@ void XMLTokenizer::startElementNs(const xmlChar* xmlLocalName, const xmlChar* xm
}

ScriptController* jsProxy = m_doc->frame() ? m_doc->frame()->script() : 0;
if (jsProxy && m_doc->frame()->script()->canExecuteScripts())
if (jsProxy && m_doc->frame()->script()->canExecuteScripts(NotAboutToExecuteScript))
jsProxy->setEventHandlerLineNumber(lineNumber());

handleElementAttributes(newElement.get(), libxmlAttributes, nb_attributes, ec, m_scriptingPermission);
Expand Down
2 changes: 1 addition & 1 deletion WebCore/html/HTMLMediaElement.cpp
Expand Up @@ -1194,7 +1194,7 @@ bool HTMLMediaElement::controls() const
Frame* frame = document()->frame();

// always show controls when scripting is disabled
if (frame && !frame->script()->canExecuteScripts())
if (frame && !frame->script()->canExecuteScripts(NotAboutToExecuteScript))
return true;

return hasAttribute(controlsAttr);
Expand Down
4 changes: 2 additions & 2 deletions WebCore/html/HTMLTokenizer.cpp
Expand Up @@ -1511,7 +1511,7 @@ HTMLTokenizer::State HTMLTokenizer::parseTag(SegmentedString& src, State state)
m_scriptTagSrcAttrValue = String();
m_scriptTagCharsetAttrValue = String();
if (m_currentToken.attrs && !m_fragment) {
if (m_doc->frame() && m_doc->frame()->script()->canExecuteScripts()) {
if (m_doc->frame() && m_doc->frame()->script()->canExecuteScripts(NotAboutToExecuteScript)) {
if ((a = m_currentToken.attrs->getAttributeItem(srcAttr)))
m_scriptTagSrcAttrValue = m_doc->completeURL(deprecatedParseURL(a->value())).string();
}
Expand Down Expand Up @@ -1921,7 +1921,7 @@ void HTMLTokenizer::finish()
PassRefPtr<Node> HTMLTokenizer::processToken()
{
ScriptController* scriptController = (!m_fragment && m_doc->frame()) ? m_doc->frame()->script() : 0;
if (scriptController && scriptController->canExecuteScripts())
if (scriptController && scriptController->canExecuteScripts(NotAboutToExecuteScript))
// FIXME: Why isn't this m_currentScriptTagStartLineNumber? I suspect this is wrong.
scriptController->setEventHandlerLineNumber(m_currentTagStartLineNumber + 1); // Script line numbers are 1 based.
if (m_dest > m_buffer) {
Expand Down
2 changes: 1 addition & 1 deletion WebCore/inspector/InspectorController.cpp
Expand Up @@ -230,7 +230,7 @@ static bool canPassNodeToJavaScript(Node* node)
if (!node)
return false;
Frame* frame = node->document()->frame();
return frame && frame->script()->canExecuteScripts();
return frame && frame->script()->canExecuteScripts(NotAboutToExecuteScript);
}

void InspectorController::inspect(Node* node)
Expand Down

0 comments on commit f3ca3bd

Please sign in to comment.