Bug 231279 – ASSERTion failure in WebCore::deviceClass() on macCatalyst

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

[Tim Horton]

ASSERTion failure in WebCore::deviceClass() on macCatalyst

[Tim Horton]

Created attachment 440331 [details]
Patch

[EWS]

Committed r283638 (242589@main): <https://commits.webkit.org/242589@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 440331 [details].

[Radar WebKit Bug Importer]

<rdar://problem/83943064>

[Darin Adler]

Comment on attachment 440331 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        * platform/ios/Device.cpp:
> +        (WebCore::deviceClass):

Separately, should we stop exporting this unclear-how-to-use correctly function and instead expose more-semantic functions that match our uses?

3 uses are checking against iPad, so it can support a different media playback style than others in the iOS family
2 uses are checking for iPhone + iPod since we are omitting defaultMediaSource there (not sure why)
1 use is checking for iPhone + iPod + Apple Watch, to try to approximate -[UIDevice userInterfaceIdiom] when we can’t call it.

That’s it, just those 3 types of callers. Maybe we just expose three functions instead of using MGDeviceClass?

[Tim Horton]

(In reply to Darin Adler from comment #4)
> Comment on attachment 440331 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=440331&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        * platform/ios/Device.cpp:
> > +        (WebCore::deviceClass):
> 
> Separately, should we stop exporting this unclear-how-to-use correctly
> function and instead expose more-semantic functions that match our uses?
> 
> 3 uses are checking against iPad, so it can support a different media
> playback style than others in the iOS family

I expect these want to be iPhone+iPod checks instead, so we can probably expose just 2. And I agree!

[Darin Adler]

Comment on attachment 440331 [details]
Patch

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

>>> Source/WebCore/ChangeLog:9
>>> +        (WebCore::deviceClass):
>> 
>> Separately, should we stop exporting this unclear-how-to-use correctly function and instead expose more-semantic functions that match our uses?
>> 
>> 3 uses are checking against iPad, so it can support a different media playback style than others in the iOS family
>> 2 uses are checking for iPhone + iPod since we are omitting defaultMediaSource there (not sure why)
>> 1 use is checking for iPhone + iPod + Apple Watch, to try to approximate -[UIDevice userInterfaceIdiom] when we can’t call it.
>> 
>> That’s it, just those 3 types of callers. Maybe we just expose three functions instead of using MGDeviceClass?
> 
> I expect these want to be iPhone+iPod checks instead, so we can probably expose just 2. And I agree!

Then, I think we can expose just one function: something that returns true for iPhone+iPod+Watch, since in the hypothetical where we had media on watch, it would be iPhone-style, not iPad-style. We have a phone-style family (iPhone+iPod+Watch) and a larger-screen-style family (iPad+TV). And of course many features that should adapt to the current size/shape rather than the platform (so narrow apps on iPad act enough like iPhone). I would be so tempted to name this deviceClassIsSmallScreen.

But don’t ask me how this function differs from deviceHasIPadCapability.

[Tim Horton]

(In reply to Darin Adler from comment #6)
> Comment on attachment 440331 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=440331&action=review
> 
> >>> Source/WebCore/ChangeLog:9
> >>> +        (WebCore::deviceClass):
> >> 
> >> Separately, should we stop exporting this unclear-how-to-use correctly function and instead expose more-semantic functions that match our uses?
> >> 
> >> 3 uses are checking against iPad, so it can support a different media playback style than others in the iOS family
> >> 2 uses are checking for iPhone + iPod since we are omitting defaultMediaSource there (not sure why)
> >> 1 use is checking for iPhone + iPod + Apple Watch, to try to approximate -[UIDevice userInterfaceIdiom] when we can’t call it.
> >> 
> >> That’s it, just those 3 types of callers. Maybe we just expose three functions instead of using MGDeviceClass?
> > 
> > I expect these want to be iPhone+iPod checks instead, so we can probably expose just 2. And I agree!
> 
> Then, I think we can expose just one function: something that returns true
> for iPhone+iPod+Watch, since in the hypothetical where we had media on
> watch, it would be iPhone-style, not iPad-style. We have a phone-style
> family (iPhone+iPod+Watch) and a larger-screen-style family (iPad+TV). And
> of course many features that should adapt to the current size/shape rather
> than the platform (so narrow apps on iPad act enough like iPhone). I would
> be so tempted to name this deviceClassIsSmallScreen.

Good point! And then it 100% mimics the API we expose on top of UIUserInterfaceIdiom, but is non-adaptive. Seems like a great solution to reduce confusion. (except for the name; which we currently say "isPhoneOrWatch" but "isSmallScreen" is probably better since it does include iPod Touch as well)

> But don’t ask me how this function differs from deviceHasIPadCapability.

I don't understand this one either :) Need to do some digging.