[webkit-reviews] review granted: [Bug 44280] Add modifier key info to policy client functions in WebKit2 : [Attachment 64883] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Sam Weinig
<sam at webkit.org>'s request for review:
Bug 44280: Add modifier key info to policy client functions in WebKit2
https://bugs.webkit.org/show_bug.cgi?id=44280

Attachment 64883: Patch
https://bugs.webkit.org/attachment.cgi?id=64883&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
> @@ -521,21 +521,23 @@ void WebPageProxy::didReceiveMessage(Cor
>	   case WebPageProxyMessage::DecidePolicyForNavigationAction: {
>	       uint64_t frameID;
>	       uint32_t navigationType;
> +	       uint32_t modifiers;
>	       String url;
>	       uint64_t listenerID;
> -	       if (!arguments->decode(CoreIPC::Out(frameID, navigationType,
url, listenerID)))
> +	       if (!arguments->decode(CoreIPC::Out(frameID, navigationType,
modifiers, url, listenerID)))
>		   return;
> -	       decidePolicyForNavigationAction(webFrame(frameID),
static_cast<NavigationType>(navigationType), url, listenerID);
> +	       decidePolicyForNavigationAction(webFrame(frameID),
static_cast<NavigationType>(navigationType),
static_cast<WebEvent::Modifiers>(modifiers), url, listenerID);
>	       break;
>	   }
>	   case WebPageProxyMessage::DecidePolicyForNewWindowAction: {
>	       uint64_t frameID;
>	       uint32_t navigationType;
> +	       uint32_t modifiers;

Can we use WebEvent::Modifiers here?


> +static uint32_t modifiersForNavigationAction(const NavigationAction&
navigationAction)
> +{
> +    uint32_t modifiers = 0;
> +    if (const UIEventWithKeyState* keyStateEvent =
findEventWithKeyState(const_cast<Event*>(navigationAction.event()))) {
> +	   if (keyStateEvent->shiftKey())
> +	       modifiers |= WebEvent::ShiftKey;
> +	   if (keyStateEvent->ctrlKey())
> +	       modifiers |= WebEvent::ControlKey;
> +	   if (keyStateEvent->altKey())
> +	       modifiers |= WebEvent::AltKey;
> +	   if (keyStateEvent->metaKey())
> +	       modifiers |= WebEvent::MetaKey;
> +    }
> +
> +    return modifiers;
> +}

And here?

>  void
WebFrameLoaderClient::dispatchDecidePolicyForMIMEType(FramePolicyFunction
function, const String& MIMEType, const ResourceRequest& request)
>  {
>      WebPage* webPage = m_frame->page();
> @@ -436,8 +455,10 @@ void WebFrameLoaderClient::dispatchDecid
>      // FIXME: Pass the frame name.
>      const String& url = request.url().string(); // FIXME: Pass entire
request.
>  
> +    uint32_t modifiers = modifiersForNavigationAction(navigationAction);
> +
>     
WebProcess::shared().connection()->send(WebPageProxyMessage::DecidePolicyForNew
WindowAction, webPage->pageID(),
> -					       CoreIPC::In(m_frame->frameID(),
(uint32_t)navigationAction.type(), url, listenerID));
> +					       CoreIPC::In(m_frame->frameID(),
(uint32_t)navigationAction.type(), modifiers, url, listenerID));
>  }

And here?

>  void
WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(FramePolicyFuncti
on function, const NavigationAction& navigationAction, const ResourceRequest&
request, PassRefPtr<FormState>)
> @@ -451,8 +472,10 @@ void WebFrameLoaderClient::dispatchDecid
>      // FIXME: Pass more than just the navigation action type.
>      const String& url = request.url().string(); // FIXME: Pass entire
request.
>  
> +    uint32_t modifiers = modifiersForNavigationAction(navigationAction);
> +
>     
WebProcess::shared().connection()->send(WebPageProxyMessage::DecidePolicyForNav
igationAction, webPage->pageID(),
> -					       CoreIPC::In(m_frame->frameID(),
(uint32_t)navigationAction.type(), url, listenerID));
> +					       CoreIPC::In(m_frame->frameID(),
(uint32_t)navigationAction.type(), modifiers, url, listenerID));
>  }

And here?

r=me