Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for icon_variants in Web Extensions. #32970

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

xeenon
Copy link
Contributor

@xeenon xeenon commented Aug 31, 2024

56a9103

Add support for icon_variants in Web Extensions.
https://webkit.org/b/278818
rdar://problem/134885372

Reviewed by Brian Weinstein.

Add support for `icon_variants` manifest parsing under the `WK_WEB_EXTENSIONS_ICON_VARIANTS` flag,
with optimizations to ensure efficient icon loading.

This change introduces `icon_variants` manifest parsing, explicitly supporting different icon sets,
such as dark mode icons. To achieve this efficiently, icons are now cached by size, reducing disk
I/O by avoiding repeated loads when the browser frequently requests the same icon. The cache is
automatically invalidated when device scales change, such as when connecting or disconnecting a
display with a different scale factor.

Only the necessary icons are loaded based on the specific scale factor of all screens, halving the
image loads compared to previous behavior. This ensures that even as more extensions adopt dark
mode icons, typical image loads remain at two images (light and dark).

Proposal: https://github.com/w3c/webextensions/blob/main/proposals/dark_mode_extension_icons.md
WECG issue: w3c/webextensions#229

* Source/WTF/wtf/PlatformEnableCocoa.h: Added ENABLE_WK_WEB_EXTENSIONS_ICON_VARIANTS.
* Source/WebCore/en.lproj/Localizable.strings: Updated.
* Source/WebKit/Platform/cocoa/CocoaHelpers.h:
* Source/WebKit/Platform/cocoa/CocoaHelpers.mm:
(WebKit::availableScreenScales): Added.
(WebKit::largestDisplayScale): Added.
* Source/WebKit/Platform/spi/ios/UIKitSPI.h:
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionActionCocoa.mm:
(WebKit::WebExtensionAction::icon):
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionCocoa.mm:
(WebKit::WebExtension::icon):
(WebKit::WebExtension::actionIcon):
(WebKit::WebExtension::populateActionPropertiesIfNeeded):
(WebKit::WebExtension::populateSidebarActionProperties):
(WebKit::WebExtension::populateSidePanelProperties):
(WebKit::WebExtension::imageForPath):
(WebKit::WebExtension::bestSizeInIconsDictionary): Added.
(WebKit::WebExtension::pathForBestImageInIconsDictionary):
(WebKit::WebExtension::bestImageInIconsDictionary):
(WebKit::WebExtension::bestImageForIconsDictionaryManifestKey):
(WebKit::toColorSchemes): Added.
(WebKit::WebExtension::iconsDictionaryForBestIconVariant): Added.
(WebKit::WebExtension::bestImageForIconVariants): Added.
(WebKit::WebExtension::bestImageForIconVariantsManifestKey): Added.
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionMenuItemCocoa.mm:
(WebKit::WebExtensionMenuItem::icon const):
* Source/WebKit/UIProcess/Extensions/WebExtension.h:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtension.mm:
(TestWebKitAPI::TEST(WKWebExtension, MultipleIconVariants)): Added.
(TestWebKitAPI::TEST(WKWebExtension, SingleIconVariant)): Added.
(TestWebKitAPI::TEST(WKWebExtension, AnySizeIconVariant)): Added.
(TestWebKitAPI::TEST(WKWebExtension, NoIconVariants)): Added.
(TestWebKitAPI::TEST(WKWebExtension, IconsAndIconVariantsSpecified)): Added.
(TestWebKitAPI::TEST(WKWebExtension, ActionIconVariantsMultiple)): Added.
(TestWebKitAPI::TEST(WKWebExtension, ActionIconSingleVariant)): Added.
(TestWebKitAPI::TEST(WKWebExtension, ActionIconAnySizeVariant)): Added.
(TestWebKitAPI::TEST(WKWebExtension, ActionNoIconVariants)): Added.
(TestWebKitAPI::TEST(WKWebExtension, ActionIconsAndIconVariantsSpecified)): Added.
* Tools/TestWebKitAPI/cocoa/WebExtensionUtilities.h:
* Tools/TestWebKitAPI/cocoa/WebExtensionUtilities.mm:
(TestWebKitAPI::Util::performWithAppearance): Added.
(TestWebKitAPI::Util::pixelColor): Added.
(TestWebKitAPI::Util::toSRGBColor): Added.
(TestWebKitAPI::Util::compareColors): Added.

Canonical link: https://commits.webkit.org/283118@main

6efdad3

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac 🛠 wpe 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2 🧪 wincairo-tests
✅ 🧪 webkitperl 🧪 ios-wk2 🧪 api-mac 🧪 api-wpe
🧪 ios-wk2-wpt 🧪 mac-wk1 🛠 wpe-cairo
🛠 🧪 jsc 🧪 api-ios 🧪 mac-wk2 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision 🧪 mac-AS-debug-wk2 🧪 gtk-wk2
✅ 🛠 vision-sim 🧪 mac-wk2-stress 🧪 api-gtk
🧪 vision-wk2 🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 🧪 unsafe-merge 🛠 tv 🧪 jsc-armv7-tests
🛠 tv-sim
🛠 watch
✅ 🛠 watch-sim

@xeenon xeenon self-assigned this Aug 31, 2024
@xeenon xeenon added the WebKit Extensions Bugs related to extension support. label Aug 31, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 31, 2024
@xeenon xeenon removed the merging-blocked Applied to prevent a change from being merged label Aug 31, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 31, 2024
@xeenon xeenon removed the merging-blocked Applied to prevent a change from being merged label Aug 31, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 31, 2024
@xeenon xeenon removed the merging-blocked Applied to prevent a change from being merged label Aug 31, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 31, 2024
@xeenon xeenon removed the merging-blocked Applied to prevent a change from being merged label Sep 2, 2024
// The returned image retains its link to the image asset and adapts to trait changes,
// automatically displaying the correct variant based on the current traits.
return [imageAsset imageWithTraitCollection:UITraitCollection.currentTraitCollection];
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// ! USE(APPKIT)

}

#if ENABLE(WK_WEB_EXTENSIONS_ICON_VARIANTS)
static OptionSet<WebExtension::ColorScheme> toColorSchemes(id value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not make the argument NSArray *? are we expecting this method to be called with other types in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might not be an array, and that case is treated as invalid, per the proposal. Instead of type checking at the call site, I type check inside here.

Incorrectly typed color_schemes values will be ignored, with an optional warning.

@xeenon xeenon added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks and removed unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks labels Sep 3, 2024
https://webkit.org/b/278818
rdar://problem/134885372

Reviewed by Brian Weinstein.

Add support for `icon_variants` manifest parsing under the `WK_WEB_EXTENSIONS_ICON_VARIANTS` flag,
with optimizations to ensure efficient icon loading.

This change introduces `icon_variants` manifest parsing, explicitly supporting different icon sets,
such as dark mode icons. To achieve this efficiently, icons are now cached by size, reducing disk
I/O by avoiding repeated loads when the browser frequently requests the same icon. The cache is
automatically invalidated when device scales change, such as when connecting or disconnecting a
display with a different scale factor.

Only the necessary icons are loaded based on the specific scale factor of all screens, halving the
image loads compared to previous behavior. This ensures that even as more extensions adopt dark
mode icons, typical image loads remain at two images (light and dark).

Proposal: https://github.com/w3c/webextensions/blob/main/proposals/dark_mode_extension_icons.md
WECG issue: w3c/webextensions#229

* Source/WTF/wtf/PlatformEnableCocoa.h: Added ENABLE_WK_WEB_EXTENSIONS_ICON_VARIANTS.
* Source/WebCore/en.lproj/Localizable.strings: Updated.
* Source/WebKit/Platform/cocoa/CocoaHelpers.h:
* Source/WebKit/Platform/cocoa/CocoaHelpers.mm:
(WebKit::availableScreenScales): Added.
(WebKit::largestDisplayScale): Added.
* Source/WebKit/Platform/spi/ios/UIKitSPI.h:
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionActionCocoa.mm:
(WebKit::WebExtensionAction::icon):
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionCocoa.mm:
(WebKit::WebExtension::icon):
(WebKit::WebExtension::actionIcon):
(WebKit::WebExtension::populateActionPropertiesIfNeeded):
(WebKit::WebExtension::populateSidebarActionProperties):
(WebKit::WebExtension::populateSidePanelProperties):
(WebKit::WebExtension::imageForPath):
(WebKit::WebExtension::bestSizeInIconsDictionary): Added.
(WebKit::WebExtension::pathForBestImageInIconsDictionary):
(WebKit::WebExtension::bestImageInIconsDictionary):
(WebKit::WebExtension::bestImageForIconsDictionaryManifestKey):
(WebKit::toColorSchemes): Added.
(WebKit::WebExtension::iconsDictionaryForBestIconVariant): Added.
(WebKit::WebExtension::bestImageForIconVariants): Added.
(WebKit::WebExtension::bestImageForIconVariantsManifestKey): Added.
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionMenuItemCocoa.mm:
(WebKit::WebExtensionMenuItem::icon const):
* Source/WebKit/UIProcess/Extensions/WebExtension.h:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtension.mm:
(TestWebKitAPI::TEST(WKWebExtension, MultipleIconVariants)): Added.
(TestWebKitAPI::TEST(WKWebExtension, SingleIconVariant)): Added.
(TestWebKitAPI::TEST(WKWebExtension, AnySizeIconVariant)): Added.
(TestWebKitAPI::TEST(WKWebExtension, NoIconVariants)): Added.
(TestWebKitAPI::TEST(WKWebExtension, IconsAndIconVariantsSpecified)): Added.
(TestWebKitAPI::TEST(WKWebExtension, ActionIconVariantsMultiple)): Added.
(TestWebKitAPI::TEST(WKWebExtension, ActionIconSingleVariant)): Added.
(TestWebKitAPI::TEST(WKWebExtension, ActionIconAnySizeVariant)): Added.
(TestWebKitAPI::TEST(WKWebExtension, ActionNoIconVariants)): Added.
(TestWebKitAPI::TEST(WKWebExtension, ActionIconsAndIconVariantsSpecified)): Added.
* Tools/TestWebKitAPI/cocoa/WebExtensionUtilities.h:
* Tools/TestWebKitAPI/cocoa/WebExtensionUtilities.mm:
(TestWebKitAPI::Util::performWithAppearance): Added.
(TestWebKitAPI::Util::pixelColor): Added.
(TestWebKitAPI::Util::toSRGBColor): Added.
(TestWebKitAPI::Util::compareColors): Added.

Canonical link: https://commits.webkit.org/283118@main
@webkit-commit-queue
Copy link
Collaborator

Committed 283118@main (56a9103): https://commits.webkit.org/283118@main

Reviewed commits have been landed. Closing PR #32970 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 56a9103 into WebKit:main Sep 3, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Sep 3, 2024
@xeenon xeenon deleted the bug/278818 branch September 3, 2024 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebKit Extensions Bugs related to extension support.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants