RESOLVED FIXED 72959
[Qt] [WK2] Move PagePolicyClient related code to QtWebPagePolicyClient
https://bugs.webkit.org/show_bug.cgi?id=72959
Summary [Qt] [WK2] Move PagePolicyClient related code to QtWebPagePolicyClient
Caio Marcelo de Oliveira Filho
Reported 2011-11-22 08:01:11 PST
[Qt] [WK2] Move PagePolicyClient related code to QtWebPagePolicyClient
Attachments
Patch (26.45 KB, patch)
2011-11-22 08:03 PST, Caio Marcelo de Oliveira Filho
kling: review+
Caio Marcelo de Oliveira Filho
Comment 1 2011-11-22 08:03:34 PST
Andreas Kling
Comment 2 2011-11-23 04:53:45 PST
Comment on attachment 116228 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=116228&action=review r=me with some nits that you don't need to fix in this patch, but may want to consider in the future. :) > Source/WebKit2/UIProcess/qt/QtWebPagePolicyClient.cpp:64 > +static Qt::MouseButton toQtMouseButton(WKEventMouseButton button) > +{ > + switch (button) { > + case kWKEventMouseButtonLeftButton: > + return Qt::LeftButton; > + case kWKEventMouseButtonMiddleButton: > + return Qt::MiddleButton; > + case kWKEventMouseButtonRightButton: > + return Qt::RightButton; > + } > + return Qt::NoButton; > +} In this kind of method, we should have a case for kWKEventMouseButtonNoButton (returning Qt::NoButton) and then putting an ASSERT_NOT_REACHED() after the switch. > Source/WebKit2/UIProcess/qt/QtWebPagePolicyClient.cpp:97 > + switch (action) { > + case Use: > + WKFramePolicyListenerUse(listener); > + break; > + case Download: > + WKFramePolicyListenerDownload(listener); > + break; > + case Ignore: > + WKFramePolicyListenerIgnore(listener); > + break; > + } Similar situation here, we could replace the breaks by returns and put an ASSERT_NOT_REACHED() after the switch.
Caio Marcelo de Oliveira Filho
Comment 3 2011-11-23 05:21:11 PST
Note You need to log in before you can comment on or make changes to this bug.