Changeset 89007 in webkit

Timestamp:

Jun 15, 2011 11:15:29 PM (13 years ago)

Author:

abarth@webkit.org

Message:

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

Reviewed by Eric Seidel.

Remove Event::fromUserGesture
​https://bugs.webkit.org/show_bug.cgi?id=62778

This function is a remnant from the old user-gesture design. The list
of events here is redundant with our selection of call sites for
setting the user gesture indicator.

As part of this patch, I've also cleaned up the implementation of
UserGestureIndicator itself to always be definite about whether we're
processing a user gesture. We now start out in a definite state (no
user gesture) and inductively state in a definite state.

• WebCore.exp.in:
• bindings/js/ScriptController.cpp: (WebCore::ScriptController::processingUserGesture):
• bindings/v8/ScriptController.cpp: (WebCore::ScriptController::processingUserGesture):
• dom/Event.cpp:
• dom/Event.h:
• dom/UserGestureIndicator.cpp: (WebCore::isDefinite): (WebCore::UserGestureIndicator::UserGestureIndicator): (WebCore::UserGestureIndicator::~UserGestureIndicator):
• dom/UserGestureIndicator.h: (WebCore::UserGestureIndicator::processingUserGesture):
• html/MediaDocument.cpp: (WebCore::MediaDocument::defaultEventHandler):
• html/shadow/MediaControlElements.cpp: (WebCore::MediaControlSeekButtonElement::defaultEventHandler):
• html/shadow/TextControlInnerElements.cpp: (WebCore::InputFieldSpeechButtonElement::defaultEventHandler):

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

Reviewed by Eric Seidel.

Remove Event::fromUserGesture
​https://bugs.webkit.org/show_bug.cgi?id=62778

This call site is not definite about there not being a user gesture.

• src/WebPluginContainerImpl.cpp: (WebKit::WebPluginContainerImpl::loadFrameRequest):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/WebCore.exp.in (modified) (1 diff)
• WebCore/bindings/js/ScriptController.cpp (modified) (1 diff)
• WebCore/bindings/v8/ScriptController.cpp (modified) (1 diff)
• WebCore/dom/Event.cpp (modified) (1 diff)
• WebCore/dom/Event.h (modified) (1 diff)
• WebCore/dom/UserGestureIndicator.cpp (modified) (1 diff)
• WebCore/dom/UserGestureIndicator.h (modified) (2 diffs)
• WebCore/html/MediaDocument.cpp (modified) (3 diffs)
• WebCore/html/shadow/MediaControlElements.cpp (modified) (2 diffs)
• WebCore/html/shadow/TextControlInnerElements.cpp (modified) (2 diffs)
• WebKit/chromium/ChangeLog (modified) (1 diff)
• WebKit/chromium/src/WebPluginContainerImpl.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-06-15  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        Remove Event::fromUserGesture
+        https://bugs.webkit.org/show_bug.cgi?id=62778
+
+        This function is a remnant from the old user-gesture design.  The list
+        of events here is redundant with our selection of call sites for
+        setting the user gesture indicator.
+
+        As part of this patch, I've also cleaned up the implementation of
+        UserGestureIndicator itself to always be definite about whether we're
+        processing a user gesture.  We now start out in a definite state (no
+        user gesture) and inductively state in a definite state.
+
+        * WebCore.exp.in:
+        * bindings/js/ScriptController.cpp:
+        (WebCore::ScriptController::processingUserGesture):
+        * bindings/v8/ScriptController.cpp:
+        (WebCore::ScriptController::processingUserGesture):
+        * dom/Event.cpp:
+        * dom/Event.h:
+        * dom/UserGestureIndicator.cpp:
+        (WebCore::isDefinite):
+        (WebCore::UserGestureIndicator::UserGestureIndicator):
+        (WebCore::UserGestureIndicator::~UserGestureIndicator):
+        * dom/UserGestureIndicator.h:
+        (WebCore::UserGestureIndicator::processingUserGesture):
+        * html/MediaDocument.cpp:
+        (WebCore::MediaDocument::defaultEventHandler):
+        * html/shadow/MediaControlElements.cpp:
+        (WebCore::MediaControlSeekButtonElement::defaultEventHandler):
+        * html/shadow/TextControlInnerElements.cpp:
+        (WebCore::InputFieldSpeechButtonElement::defaultEventHandler):
+
 2011-06-08  Keishi Hattori  <keishi@webkit.org>
 

trunk/Source/WebCore/WebCore.exp.in

@@ -543 +543 @@
 __ZN7WebCore20ResourceResponseBaseC2Ev
 __ZN7WebCore20SpaceSplitStringData12createVectorEv
-__ZN7WebCore20UserGestureIndicator23s_processingUserGestureE
+__ZN7WebCore20UserGestureIndicator7s_stateE
 __ZN7WebCore20UserGestureIndicatorC1ENS_26ProcessingUserGestureStateE
 __ZN7WebCore20UserGestureIndicatorD1Ev

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

@@ -244 +244 @@
 bool ScriptController::processingUserGesture()
 {
-    ExecState* exec = JSMainThreadExecState::currentState();
-    Frame* frame = exec ? toDynamicFrame(exec) : 0;
-    // No script is running, so it is user-initiated unless the gesture stack
-    // explicitly says it is not.
-    if (!frame)
-        return UserGestureIndicator::getUserGestureState() != DefinitelyNotProcessingUserGesture;
-
-    // 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 = frame->script()->existingWindowShell(currentWorld(exec)))
-        if (Event* event = shell->window()->currentEvent())
-            return event->fromUserGesture();
-
     return UserGestureIndicator::processingUserGesture();
 }

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

@@ -154 +154 @@
 bool ScriptController::processingUserGesture()
 {
-    Frame* firstFrame = V8Proxy::retrieveFrameForEnteredContext();
-    if (!firstFrame)
-        return UserGestureIndicator::getUserGestureState() != DefinitelyNotProcessingUserGesture;
-
-    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;
-    if (event)
-        return event->fromUserGesture();
-
     return UserGestureIndicator::processingUserGesture();
 }

trunk/Source/WebCore/dom/Event.cpp

@@ -258 +258 @@
 #endif
 
-bool Event::fromUserGesture()
-{
-    if (!UserGestureIndicator::processingUserGesture())
-        return false;
-
-    const AtomicString& type = this->type();
-    return
-        // mouse events
-        type == eventNames().clickEvent || type == eventNames().mousedownEvent 
-        || type == eventNames().mouseupEvent || type == eventNames().dblclickEvent 
-        // keyboard events
-        || type == eventNames().keydownEvent || type == eventNames().keypressEvent
-        || type == eventNames().keyupEvent
-#if ENABLE(TOUCH_EVENTS)
-        // touch events
-        || type == eventNames().touchstartEvent || type == eventNames().touchmoveEvent
-        || type == eventNames().touchendEvent || type == eventNames().touchcancelEvent
-#endif
-        // other accepted events
-        || type == eventNames().selectEvent || type == eventNames().changeEvent
-        || type == eventNames().focusEvent || type == eventNames().blurEvent
-        || type == eventNames().submitEvent;
-}
-
 bool Event::storesResultAsString() const
 {

trunk/Source/WebCore/dom/Event.h

@@ -151 +151 @@
         virtual bool isStreamEvent() const;
 #endif
-        bool fromUserGesture();
-        
         bool propagationStopped() const { return m_propagationStopped || m_immediatePropagationStopped; }
         bool immediatePropagationStopped() const { return m_immediatePropagationStopped; }

trunk/Source/WebCore/dom/UserGestureIndicator.cpp

@@ -29 +29 @@
 namespace WebCore {
 
-ProcessingUserGestureState UserGestureIndicator::s_processingUserGesture = PossiblyProcessingUserGesture;
+static bool isDefinite(ProcessingUserGestureState state)
+{
+    return state == DefinitelyProcessingUserGesture || state == DefinitelyNotProcessingUserGesture;
+}
+
+ProcessingUserGestureState UserGestureIndicator::s_state = DefinitelyNotProcessingUserGesture;
 
 UserGestureIndicator::UserGestureIndicator(ProcessingUserGestureState state)
-    : m_previousValue(s_processingUserGesture)
+    : m_previousState(s_state)
 {
-    s_processingUserGesture = state;
+    // We overwrite s_state only if the caller is definite about the gesture state.
+    if (isDefinite(state))
+        s_state = state;
+    ASSERT(isDefinite(s_state));
 }
 
 UserGestureIndicator::~UserGestureIndicator()
 {
-    s_processingUserGesture = m_previousValue;
+    s_state = m_previousState;
+    ASSERT(isDefinite(s_state));
 }
 
-} // namespace WebCore
+}

trunk/Source/WebCore/dom/UserGestureIndicator.h

@@ -40 +40 @@
     WTF_MAKE_NONCOPYABLE(UserGestureIndicator);
 public:
-    static bool processingUserGesture() { return s_processingUserGesture == DefinitelyProcessingUserGesture; }
-    static ProcessingUserGestureState getUserGestureState() { return s_processingUserGesture; }
+    static bool processingUserGesture() { return s_state == DefinitelyProcessingUserGesture; }
 
     explicit UserGestureIndicator(ProcessingUserGestureState);
@@ -47 +46 @@
 
 private:
-    static ProcessingUserGestureState s_processingUserGesture;
-    ProcessingUserGestureState m_previousValue;
-};    
+    static ProcessingUserGestureState s_state;
+    ProcessingUserGestureState m_previousState;
+};
 
-} // namespace WebCore
+}
 
-#endif // UserGestureIndicator_h
+#endif

trunk/Source/WebCore/html/MediaDocument.cpp

@@ -41 +41 @@
 #include "NodeList.h"
 #include "RawDataDocumentParser.h"
+#include "ScriptController.h"
 
 namespace WebCore {
@@ -165 +166 @@
         if (event->type() == eventNames().clickEvent) {
             if (!video->canPlay()) {
-                video->pause(event->fromUserGesture());
+                video->pause(ScriptController::processingUserGesture());
                 event->setDefaultHandled();
             }
         } else if (event->type() == eventNames().dblclickEvent) {
             if (video->canPlay()) {
-                video->play(event->fromUserGesture());
+                video->play(ScriptController::processingUserGesture());
                 event->setDefaultHandled();
             }
@@ -185 +186 @@
             if (video->paused()) {
                 if (video->canPlay())
-                    video->play(event->fromUserGesture());
+                    video->play(ScriptController::processingUserGesture());
             } else
-                video->pause(event->fromUserGesture());
+                video->pause(ScriptController::processingUserGesture());
             event->setDefaultHandled();
         }

trunk/Source/WebCore/html/shadow/MediaControlElements.cpp

@@ -48 +48 @@
 #include "RenderTheme.h"
 #include "RenderView.h"
+#include "ScriptController.h"
 #include "Settings.h"
 
@@ -548 +549 @@
             frame->eventHandler()->setCapturingMouseEventsNode(this);
         }
-        mediaElement()->pause(event->fromUserGesture());
+        mediaElement()->pause(ScriptController::processingUserGesture());
         m_seekTimer.startRepeating(cSeekRepeatDelay);
         event->setDefaultHandled();

trunk/Source/WebCore/html/shadow/TextControlInnerElements.cpp

@@ -40 +40 @@
 #include "RenderLayer.h"
 #include "RenderTextControlSingleLine.h"
+#include "ScriptController.h"
 #include "ScrollbarTheme.h"
 #include "SpeechInput.h"
@@ -389 +390 @@
 {
     // For privacy reasons, only allow clicks directly coming from the user.
-    if (!event->fromUserGesture()) {
+    if (!ScriptController::processingUserGesture()) {
         HTMLDivElement::defaultEventHandler(event);
         return;

trunk/Source/WebKit/chromium/ChangeLog

@@ +1 @@
+2011-06-15  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        Remove Event::fromUserGesture
+        https://bugs.webkit.org/show_bug.cgi?id=62778
+
+        This call site is not definite about there not being a user gesture.
+
+        * src/WebPluginContainerImpl.cpp:
+        (WebKit::WebPluginContainerImpl::loadFrameRequest):
+
 2011-06-15  Darin Adler  <darin@apple.com>
 

trunk/Source/WebKit/chromium/src/WebPluginContainerImpl.cpp

@@ -366 +366 @@
 }
 
-void WebPluginContainerImpl::loadFrameRequest(
-    const WebURLRequest& request, const WebString& target, bool notifyNeeded, void* notifyData)
+void WebPluginContainerImpl::loadFrameRequest(const WebURLRequest& request, const WebString& target, bool notifyNeeded, void* notifyData)
 {
     Frame* frame = m_element->document()->frame();
@@ -383 +382 @@
     }
 
-    FrameLoadRequest frameRequest(frame->document()->securityOrigin(),
-        request.toResourceRequest(), target);
-
-    UserGestureIndicator gestureIndicator(request.hasUserGesture() ?
-        DefinitelyProcessingUserGesture : DefinitelyNotProcessingUserGesture);
-
-    frame->loader()->loadFrameRequest(
-        frameRequest,
-        false,  // lock history
-        false,  // lock back forward list
-        0,      // event
-        0,     // form state
-        SendReferrer);
+    FrameLoadRequest frameRequest(frame->document()->securityOrigin(), request.toResourceRequest(), target);
+    UserGestureIndicator gestureIndicator(request.hasUserGesture() ? DefinitelyProcessingUserGesture : PossiblyProcessingUserGesture);
+    frame->loader()->loadFrameRequest(frameRequest, false, false, 0, 0, SendReferrer);
 }