Bug 132033 – [iOS][WK2] Split iOS touch event dispatch for the regular touch event dispatch

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Benjamin Poulain]

[iOS][WK2] Split iOS touch event dispatch for the regular touch event dispatch

[Benjamin Poulain]

Created attachment 229936 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 229936 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=229936&action=review

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1515
> +    m_process->responsivenessTimer()->start();
> +    bool handled = false;
> +    m_process->sendSync(Messages::WebPage::TouchEventSync(event), Messages::WebPage::TouchEventSync::Reply(handled), m_pageID);
> +    didReceiveEvent(event.type(), handled);
> +    m_pageClient.doneWithTouchEvent(event, handled);
> +    m_process->responsivenessTimer()->stop();

Would be nice to have a RAII class for start/stop.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1531
> +    m_process->send(Messages::EventDispatcher::TouchEvent(m_pageID, event), 0);

What is the last param?

> Source/WebKit2/UIProcess/WebPageProxy.h:698
> +    void handleSynchronousTouchEvent(const NativeWebTouchEvent&);

handleTouchEventSynchronously?

> Source/WebKit2/UIProcess/WebPageProxy.h:699
> +    void sendAsynchronousTouchEvent(const NativeWebTouchEvent&);

handleTouchEventAsynchronously?

> Source/WebKit2/WebProcess/WebPage/EventDispatcher.cpp:177
> +            TouchEventQueue& queuedEvents = addResult.iterator->value;
> +            ASSERT(!queuedEvents.isEmpty());
> +            const WebTouchEvent& lastTouchEvent = queuedEvents.last();
> +            WebEvent::Type type = lastTouchEvent.type();
> +            if (type == WebEvent::TouchMove)
> +                queuedEvents.last() = touchEvent;

Not quite sure what you are doing here. Coalescing move events? Can you add a comment or name the variables to clarify?

> Source/WebKit2/WebProcess/WebPage/EventDispatcher.h:77
> +    void touchEvent(uint64_t pageID, const WebTouchEvent&);

I wish we had a typedef for pageID

> Source/WebKit2/WebProcess/WebPage/EventDispatcher.h:100
> +    // FIXME: verify what is the right value for the inline capacity.

Do this?

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2062
> +void WebPage::dispatchAsynchronousTouchEventsQueue(const Vector<WebTouchEvent, 5>& queue)

You don't dispatch a queue, so dispatchAsynchronousTouchEventsInQueue or dispatchAsynchronousTouchEvents.

[Benjamin Poulain]

Created attachment 229950 [details]
Patch

[Benjamin Poulain]

Thanks for the review!

> > Source/WebKit2/UIProcess/WebPageProxy.cpp:1531
> > +    m_process->send(Messages::EventDispatcher::TouchEvent(m_pageID, event), 0);
> 
> What is the last param?

The target, but in this case it is on a different queue.

> > Source/WebKit2/WebProcess/WebPage/EventDispatcher.h:100
> > +    // FIXME: verify what is the right value for the inline capacity.
> 
> Do this?

Oops :)

[Benjamin Poulain]

Comment on attachment 229950 [details]
Patch

Clearing flags on attachment: 229950

Committed r167698: <http://trac.webkit.org/changeset/167698>

[Benjamin Poulain]

All reviewed patches have been landed.  Closing bug.