[prev in list] [next in list] [prev in thread] [next in thread] 

List:       webkit-unassigned
Subject:    [Webkit-unassigned] [Bug 199348] Use separate variables for moving and stationary scrolling relation
From:       bugzilla-daemon () webkit ! org
Date:       2019-06-30 20:04:05
Message-ID: bug-199348-2851-pnp9Scsr6B () https ! bugs ! webkit ! org/
[Download RAW message or body]

--1561925050.e0D1e3d14.27270
Date: Sun, 30 Jun 2019 13:04:10 -0700
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Bugzilla-URL: https://bugs.webkit.org/
Auto-Submitted: auto-generated

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

Darin Adler <darin@apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |darin@apple.com
 Attachment #373195|review?                     |review+
              Flags|                            |

--- Comment #3 from Darin Adler <darin@apple.com> ---
Comment on attachment 373195
  --> https://bugs.webkit.org/attachment.cgi?id=373195
patch

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

While I'm not a real expert on this code, this looks good to me.

> Source/WebCore/page/scrolling/ScrollingTree.h:187
> +    HashSet<RefPtr<ScrollingTreeOverflowScrollProxyNode>> \
> m_activeOverflowScrollProxyNodes; +    HashSet<RefPtr<ScrollingTreePositionedNode>> \
> m_activePositionedNodes;

A number of places elsewhere use HashSet<Ref<>> rather than HashSet<RefPtr<>>. Is \
there a reason to prefer RefPtr here?

> Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:134
> +                return (WKChildScrollView *)actingParent->uiView();

Not new, but: Would be nice to have a type checking cast like downcast<> here for \
clarity/assertion. Also unclear to me how solid the guarantee behind this typecast \
is.

-- 
You are receiving this mail because:
You are the assignee for the bug.
--1561925050.e0D1e3d14.27270
Date: Sun, 30 Jun 2019 13:04:10 -0700
MIME-Version: 1.0
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Bugzilla-URL: https://bugs.webkit.org/
Auto-Submitted: auto-generated

<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 - Use separate variables for moving and stationary scrolling \
relationships in RemoteLayerTreeNode"  \
href="https://bugs.webkit.org/show_bug.cgi?id=199348">bug 199348</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;">CC</td>
           <td>
               &nbsp;
           </td>
           <td>darin&#64;apple.com
           </td>
         </tr>

         <tr>
           <td style="text-align:right;">Attachment #373195 Flags</td>
           <td>review?
           </td>
           <td>review+
           </td>
         </tr></table>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Use separate variables for moving and stationary scrolling \
relationships in RemoteLayerTreeNode"  \
href="https://bugs.webkit.org/show_bug.cgi?id=199348#c3">Comment # 3</a>  on <a \
class="bz_bug_link   bz_status_NEW "
   title="NEW - Use separate variables for moving and stationary scrolling \
relationships in RemoteLayerTreeNode"  \
href="https://bugs.webkit.org/show_bug.cgi?id=199348">bug 199348</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=373195&amp;action=diff" name="attach_373195" \
title="patch">attachment 373195</a> <a \
href="attachment.cgi?id=373195&amp;action=edit" title="patch">[details]</a></span> \
patch

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


While I'm not a real expert on this code, this looks good to me.

<span class="quote">&gt; Source/WebCore/page/scrolling/ScrollingTree.h:187
&gt; +    HashSet&lt;RefPtr&lt;ScrollingTreeOverflowScrollProxyNode&gt;&gt; \
m_activeOverflowScrollProxyNodes; &gt; +    \
HashSet&lt;RefPtr&lt;ScrollingTreePositionedNode&gt;&gt; \
m_activePositionedNodes;</span >

A number of places elsewhere use HashSet&lt;Ref&lt;&gt;&gt; rather than \
HashSet&lt;RefPtr&lt;&gt;&gt;. Is there a reason to prefer RefPtr here?

<span class="quote">&gt; \
Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:134 &gt; +        \
return (WKChildScrollView *)actingParent-&gt;uiView();</span >

Not new, but: Would be nice to have a type checking cast like downcast&lt;&gt; here \
for clarity/assertion. Also unclear to me how solid the guarantee behind this \
typecast is.</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>
--1561925050.e0D1e3d14.27270--


[Attachment #3 (text/plain)]

_______________________________________________
webkit-unassigned mailing list
webkit-unassigned@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-unassigned


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic