Bug 167061 – [Cocoa] Unify FontPlatformData's hashing and equality operators

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Myles C. Maxfield]

[Cocoa] Unify FontPlatformData's hashing and equality operators

[Myles C. Maxfield]

Created attachment 298872 [details]
Patch

[Darin Adler]

Comment on attachment 298872 [details]
Patch

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

> Source/WebCore/platform/graphics/FontPlatformData.h:-175
> -    unsigned hash() const

Inlining this function was not important for performance?

> Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm:71
> +    ASSERT(m_font && other.m_font);

Best practice for assertions is to always break them up into separate lines rather than doing a single assertion with &&. That way we can tell which assertion failed.

[Build Bot]

Comment on attachment 298872 [details]
Patch

Attachment 298872 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2890892

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 298873 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 298872 [details]
Patch

Attachment 298872 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2890890

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 298874 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 298872 [details]
Patch

Attachment 298872 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2890924

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 298877 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 298872 [details]
Patch

Attachment 298872 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2890900

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 298878 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Myles C. Maxfield]

Created attachment 298879 [details]
Patch for committing

[Myles C. Maxfield]

Created attachment 298882 [details]
Patch for committing

[Myles C. Maxfield]

Comment on attachment 298872 [details]
Patch

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

>> Source/WebCore/platform/graphics/FontPlatformData.h:-175
>> -    unsigned hash() const
> 
> Inlining this function was not important for performance?

Running PLT overnight, I don't see any regression.

[WebKit Commit Bot]

Comment on attachment 298882 [details]
Patch for committing

Clearing flags on attachment: 298882

Committed r210778: <http://trac.webkit.org/changeset/210778>

[Myles C. Maxfield]

*** Bug 161009 has been marked as a duplicate of this bug. ***

[Ryan Haddad]

Reverted r210778 for reason:

This change caused multiple LayoutTests to crash.

Committed r210997: <http://trac.webkit.org/changeset/210997>

[Ryan Haddad]

*** Bug 167142 has been marked as a duplicate of this bug. ***

[Myles C. Maxfield]

Created attachment 299482 [details]
Patch for committing

[WebKit Commit Bot]

Comment on attachment 299482 [details]
Patch for committing

Clearing flags on attachment: 299482

Committed r211029: <http://trac.webkit.org/changeset/211029>