231279 2021-10-05 22:43:22 -0700 ASSERTion failure in WebCore::deviceClass() on macCatalyst 2021-10-06 13:11:02 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 thorton thorton aestes andersca darin dino sam simon.fraser webkit-bug-importer wenson_hsieh oldest_to_newest 1800953 0 thorton 2021-10-05 22:43:22 -0700 ASSERTion failure in WebCore::deviceClass() on macCatalyst 1800954 1 440331 thorton 2021-10-05 22:43:58 -0700 Created attachment 440331 Patch 1801207 2 ews-feeder 2021-10-06 11:17:49 -0700 Committed r283638 (242589@main): <https://commits.webkit.org/242589@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 440331. 1801208 3 webkit-bug-importer 2021-10-06 11:19:52 -0700 <rdar://problem/83943064> 1801233 4 440331 darin 2021-10-06 11:55:03 -0700 Comment on attachment 440331 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? 1801236 5 thorton 2021-10-06 11:59:57 -0700 (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! 1801264 6 440331 darin 2021-10-06 12:34:19 -0700 Comment on attachment 440331 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. 1801288 7 thorton 2021-10-06 13:11:02 -0700 (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. 440331 2021-10-05 22:43:58 -0700 2021-10-06 11:17:50 -0700 Patch bug-231279-20211005224357.patch text/plain 1832 thorton SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDI4MzYwMCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBACisyMDIxLTEwLTA1ICBUaW0gSG9y dG9uICA8dGltb3RoeV9ob3J0b25AYXBwbGUuY29tPgorCisgICAgICAgIEFTU0VSVGlvbiBmYWls dXJlIGluIFdlYkNvcmU6OmRldmljZUNsYXNzKCkgb24gbWFjQ2F0YWx5c3QKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIzMTI3OQorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogcGxhdGZvcm0vaW9zL0Rldmlj ZS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpkZXZpY2VDbGFzcyk6CisgICAgICAgIFNpbXBsaWZ5 IGRldmljZUNsYXNzKCk7IHRoZXJlJ3Mgbm8gcmVhc29uIHRvIGFzc2VydCBoZXJlLCBjYWxsZXJz CisgICAgICAgIHNob3VsZCBiZSB3cml0dGVuIHRvIGFzc3VtZSB0aGV5IGRvbid0IGtub3cgdGhl IGZ1dHVyZS4gQWxzbywgdGhpcworICAgICAgICBoYXMgYmVlbiBhc3NlcnRpbmcgb24gbWFjT1Mg Zm9yIGFnZXMuLi4KKwogMjAyMS0xMC0wNSAgSm9obiBXaWxhbmRlciAgPHdpbGFuZGVyQGFwcGxl LmNvbT4KIAogICAgICAgICBQQ006IEFsbG93IG1lYXN1cmVtZW50IG9mIGxpbmtzIGluIG5lc3Rl ZCwgY3Jvc3Mtc2l0ZSBpZnJhbWVzCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9pb3Mv RGV2aWNlLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9pb3MvRGV2 aWNlLmNwcAkocmV2aXNpb24gMjgzNjAwKQorKysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vaW9z L0RldmljZS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTM3LDIxICszNyw3IEBAIG5hbWVzcGFjZSBX ZWJDb3JlIHsKIAogTUdEZXZpY2VDbGFzcyBkZXZpY2VDbGFzcygpCiB7Ci0gICAgc3RhdGljIE1H RGV2aWNlQ2xhc3MgZGV2aWNlQ2xhc3MgPSBbXSB7Ci0gICAgICAgIGludCBkZXZpY2VDbGFzc051 bWJlciA9IE1HR2V0U0ludDMyQW5zd2VyKGtNR1FEZXZpY2VDbGFzc051bWJlciwgTUdEZXZpY2VD bGFzc0ludmFsaWQpOwotICAgICAgICBzd2l0Y2ggKGRldmljZUNsYXNzTnVtYmVyKSB7Ci0gICAg ICAgIGNhc2UgTUdEZXZpY2VDbGFzc0ludmFsaWQ6Ci0gICAgICAgIGNhc2UgTUdEZXZpY2VDbGFz c2lQaG9uZToKLSAgICAgICAgY2FzZSBNR0RldmljZUNsYXNzaVBvZDoKLSAgICAgICAgY2FzZSBN R0RldmljZUNsYXNzaVBhZDoKLSAgICAgICAgY2FzZSBNR0RldmljZUNsYXNzQXBwbGVUVjoKLSAg ICAgICAgY2FzZSBNR0RldmljZUNsYXNzV2F0Y2g6Ci0gICAgICAgICAgICBicmVhazsKLSAgICAg ICAgZGVmYXVsdDoKLSAgICAgICAgICAgIEFTU0VSVF9OT1RfUkVBQ0hFRCgpOwotICAgICAgICB9 Ci0gICAgICAgIHJldHVybiBzdGF0aWNfY2FzdDxNR0RldmljZUNsYXNzPihkZXZpY2VDbGFzc051 bWJlcik7Ci0gICAgfSgpOworICAgIHN0YXRpYyBNR0RldmljZUNsYXNzIGRldmljZUNsYXNzID0g c3RhdGljX2Nhc3Q8TUdEZXZpY2VDbGFzcz4oTUdHZXRTSW50MzJBbnN3ZXIoa01HUURldmljZUNs YXNzTnVtYmVyLCBNR0RldmljZUNsYXNzSW52YWxpZCkpOwogICAgIHJldHVybiBkZXZpY2VDbGFz czsKIH0KIAo=