<html>
    <head>
      <base href="https://bugs.webkit.org/" />
    </head>
    <body><span class="vcard"><a class="email" href="mailto:mcatanzaro&#64;igalia.com" title="Michael Catanzaro &lt;mcatanzaro&#64;igalia.com&gt;"> <span class="fn">Michael Catanzaro</span></a>
</span> changed
              <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [GTK] Rework scrollbars theming code for GTK+ 3.20"
   href="https://bugs.webkit.org/show_bug.cgi?id=156462">bug 156462</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 #276138 Flags</td>
           <td>review?
           </td>
           <td>review+
           </td>
         </tr></table>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [GTK] Rework scrollbars theming code for GTK+ 3.20"
   href="https://bugs.webkit.org/show_bug.cgi?id=156462#c2">Comment # 2</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [GTK] Rework scrollbars theming code for GTK+ 3.20"
   href="https://bugs.webkit.org/show_bug.cgi?id=156462">bug 156462</a>
              from <span class="vcard"><a class="email" href="mailto:mcatanzaro&#64;igalia.com" title="Michael Catanzaro &lt;mcatanzaro&#64;igalia.com&gt;"> <span class="fn">Michael Catanzaro</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=276138&amp;action=diff" name="attach_276138" title="Patch">attachment 276138</a> <a href="attachment.cgi?id=276138&amp;action=edit" title="Patch">[details]</a></span>
Patch

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

<span class="quote">&gt; Source/WebCore/ChangeLog:9
&gt; +        needed for scrollbars, this patch uses the RenderthemGadget classes introduced in r199292 to render the native</span >

Renderthem!

<span class="quote">&gt; Source/WebCore/platform/gtk/RenderThemeGadget.cpp:70
&gt; +    // Scrollbars need to use its GType to be able to get non-CSS style properties.</span >

Wow, I thought the type was ignored....

<span class="quote">&gt; Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:86
&gt; +static GRefPtr&lt;GtkStyleContext&gt; createChildStyleContext(GtkStyleContext* parent, const char* name)</span >

I'd use className instead of name to make it more clear what that parameter is for.

<span class="quote">&gt; Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:157
&gt; +        FALLTHROUGH;</span >

Didn't know we had this. Cool.

<span class="quote">&gt; Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:309
&gt; +    // painting the thumb can be skipped. We don't have to be exact here.</span >

I would name the method something like ScrollbarThemeGtk::probablyHasThumb.

<span class="quote">&gt; Source/WebCore/platform/gtk/ScrollbarThemeGtk.h:65
&gt; +    bool m_hasForwardButtonStartPart : 1;</span >

I don't pretend to know if using a bitfield here would improve performance because due to better cache locality, or hurt performance due to the nature of bitfields (seems more likely to me?), but it only saves three bytes, and it's not like this structure is going to be allocated frequently (should be just once, right?) so I would personally use normal bools here.</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>