Bug 153185 – Remove TextRun::allowsRoundingHacks()

• Status: RESOLVED
• Resolution: FIXED
• Component: Text

[Simon Fraser (smfr)]

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.

[Myles C. Maxfield]

(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.

[Myles C. Maxfield]

Oh, rounding hacks are disallowed by default.

[Myles C. Maxfield]

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/

[Myles C. Maxfield]

Created attachment 269180 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 269180 [details]
Patch

r+ assuming GTK builds

[Myles C. Maxfield]

Created attachment 269181 [details]
Patch

[Myles C. Maxfield]

Committed r195180: <http://trac.webkit.org/changeset/195180>

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?

[Myles C. Maxfield]

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

(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.

[Myles C. Maxfield]

(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.

[Myles C. Maxfield]

(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.