[Webkit-unassigned] [Bug 167022] Ignore Connection Assertion if we are not using connection to send messages

[bugzilla-daemon at webkit.org]

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

--- Comment #5 from Darin Adler <darin at apple.com> ---
Comment on attachment 298803
  --> https://bugs.webkit.org/attachment.cgi?id=298803
Patch

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

> Source/WebKit2/UIProcess/ChildProcessProxy.h:55
> -    IPC::Connection* connection() const
> +    enum class IgnoreAssertion { No, Yes };
> +    IPC::Connection* connection(IgnoreAssertion ignoreAssertion = IgnoreAssertion::No) const
>      {
> -        ASSERT(m_connection);
> +        if (ignoreAssertion == IgnoreAssertion::No)
> +            ASSERT(m_connection);
>          return m_connection.get();
>      }

I don’t see any good reason to use a single function with an argument for this. It would be more straightforward to have two separate functions for these two separate purposes.

One good approach would be to have a function with a boolean return value that takes a Connection& and answers whether it is this process proxy’s connection; maybe named “hasThisConnection” or some other phrase like that. Longer term, the one named "connection" that gets you the connection should be changed to return a reference, not a pointer, to express that it returns something that is never null. And of course would continue to assert that it is non-null.

> Source/WebKit2/UIProcess/Cocoa/WebPasteboardProxyCocoa.mm:29
>  #import "config.h"
> +#import "ChildProcessProxy.h"
>  #import "WebPasteboardProxy.h"
>  #import "WebProcessProxy.h"

The formatting here is unconventional for WebKit. The only header that should be next to "config.h" without a blank line after it would be the header for this implementation file. If this file does not have its own, then there should be a blank line after "config.h".

This include should not be added. I don’t see any reason we need to include it. We are calling the connection function, so clearly we are including the header that contains the function definition. So there is no additional need to include a header to get the type that is defined next to the function.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170115/20abd323/attachment.html>