Changeset 89001 in webkit

Timestamp:

Jun 15, 2011 9:10:36 PM (13 years ago)

Author:

abarth@webkit.org

Message:

2011-06-15 Adam Barth <​abarth@webkit.org>

Reviewed by Eric Seidel.

Remove ScriptController::setAllowPopupsFromPlugin
​https://bugs.webkit.org/show_bug.cgi?id=62706

• Plugins/Hosted/NetscapePluginInstanceProxy.mm: (WebKit::NetscapePluginInstanceProxy::evaluate):
• Plugins/WebNetscapePluginView.mm: (-[WebNetscapePluginView sendEvent:isDrawRect:]):

2011-06-15 Adam Barth <​abarth@webkit.org>

Reviewed by Eric Seidel.

Remove ScriptController::setAllowPopupsFromPlugin
​https://bugs.webkit.org/show_bug.cgi?id=62706

This API is just a poor man's UserGestureIndicator. We should use the
real deal.

• bindings/js/ScriptController.cpp: (WebCore::ScriptController::ScriptController): (WebCore::ScriptController::processingUserGesture):
• bindings/js/ScriptController.h:
• bindings/v8/NPV8Object.cpp: (_NPN_EvaluateHelper):
• bindings/v8/ScriptController.cpp: (WebCore::ScriptController::ScriptController): (WebCore::ScriptController::processingUserGesture):
• bindings/v8/ScriptController.h:

2011-06-15 Adam Barth <​abarth@webkit.org>

Reviewed by Eric Seidel.

Remove ScriptController::setAllowPopupsFromPlugin
​https://bugs.webkit.org/show_bug.cgi?id=62706

• WebProcess/Plugins/PluginView.cpp: (WebKit::PluginView::performJavaScriptURLRequest): (WebKit::PluginView::evaluate):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/bindings/js/ScriptController.cpp (modified) (2 diffs)
• WebCore/bindings/js/ScriptController.h (modified) (2 diffs)
• WebCore/bindings/v8/NPV8Object.cpp (modified) (2 diffs)
• WebCore/bindings/v8/ScriptController.cpp (modified) (2 diffs)
• WebCore/bindings/v8/ScriptController.h (modified) (2 diffs)
• WebKit/mac/ChangeLog (modified) (1 diff)
• WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm (modified) (2 diffs)
• WebKit/mac/Plugins/WebNetscapePluginView.mm (modified) (2 diffs)
• WebKit2/ChangeLog (modified) (1 diff)
• WebKit2/WebProcess/Plugins/PluginView.cpp (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-06-15  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        Remove ScriptController::setAllowPopupsFromPlugin
+        https://bugs.webkit.org/show_bug.cgi?id=62706
+
+        This API is just a poor man's UserGestureIndicator.  We should use the
+        real deal.
+
+        * bindings/js/ScriptController.cpp:
+        (WebCore::ScriptController::ScriptController):
+        (WebCore::ScriptController::processingUserGesture):
+        * bindings/js/ScriptController.h:
+        * bindings/v8/NPV8Object.cpp:
+        (_NPN_EvaluateHelper):
+        * bindings/v8/ScriptController.cpp:
+        (WebCore::ScriptController::ScriptController):
+        (WebCore::ScriptController::processingUserGesture):
+        * bindings/v8/ScriptController.h:
+
 2011-06-15  Adam Barth  <abarth@webkit.org>
 

trunk/Source/WebCore/bindings/js/ScriptController.cpp

@@ -66 +66 @@
     , m_processingTimerCallback(false)
     , m_paused(false)
-    , m_allowPopupsFromPlugin(false)
 #if ENABLE(NETSCAPE_PLUGIN_API)
     , m_windowScriptNPObject(0)
@@ -253 +252 @@
         return UserGestureIndicator::getUserGestureState() != DefinitelyNotProcessingUserGesture;
 
-    // FIXME: We check the plugin popup flag and javascript anchor navigation
-    // from the dynamic frame becuase they should only be initiated on the
-    // dynamic frame in which execution began if they do happen.
-    ScriptController* scriptController = frame->script();
-    ASSERT(scriptController);
-    if (scriptController->allowPopupsFromPlugin() || scriptController->isJavaScriptAnchorNavigation())
+    // FIXME: Remove the isJavaScriptAnchorNavigation check once https://bugs.webkit.org/show_bug.cgi?id=62702 is fixed.
+    if (frame->script()->isJavaScriptAnchorNavigation())
         return true;
 
     // If a DOM event is being processed, check that it was initiated by the user
     // and that it is in the whitelist of event types allowed to generate pop-ups.
-    if (JSDOMWindowShell* shell = scriptController->existingWindowShell(currentWorld(exec)))
+    if (JSDOMWindowShell* shell = frame->script()->existingWindowShell(currentWorld(exec)))
         if (Event* event = shell->window()->currentEvent())
             return event->fromUserGesture();

trunk/Source/WebCore/bindings/js/ScriptController.h

@@ -122 +122 @@
     bool isPaused() const { return m_paused; }
 
-    void setAllowPopupsFromPlugin(bool allowPopupsFromPlugin) { m_allowPopupsFromPlugin = allowPopupsFromPlugin; }
-    bool allowPopupsFromPlugin() const { return m_allowPopupsFromPlugin; }
-    
     const String* sourceURL() const { return m_sourceURL; } // 0 if we are not evaluating any script
 
@@ -183 +180 @@
     bool m_processingTimerCallback;
     bool m_paused;
-    bool m_allowPopupsFromPlugin;
 
     // The root object used for objects bound outside the context of a plugin, such

trunk/Source/WebCore/bindings/v8/NPV8Object.cpp

@@ -35 +35 @@
 #include "PlatformString.h"
 #include "ScriptSourceCode.h"
+#include "UserGestureIndicator.h"
 #include "V8GCController.h"
 #include "V8Helpers.h"
@@ -303 +304 @@
     ExceptionCatcher exceptionCatcher;
 
+    // FIXME: Is this branch still needed after switching to using UserGestureIndicator?
     String filename;
     if (!popupsAllowed)
         filename = "npscript";
 
-    // Set popupsAllowed flag to the current execution frame, so WebKit can get
-    // right gesture status for popups initiated from plugins.
-    Frame* frame = proxy->frame();
-    ASSERT(frame);
-    bool oldAllowPopups = frame->script()->allowPopupsFromPlugin();
-    frame->script()->setAllowPopupsFromPlugin(popupsAllowed);
-
     String script = String::fromUTF8(npScript->UTF8Characters, npScript->UTF8Length);
+
+    UserGestureIndicator gestureIndicator(popupsAllowed ? DefinitelyProcessingUserGesture : PossiblyProcessingUserGesture);
     v8::Local<v8::Value> v8result = proxy->evaluate(ScriptSourceCode(script, KURL(ParsedURLString, filename)), 0);
-    // Restore the old flag.
-    frame->script()->setAllowPopupsFromPlugin(oldAllowPopups);
 
     if (v8result.IsEmpty())

trunk/Source/WebCore/bindings/v8/ScriptController.cpp

@@ -112 +112 @@
     , m_processingTimerCallback(false)
     , m_paused(false)
-    , m_allowPopupsFromPlugin(false)
     , m_proxy(adoptPtr(new V8Proxy(frame)))
 #if ENABLE(NETSCAPE_PLUGIN_API)
@@ -156 +155 @@
 bool ScriptController::processingUserGesture()
 {
-    Frame* activeFrame = V8Proxy::retrieveFrameForEnteredContext();
-    // No script is running, so it is user-initiated unless the gesture stack
-    // explicitly says it is not.
-    if (!activeFrame)
+    Frame* firstFrame = V8Proxy::retrieveFrameForEnteredContext();
+    if (!firstFrame)
         return UserGestureIndicator::getUserGestureState() != DefinitelyNotProcessingUserGesture;
 
-    V8Proxy* activeProxy = activeFrame->script()->proxy();
-
-    v8::HandleScope handleScope;
-    v8::Handle<v8::Context> v8Context = V8Proxy::mainWorldContext(activeFrame);
-    // FIXME: find all cases context can be empty:
-    //  1) JS is disabled;
-    //  2) page is NULL;
+    v8::HandleScope handleScope;
+    v8::Handle<v8::Context> v8Context = V8Proxy::mainWorldContext(firstFrame);
     if (v8Context.IsEmpty())
         return true;
-
     v8::Context::Scope scope(v8Context);
-
     v8::Handle<v8::Object> global = v8Context->Global();
     v8::Handle<v8::String> eventSymbol = V8HiddenPropertyName::event();
     v8::Handle<v8::Value> jsEvent = global->GetHiddenValue(eventSymbol);
     Event* event = V8DOMWrapper::isValidDOMObject(jsEvent) ? V8Event::toNative(v8::Handle<v8::Object>::Cast(jsEvent)) : 0;
-
-    // Based on code from JSC's ScriptController::processingUserGesture.
-    // Note: This is more liberal than Firefox's implementation.
-    if (event) {
-        // Event::fromUserGesture will return false when UserGestureIndicator::processingUserGesture() returns false.
+    if (event)
         return event->fromUserGesture();
-    }
-    // FIXME: We check the javascript anchor navigation from the last entered
-    // frame becuase it should only be initiated on the last entered frame in
-    // which execution began if it does happen.    
-    const String* sourceURL = activeFrame->script()->sourceURL();
-    if (sourceURL && sourceURL->isNull() && !activeProxy->timerCallback()) {
-        // This is the <a href="javascript:window.open('...')> case -> we let it through.
+
+    // FIXME: Remove this check once https://bugs.webkit.org/show_bug.cgi?id=62702 is fixed.
+    const String* sourceURL = firstFrame->script()->sourceURL();
+    if (sourceURL && sourceURL->isNull() && !firstFrame->script()->proxy()->timerCallback())
         return true;
-    }
-    if (activeFrame->script()->allowPopupsFromPlugin())
-        return true;
-    // This is the <script>window.open(...)</script> case or a timer callback -> block it.
-    // Based on JSC version, use returned value of UserGestureIndicator::processingUserGesture for all other situations. 
+
     return UserGestureIndicator::processingUserGesture();
 }

trunk/Source/WebCore/bindings/v8/ScriptController.h

@@ -190 +190 @@
     static void getAllWorlds(Vector<DOMWrapperWorld*>& worlds);
 
-    void setAllowPopupsFromPlugin(bool allowPopupsFromPlugin) { m_allowPopupsFromPlugin = allowPopupsFromPlugin; }
-    bool allowPopupsFromPlugin() const { return m_allowPopupsFromPlugin; }
-
 private:
     Frame* m_frame;
@@ -201 +198 @@
     bool m_processingTimerCallback;
     bool m_paused;
-    bool m_allowPopupsFromPlugin;
 
     OwnPtr<V8Proxy> m_proxy;

trunk/Source/WebKit/mac/ChangeLog

@@ +1 @@
+2011-06-15  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        Remove ScriptController::setAllowPopupsFromPlugin
+        https://bugs.webkit.org/show_bug.cgi?id=62706
+
+        * Plugins/Hosted/NetscapePluginInstanceProxy.mm:
+        (WebKit::NetscapePluginInstanceProxy::evaluate):
+        * Plugins/WebNetscapePluginView.mm:
+        (-[WebNetscapePluginView sendEvent:isDrawRect:]):
+
 2011-06-15  David Kilzer  <ddkilzer@apple.com>
 

trunk/Source/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm

@@ -56 +56 @@
 #import <WebCore/ScriptValue.h>
 #import <WebCore/StringSourceProvider.h>
+#import <WebCore/UserGestureIndicator.h>
 #import <WebCore/npruntime_impl.h>
 #import <WebCore/runtime_object.h>
@@ -870 +871 @@
     ExecState* exec = globalObject->globalExec();
 
-    bool oldAllowPopups = frame->script()->allowPopupsFromPlugin();
-    frame->script()->setAllowPopupsFromPlugin(allowPopups);
-    
     globalObject->globalData().timeoutChecker.start();
+
+    UserGestureIndicator gestureIndicator(allowPopups ? DefinitelyProcessingUserGesture : PossiblyProcessingUserGesture);
     Completion completion = JSC::evaluate(exec, globalObject->globalScopeChain(), makeSource(script));
+
     globalObject->globalData().timeoutChecker.stop();
     ComplType type = completion.complType();
 
-    frame->script()->setAllowPopupsFromPlugin(oldAllowPopups);
-    
     JSValue result;
     if (type == Normal)

trunk/Source/WebKit/mac/Plugins/WebNetscapePluginView.mm

@@ -71 +71 @@
 #import <WebCore/SecurityOrigin.h>
 #import <WebCore/SoftLinking.h> 
+#import <WebCore/UserGestureIndicator.h>
 #import <WebCore/WebCoreObjCExtras.h>
 #import <WebCore/WebCoreURLResponse.h>
@@ -666 +667 @@
     // Set the pluginAllowPopup flag.
     ASSERT(_eventHandler);
-    bool oldAllowPopups = frame->script()->allowPopupsFromPlugin();
-    frame->script()->setAllowPopupsFromPlugin(_eventHandler->currentEventIsUserGesture());    
     {
         JSC::JSLock::DropAllLocks dropAllLocks(JSC::SilenceAssertionsOnly);
+        UserGestureIndicator gestureIndicator(_eventHandler->currentEventIsUserGesture() ? DefinitelyProcessingUserGesture : PossiblyProcessingUserGesture);
         acceptedEvent = [_pluginPackage.get() pluginFuncs]->event(plugin, event);
     }
-    // Restore the old pluginAllowPopup flag.
-    frame->script()->setAllowPopupsFromPlugin(oldAllowPopups);     
     [self didCallPlugInFunction];
-        
+
     if (portState) {
         if ([self currentWindow])

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2011-06-15  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        Remove ScriptController::setAllowPopupsFromPlugin
+        https://bugs.webkit.org/show_bug.cgi?id=62706
+
+        * WebProcess/Plugins/PluginView.cpp:
+        (WebKit::PluginView::performJavaScriptURLRequest):
+        (WebKit::PluginView::evaluate):
+
 2011-06-15  Ryuan Choi  <ryuan.choi@samsung.com>
 

trunk/Source/WebKit2/WebProcess/Plugins/PluginView.cpp

@@ -59 +59 @@
 #include <WebCore/ScrollView.h>
 #include <WebCore/Settings.h>
+#include <WebCore/UserGestureIndicator.h>
 
 using namespace JSC;
@@ -779 +780 @@
     // grab references to the plug-in here.
     RefPtr<Plugin> plugin = m_plugin;
-
-    bool oldAllowPopups = frame->script()->allowPopupsFromPlugin();
-    frame->script()->setAllowPopupsFromPlugin(request->allowPopups());
-    
-    ScriptValue result = frame->script()->executeScript(jsString);
-
-    frame->script()->setAllowPopupsFromPlugin(oldAllowPopups);
+    ScriptValue result = frame->script()->executeScript(jsString, request->allowPopups());
 
     // Check if evaluating the JavaScript destroyed the plug-in.
@@ -980 +975 @@
 bool PluginView::evaluate(NPObject* npObject, const String& scriptString, NPVariant* result, bool allowPopups)
 {
-    RefPtr<Frame> frame = m_pluginElement->document()->frame();
-    if (!frame)
+    // FIXME: Is this check necessary?
+    if (!m_pluginElement->document()->frame())
         return false;
-
-    bool oldAllowPopups = frame->script()->allowPopupsFromPlugin();
-    frame->script()->setAllowPopupsFromPlugin(allowPopups);
 
     // Calling evaluate will run JavaScript that can potentially remove the plug-in element, so we need to
@@ -991 +983 @@
     NPRuntimeObjectMap::PluginProtector pluginProtector(&m_npRuntimeObjectMap);
 
-    bool returnValue = m_npRuntimeObjectMap.evaluate(npObject, scriptString, result);
-
-    frame->script()->setAllowPopupsFromPlugin(oldAllowPopups);
-
-    return returnValue;
+    UserGestureIndicator gestureIndicator(allowPopups ? DefinitelyProcessingUserGesture : PossiblyProcessingUserGesture);
+    return m_npRuntimeObjectMap.evaluate(npObject, scriptString, result);
 }