[Webkit-unassigned] [Bug 150778] Add objC delegate callback for webProcessDidBecomeResponsive and webProcessDidBecomeUnresponsive.

[bugzilla-daemon at webkit.org]

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

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #264526|review?                     |review+
              Flags|                            |

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

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

> Source/WebKit2/UIProcess/API/APINavigationClient.h:87
>      // FIXME: This function should not be part of this client.
>      virtual void processDidCrash(WebKit::WebPageProxy&) { }
> +    virtual void processDidBecomeResponsive(WebKit::WebPageProxy&) { }
> +    virtual void processDidBecomeUnresponsive(WebKit::WebPageProxy&) { }

If the comment applies to all three of these, then please reword it so it says “These functions”.

Why can’t WebKit2 implement the delegate by implementing the existing loader client function? Why do we have to add this to the navigation client?

> Source/WebKit2/UIProcess/Cocoa/NavigationState.h:105
> +        virtual void processDidBecomeResponsive(WebKit::WebPageProxy&) override;
> +        virtual void processDidBecomeUnresponsive(WebKit::WebPageProxy&) override;

Please, no WebKit:: prefix here for the argument types for these new functions (match the function just above).

> Source/WebKit2/UIProcess/Cocoa/NavigationState.h:188
> +        bool webViewWebProcessDidBecomeResponsive : 1;
> +        bool webViewWebProcessDidBecomeUnresponsive : 1;

Not sure what order these booleans are in. It’s strange to have the new ones at the bottom after the QuickLook ones. Maybe we should keep these alphabetical instead of randomly ordered?

> Source/WebKit2/UIProcess/WebPageProxy.cpp:4863
> -    m_loaderClient->processDidBecomeUnresponsive(*this);
> +    if (m_navigationClient)
> +        m_navigationClient->processDidBecomeUnresponsive(*this);
> +    else
> +        m_loaderClient->processDidBecomeUnresponsive(*this);

Skipping the loader client function just because we have a navigation client doesn’t seem logical. Seems we should call both.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:4884
> -    m_loaderClient->processDidBecomeResponsive(*this);
> +    if (m_navigationClient)
> +        m_navigationClient->processDidBecomeResponsive(*this);
> +    else
> +        m_loaderClient->processDidBecomeResponsive(*this);

Ditto.

-- 
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/20151101/ea318447/attachment.html>