Bug 153185

Summary: Remove TextRun::allowsRoundingHacks()
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: TextAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: mitz, mmaxfield
Priority: P2    
Version: Safari 9   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

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.