Changeset 244554 in webkit

Timestamp:

Apr 23, 2019 12:13:48 PM (5 years ago)

Author:

timothy@apple.com

Message:

Speed up RenderTheme::systemColor on Speedometer2.
​https://bugs.webkit.org/show_bug.cgi?id=197203
rdar://problem/50056756

Reviewed by Tim Horton.

• rendering/RenderThemeIOS.mm:

(WebCore::RenderThemeIOS::systemColor const): Remove some unused code. And fetch the cache after an early return.

• rendering/RenderThemeMac.mm:

(WebCore::RenderThemeMac::systemColor const): Avoid some allocations in LocalDefaultSystemAppearance
when a CSS color is in the system color cache.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderThemeIOS.mm (modified) (2 diffs)
• rendering/RenderThemeMac.mm (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-04-23  Timothy Hatcher  <timothy@apple.com>
+
+        Speed up RenderTheme::systemColor on Speedometer2.
+        https://bugs.webkit.org/show_bug.cgi?id=197203
+        rdar://problem/50056756
+
+        Reviewed by Tim Horton.
+
+        * rendering/RenderThemeIOS.mm:
+        (WebCore::RenderThemeIOS::systemColor const): Remove some unused code. And fetch the cache after an early return.
+        * rendering/RenderThemeMac.mm:
+        (WebCore::RenderThemeMac::systemColor const): Avoid some allocations in LocalDefaultSystemAppearance
+        when a CSS color is in the system color cache.
+
 2019-04-23  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/Source/WebCore/rendering/RenderThemeIOS.mm

@@ -1419 +1419 @@
 Color RenderThemeIOS::systemColor(CSSValueID cssValueID, OptionSet<StyleColor::Options> options) const
 {
-    const bool useSystemAppearance = options.contains(StyleColor::Options::UseSystemAppearance);
     const bool forVisitedLink = options.contains(StyleColor::Options::ForVisitedLink);
-
-    auto& cache = colorCache(options);
-
-#if USE(APPLE_INTERNAL_SDK) && __has_include(<WebKitAdditions/RenderThemeIOSSystemColorAdditions.mm>)
-#include <WebKitAdditions/RenderThemeIOSSystemColorAdditions.mm>
-#endif
 
     // The system color cache below can't handle visited links. The only color value
@@ -1434 +1427 @@
         return RenderTheme::systemColor(cssValueID, options);
 
-    ASSERT_UNUSED(useSystemAppearance, !useSystemAppearance);
     ASSERT(!forVisitedLink);
 
+    auto& cache = colorCache(options);
     return cache.systemStyleColors.ensure(cssValueID, [this, cssValueID, options] () -> Color {
+#if USE(APPLE_INTERNAL_SDK) && __has_include(<WebKitAdditions/RenderThemeIOSSystemColorAdditions.mm>)
+#include <WebKitAdditions/RenderThemeIOSSystemColorAdditions.mm>
+#endif
+
         auto cssColorToSelector = [cssValueID] () -> SEL {
 #if USE(APPLE_INTERNAL_SDK) && __has_include(<WebKitAdditions/RenderThemeIOSColorToSelectorAdditions.mm>)

trunk/Source/WebCore/rendering/RenderThemeMac.mm

@@ -605 +605 @@
     const bool forVisitedLink = options.contains(StyleColor::Options::ForVisitedLink);
 
-    LocalDefaultSystemAppearance localAppearance(useDarkAppearance);
-
     auto& cache = colorCache(options);
 
     if (useSystemAppearance) {
         // Special handling for links and other system colors when the system appearance is desired.
-        auto systemAppearanceColor = [] (Color& color, SEL selector) -> Color {
+        auto systemAppearanceColor = [useDarkAppearance] (Color& color, SEL selector) -> Color {
             if (!color.isValid()) {
+                LocalDefaultSystemAppearance localAppearance(useDarkAppearance);
                 auto systemColor = wtfObjCMsgSend<NSColor *>([NSColor class], selector);
                 color = semanticColorFromNSColor(systemColor);
@@ -663 +662 @@
     ASSERT(!forVisitedLink);
 
-    return cache.systemStyleColors.ensure(cssValueID, [this, cssValueID, options, &localAppearance] () -> Color {
+    return cache.systemStyleColors.ensure(cssValueID, [this, cssValueID, options, useDarkAppearance] () -> Color {
+        LocalDefaultSystemAppearance localAppearance(useDarkAppearance);
+
         auto selectCocoaColor = [cssValueID] () -> SEL {
             switch (cssValueID) {