[webkit-reviews] review granted: [Bug 180305] Make some minor adjustments to TouchBarMenuData and TouchBarMenuItemData : [Attachment 328216] Merge in dbates' suggestions as well

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 180305: Make some minor adjustments to TouchBarMenuData and
TouchBarMenuItemData
https://bugs.webkit.org/show_bug.cgi?id=180305

Attachment 328216: Merge in dbates' suggestions as well

https://bugs.webkit.org/attachment.cgi?id=328216&action=review




--- Comment #3 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 328216
  --> https://bugs.webkit.org/attachment.cgi?id=328216
Merge in dbates' suggestions as well

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

r=me

> Source/WebKit/ChangeLog:37
> +	   (WebKit::operator==):

Heh, prepareChangeLog does the best it can do here. You could move this up
above your comment on line 34-35 though, it is the TouchBarMenuItemData's
operator==

> Source/WebKit/Shared/TouchBarMenuItemData.cpp:36
> +static ItemType getItemType(const String&)

The parameter is unused right now, but we plan on using it? Otherwise we could
just inline this and avoid the attributeWithotuSynchronization for the
`typeAttr` below.

> Source/WebKit/Shared/TouchBarMenuItemData.h:54
>      bool validTouchBarDisplay { true };

You can put the boolean at the bottom of the list of members to potentially
avoid unnecessary padding bytes in the struct.