<html>
    <head>
      <base href="https://bugs.webkit.org/" />
    </head>
    <body><span class="vcard"><a class="email" href="mailto:darin&#64;apple.com" title="Darin Adler &lt;darin&#64;apple.com&gt;"> <span class="fn">Darin Adler</span></a>
</span> changed
              <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Add objC delegate callback for webProcessDidBecomeResponsive and webProcessDidBecomeUnresponsive."
   href="https://bugs.webkit.org/show_bug.cgi?id=150778">bug 150778</a>
        <br>
             <table border="1" cellspacing="0" cellpadding="8">
          <tr>
            <th>What</th>
            <th>Removed</th>
            <th>Added</th>
          </tr>

         <tr>
           <td style="text-align:right;">Attachment #264526 Flags</td>
           <td>review?
           </td>
           <td>review+
           </td>
         </tr></table>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Add objC delegate callback for webProcessDidBecomeResponsive and webProcessDidBecomeUnresponsive."
   href="https://bugs.webkit.org/show_bug.cgi?id=150778#c5">Comment # 5</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Add objC delegate callback for webProcessDidBecomeResponsive and webProcessDidBecomeUnresponsive."
   href="https://bugs.webkit.org/show_bug.cgi?id=150778">bug 150778</a>
              from <span class="vcard"><a class="email" href="mailto:darin&#64;apple.com" title="Darin Adler &lt;darin&#64;apple.com&gt;"> <span class="fn">Darin Adler</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=264526&amp;action=diff" name="attach_264526" title="Fix style issues.">attachment 264526</a> <a href="attachment.cgi?id=264526&amp;action=edit" title="Fix style issues.">[details]</a></span>
Fix style issues.

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=264526&amp;action=review">https://bugs.webkit.org/attachment.cgi?id=264526&amp;action=review</a>

<span class="quote">&gt; Source/WebKit2/UIProcess/API/APINavigationClient.h:87
&gt;      // FIXME: This function should not be part of this client.
&gt;      virtual void processDidCrash(WebKit::WebPageProxy&amp;) { }
&gt; +    virtual void processDidBecomeResponsive(WebKit::WebPageProxy&amp;) { }
&gt; +    virtual void processDidBecomeUnresponsive(WebKit::WebPageProxy&amp;) { }</span >

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?

<span class="quote">&gt; Source/WebKit2/UIProcess/Cocoa/NavigationState.h:105
&gt; +        virtual void processDidBecomeResponsive(WebKit::WebPageProxy&amp;) override;
&gt; +        virtual void processDidBecomeUnresponsive(WebKit::WebPageProxy&amp;) override;</span >

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

<span class="quote">&gt; Source/WebKit2/UIProcess/Cocoa/NavigationState.h:188
&gt; +        bool webViewWebProcessDidBecomeResponsive : 1;
&gt; +        bool webViewWebProcessDidBecomeUnresponsive : 1;</span >

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?

<span class="quote">&gt; Source/WebKit2/UIProcess/WebPageProxy.cpp:4863
&gt; -    m_loaderClient-&gt;processDidBecomeUnresponsive(*this);
&gt; +    if (m_navigationClient)
&gt; +        m_navigationClient-&gt;processDidBecomeUnresponsive(*this);
&gt; +    else
&gt; +        m_loaderClient-&gt;processDidBecomeUnresponsive(*this);</span >

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

<span class="quote">&gt; Source/WebKit2/UIProcess/WebPageProxy.cpp:4884
&gt; -    m_loaderClient-&gt;processDidBecomeResponsive(*this);
&gt; +    if (m_navigationClient)
&gt; +        m_navigationClient-&gt;processDidBecomeResponsive(*this);
&gt; +    else
&gt; +        m_loaderClient-&gt;processDidBecomeResponsive(*this);</span >

Ditto.</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>