<html>
    <head>
      <base href="https://bugs.webkit.org/" />
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [Coordinated Graphics] Improve scheduling of tasks between threads in CoordinatedGraphicsScene"
   href="https://bugs.webkit.org/show_bug.cgi?id=160238#c3">Comment # 3</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [Coordinated Graphics] Improve scheduling of tasks between threads in CoordinatedGraphicsScene"
   href="https://bugs.webkit.org/show_bug.cgi?id=160238">bug 160238</a>
              from <span class="vcard"><a class="email" href="mailto:cgarcia&#64;igalia.com" title="Carlos Garcia Campos &lt;cgarcia&#64;igalia.com&gt;"> <span class="fn">Carlos Garcia Campos</span></a>
</span></b>
        <pre>(In reply to <a href="show_bug.cgi?id=160238#c2">comment #2</a>)
<span class="quote">&gt; Comment on <span class=""><a href="attachment.cgi?id=284684&amp;action=diff" name="attach_284684" title="Patch">attachment 284684</a> <a href="attachment.cgi?id=284684&amp;action=edit" title="Patch">[details]</a></span>
&gt; Patch
&gt; 
&gt; View in context:
&gt; <a href="https://bugs.webkit.org/attachment.cgi?id=284684&amp;action=review">https://bugs.webkit.org/attachment.cgi?id=284684&amp;action=review</a>
&gt; 
&gt; Questions
&gt; 
&gt; &gt; Source/WebKit2/ChangeLog:12
&gt; &gt; +         - Use Function instead of std::function on dispatch methods.
&gt; 
&gt; Why, just to use our thing instead of the standard one? Is there an
&gt; advantage to WTF::Function?</span >

This is WTF::Function.

<span class="quote">&gt; &gt; Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:44
&gt; &gt; +    RunLoop::main().dispatch([protectedThis = makeRef(*this), function = WTFMove(function)] {
&gt; 
&gt; Why is this correct? We don't use |this| inside the lambda, so why the need
&gt; to keep it alive? If the ref is really needed then it should be handled
&gt; inside |function|, right?</span >

Just to ensure the scene is alive, that's why it's called protectedThis. Before this patch all caller do that themselves, and in some cases unnecessarily, because we only need it when scheduling to another thread. Now it's done here to simplify the callers.

<span class="quote">&gt; &gt; Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:56
&gt; &gt; +    m_clientRunLoop.dispatch([protectedThis = makeRef(*this), function = WTFMove(function)] {
&gt; 
&gt; Ditto.</span >

Ditto.

<span class="quote">&gt; &gt; Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:158
&gt; &gt; +    dispatchOnClientRunLoop([this] {
&gt; 
&gt; Why no protector?</span >

Because dispatchOnClientRunLoop is protecting this only when actually needed.

<span class="quote">&gt; &gt; Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:622
&gt; &gt; +    dispatchOnMainThread([this] {
&gt; 
&gt; Ditto.
&gt; 
&gt; &gt; Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:689
&gt; &gt; +    dispatchOnMainThread([this, layerID, offset] {
&gt; 
&gt; Ditto.</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>