RESOLVED FIXED 44280
Add modifier key info to policy client functions in WebKit2 
https://bugs.webkit.org/show_bug.cgi?id=44280
Summary Add modifier key info to policy client functions in WebKit2
Sam Weinig
Reported 2010-08-19 11:40:00 PDT
Add modifier key info to policy client functions in WebKit2.
Attachments
Patch (16.12 KB, patch)
2010-08-19 11:51 PDT, Sam Weinig 		aroben: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Sam Weinig
Comment 1 2010-08-19 11:51:06 PDT
Created attachment 64883 [details] Patch
Adam Roben (:aroben)
Comment 2 2010-08-19 11:56:56 PDT
Comment on attachment 64883 [details] Patch > @@ -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::DecidePolicyForNewWindowAction, 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(FramePolicyFunction 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::DecidePolicyForNavigationAction, 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
Sam Weinig
Comment 3 2010-08-19 12:02:05 PDT
(In reply to comment #2) > (From update of attachment 64883 [details]) > > @@ -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? > No. We need to give an explicit size when encoding/decoding. This will all be better with your typed IPC.
Sam Weinig
Comment 4 2010-08-19 14:28:09 PDT
Landed in r65699.
Note You need to log in before you can comment on or make changes to this bug.