Bug 153185 - Remove TextRun::allowsRoundingHacks()
Summary: Remove TextRun::allowsRoundingHacks()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: Safari 9
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-16 10:55 PST by Simon Fraser (smfr)
Modified: 2016-01-17 10:17 PST (History)
2 users (show)

See Also:


Attachments
Patch (209.62 KB, patch)
2016-01-16 14:15 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (214.94 KB, patch)
2016-01-16 14:49 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2016-01-16 10:55:07 PST
TextRun::allowsRoundingHacks() and callers seem to be unused. It's exposed as WebView SPI, but no-one seems to use it. It's also exposed in Internals.
Comment 1 Myles C. Maxfield 2016-01-16 12:53:54 PST
(In reply to comment #0)
> TextRun::allowsRoundingHacks() and callers seem to be unused. It's exposed
> as WebView SPI, but no-one seems to use it. It's also exposed in Internals.

It looks like Internals::resetToConsistentState() calls TextRun::setAllowsRoundingHacks(false) thereby forcibly disabling all rounding hacks for all layout tests.

There are some callers (RenderTheme, RenderFileUploadControl) who try to use rounding hacks; allowing them to use rounding hacks in LayoutTests will cause behavior change.

In the interest of testing what we're shipping, it seems that we should remove the flag and rebaseline the tests.
Comment 2 Myles C. Maxfield 2016-01-16 13:07:12 PST
Oh, rounding hacks are disallowed by default.
Comment 3 Myles C. Maxfield 2016-01-16 13:12:54 PST
Yeah, it looks like the only case where rounding hacks are re-enabled are in 2 of our layout tests, and for iOS 4 and below (which is no longer a supported OS). I should just remove the entire rounding hacks machinery.

Yay!!!! \o/
Comment 4 Myles C. Maxfield 2016-01-16 14:15:20 PST
Created attachment 269180 [details]
Patch
Comment 5 Simon Fraser (smfr) 2016-01-16 14:47:58 PST
Comment on attachment 269180 [details]
Patch

r+ assuming GTK builds
Comment 6 Myles C. Maxfield 2016-01-16 14:49:16 PST
Created attachment 269181 [details]
Patch
Comment 7 Myles C. Maxfield 2016-01-16 20:54:46 PST
Committed r195180: <http://trac.webkit.org/changeset/195180>
Comment 8 mitz 2016-01-16 21:00:15 PST
Comment on attachment 269181 [details]
Patch

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

> Source/WebKit/ios/Misc/WebUIKitSupport.mm:-73
> -    [WebView _setAllowsRoundingHacks:!linkedOnOrAfterIOS5()];

Isn’t this changing the behavior of program linked before iOS 5?
Comment 9 Myles C. Maxfield 2016-01-17 09:38:15 PST
Comment on attachment 269181 [details]
Patch

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

>> Source/WebKit/ios/Misc/WebUIKitSupport.mm:-73
>> -    [WebView _setAllowsRoundingHacks:!linkedOnOrAfterIOS5()];
> 
> Isn’t this changing the behavior of program linked before iOS 5?

AFAIK we don't build from ToT for iOS 4 and below
Comment 10 mitz 2016-01-17 09:43:20 PST
(In reply to comment #9)
> Comment on attachment 269181 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=269181&action=review
> 
> >> Source/WebKit/ios/Misc/WebUIKitSupport.mm:-73
> >> -    [WebView _setAllowsRoundingHacks:!linkedOnOrAfterIOS5()];
> > 
> > Isn’t this changing the behavior of program linked before iOS 5?
> 
> AFAIK we don't build from ToT for iOS 4 and below

The question is about WebKit clients linked on iOS 4.x or earlier.
Comment 11 Myles C. Maxfield 2016-01-17 09:52:41 PST
(In reply to comment #10)
> (In reply to comment #9)
> > Comment on attachment 269181 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=269181&action=review
> > 
> > >> Source/WebKit/ios/Misc/WebUIKitSupport.mm:-73
> > >> -    [WebView _setAllowsRoundingHacks:!linkedOnOrAfterIOS5()];
> > > 
> > > Isn’t this changing the behavior of program linked before iOS 5?
> > 
> > AFAIK we don't build from ToT for iOS 4 and below
> 
> The question is about WebKit clients linked on iOS 4.x or earlier.

Oh, you're talking about old apps on a new iOS. I'll determine on Monday if we can make this change.
Comment 12 Myles C. Maxfield 2016-01-17 10:17:08 PST
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > Comment on attachment 269181 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=269181&action=review
> > > 
> > > >> Source/WebKit/ios/Misc/WebUIKitSupport.mm:-73
> > > >> -    [WebView _setAllowsRoundingHacks:!linkedOnOrAfterIOS5()];
> > > > 
> > > > Isn’t this changing the behavior of program linked before iOS 5?
> > > 
> > > AFAIK we don't build from ToT for iOS 4 and below
> > 
> > The question is about WebKit clients linked on iOS 4.x or earlier.
> 
> Oh, you're talking about old apps on a new iOS. I'll determine on Monday if
> we can make this change.

Tuesday. Monday is a holiday.