[Webkit-unassigned] [Bug 47239] Generate the messages sent to the WebPageProxy

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=47239


Adam Roben (aroben) <aroben at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #69877|review?                     |review+
               Flag|                            |




--- Comment #4 from Adam Roben (aroben) <aroben at apple.com>  2010-10-06 04:46:09 PST ---
(From update of attachment 69877)
View in context: https://bugs.webkit.org/attachment.cgi?id=69877&action=review

> WebKit2/ChangeLog:48
> +        * UIProcess/WebPageProxy.cpp:
> +        (WebKit::WebPageProxy::didReceiveMessage):
> +        (WebKit::WebPageProxy::didReceiveSyncMessage):
> +        (WebKit::WebPageProxy::didStartProvisionalLoadForFrame):
> +        (WebKit::WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame):
> +        (WebKit::WebPageProxy::didFailProvisionalLoadForFrame):
> +        (WebKit::WebPageProxy::didCommitLoadForFrame):
> +        (WebKit::WebPageProxy::didFinishDocumentLoadForFrame):
> +        (WebKit::WebPageProxy::didFinishLoadForFrame):
> +        (WebKit::WebPageProxy::didFailLoadForFrame):
> +        (WebKit::WebPageProxy::didReceiveTitleForFrame):
> +        (WebKit::WebPageProxy::didFirstLayoutForFrame):
> +        (WebKit::WebPageProxy::didFirstVisuallyNonEmptyLayoutForFrame):
> +        (WebKit::WebPageProxy::didRemoveFrameFromHierarchy):
> +        (WebKit::WebPageProxy::decidePolicyForNavigationAction):
> +        (WebKit::WebPageProxy::decidePolicyForNewWindowAction):

All this boilerplate isn't really helpful. Summarizing the kinds of changes made ("Updated to pass a message struct", "Added new files to the project", etc.) would be nice. You could remove the individual function names if they all have the same description.

> WebKit2/Scripts/webkit2/messages.py:160
>          'uint16_t',
>          'uint32_t',
>          'uint64_t',
> +        'int8_t',
> +        'int16_t',
> +        'int32_t',
> +        'int64_t',
>      ])

Maybe we should sort this list.

> WebKit2/Scripts/webkit2/messages.py:333
>      special_cases = {
>          'WTF::String': '"ArgumentCoders.h"',
> +        'WebKit::InjectedBundleUserMessageEncoder': '"InjectedBundleUserMessageCoders.h"',
>      }

This one, too.

> WebKit2/Scripts/webkit2/messages.py:403
> +    for message in receiver.messages:
> +        if message.reply_parameters is not None:
> +            for reply_parameter in message.reply_parameters:
> +                type = reply_parameter.type
> +                argument_encoder_headers = argument_coder_headers_for_type(type)
> +                if argument_encoder_headers:
> +                    headers.update(argument_encoder_headers)
> +                    continue
> +
> +                type_headers = headers_for_type(type)
> +                headers.update(type_headers)
> +

Looks like you could have used your iterreplyparameters function here. My guess is that you didn't because you needed the "is not None" check. But iterreplyparameters can do that! (reply_parameter for message in self.messages if message.reply_parameters is not None for reply_parameter in message.reply_parameters)

> WebKit2/Shared/StringPairVector.h:38
> +// This class is a hack to work around the fact that the IPC message generator
> +// cannot deal with class templates with more than one paramter.
> +class StringPairVector {
> +public:

:-(

Obviously we should make the generator smarter!

> WebKit2/UIProcess/WebPageProxy.cpp:624
> -void WebPageProxy::didFailProvisionalLoadForFrame(WebFrameProxy* frame, APIObject* userData)
> +void WebPageProxy::didFailProvisionalLoadForFrame(uint64_t frameID, CoreIPC::ArgumentDecoder* arguments)
>  {
> -    m_loaderClient.didFailProvisionalLoadWithErrorForFrame(this, frame, userData);
> +    RefPtr<APIObject> userData;
> +    WebContextUserMessageDecoder messageDecoder(userData, pageNamespace()->context());
> +    if (!arguments->decode(messageDecoder))
> +        return;

It would be nice to be able to get rid of all this repeated WebContextUserMessageDecoder code.

> WebKit2/UIProcess/WebPageProxy.cpp:744
> +void WebPageProxy::decidePolicyForNavigationAction(uint64_t frameID, uint32_t opaqueNavigationType, uint32_t opaqueModifiers, int32_t opaqueMouseButton, const String& url, uint64_t listenerID)
>  {
> +    WebFrameProxy* frame = process()->webFrame(frameID);
> +    NavigationType navigationType = static_cast<NavigationType>(opaqueNavigationType);
> +    WebEvent::Modifiers modifiers = static_cast<WebEvent::Modifiers>(opaqueModifiers);
> +    WebMouseEvent::Button mouseButton = static_cast<WebMouseEvent::Button>(opaqueMouseButton);

Maybe someday we can teach the generator how to convert the types for us.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.