Changeset 230351 in webkit

Timestamp:

Apr 6, 2018 12:37:17 PM (6 years ago)

Author:

Brent Fulgham

Message:

WebCore::screenColorSpace is retrieving CGColorSpace from NSScreen directly
​https://bugs.webkit.org/show_bug.cgi?id=184343
<rdar://problem/39224881>

Reviewed by Per Arne Vollan.

Revise ScreenProperties to serialize the screen's color space, and later
retrieve that in the WebContent process. This allows us to close off
the CGSWindowServer connection.

• platform/ScreenProperties.h:

(WebCore::ScreenProperties::encode const): Add CGColorSpaceRef support.
(WebCore::ScreenProperties::decode): Ditto.

• platform/mac/PlatformScreenMac.mm:

(WebCore::displayID): Add assertion to prevent use in WebContent process.
(WebCore::firstScreen): Ditto.
(WebCore::getScreenProperties): Add support for CGColorSpaceRef.
(WebCore::screenColorSpace): Retrieve cached version when in WebContent process.
Assert that NSScreen is not accessed in WebContent process.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/ScreenProperties.h (modified) (4 diffs)
• platform/mac/PlatformScreenMac.mm (modified) (5 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-04-06  Brent Fulgham  <bfulgham@apple.com>
+
+        WebCore::screenColorSpace is retrieving CGColorSpace from NSScreen directly
+        https://bugs.webkit.org/show_bug.cgi?id=184343
+        <rdar://problem/39224881>
+
+        Reviewed by Per Arne Vollan.
+
+        Revise ScreenProperties to serialize the screen's color space, and later
+        retrieve that in the WebContent process. This allows us to close off
+        the CGSWindowServer connection.
+
+        * platform/ScreenProperties.h:
+        (WebCore::ScreenProperties::encode const): Add CGColorSpaceRef support.
+        (WebCore::ScreenProperties::decode): Ditto.
+        * platform/mac/PlatformScreenMac.mm:
+        (WebCore::displayID): Add assertion to prevent use in WebContent process.
+        (WebCore::firstScreen): Ditto.
+        (WebCore::getScreenProperties): Add support for CGColorSpaceRef.
+        (WebCore::screenColorSpace): Retrieve cached version when in WebContent process.
+        Assert that NSScreen is not accessed in WebContent process.
+
 2018-04-06  Ms2ger  <Ms2ger@igalia.com>
 

trunk/Source/WebCore/platform/ScreenProperties.h

@@ -26 +26 @@
 #pragma once
 
+#if PLATFORM(MAC)
+
 #include "FloatRect.h"
+#include <wtf/RetainPtr.h>
+#include <wtf/text/WTFString.h>
+
+typedef struct CGColorSpace *CGColorSpaceRef;
 
 namespace WebCore {
@@ -33 +39 @@
     FloatRect screenAvailableRect;
     FloatRect screenRect;
+    RetainPtr<CGColorSpaceRef> colorSpace;
     int screenDepth { 0 };
     int screenDepthPerComponent { 0 };
     bool screenHasInvertedColors { false };
     bool screenIsMonochrome { false };
+
+    enum EncodedColorSpaceDataType {
+        Null,
+        ColorSpaceName,
+        ColorSpaceData,
+    };
 
     template<class Encoder> void encode(Encoder&) const;
@@ -46 +59 @@
 {
     encoder << screenAvailableRect << screenRect << screenDepth << screenDepthPerComponent << screenHasInvertedColors << screenIsMonochrome;
+
+    if (colorSpace) {
+        // Try to encode the name.
+        if (auto name = adoptCF(CGColorSpaceCopyName(colorSpace.get()))) {
+            encoder.encodeEnum(ColorSpaceName);
+            encoder << String(name.get());
+            return;
+        }
+
+        // Failing that, just encode the ICC data.
+        if (auto profileData = adoptCF(CGColorSpaceCopyICCData(colorSpace.get()))) {
+            encoder.encodeEnum(ColorSpaceData);
+
+            Vector<uint8_t> iccData;
+            iccData.append(CFDataGetBytePtr(profileData.get()), CFDataGetLength(profileData.get()));
+
+            encoder << iccData;
+            return;
+        }
+    }
+
+    // The color space was null or failed to be encoded.
+    encoder << Null;
 }
 
@@ -81 +117 @@
         return std::nullopt;
 
-    return { { WTFMove(*screenAvailableRect), WTFMove(*screenRect), WTFMove(*screenDepth), WTFMove(*screenDepthPerComponent), WTFMove(*screenHasInvertedColors), WTFMove(*screenIsMonochrome) } };
+    EncodedColorSpaceDataType dataType;
+    if (!decoder.decodeEnum(dataType))
+        return std::nullopt;
+
+    RetainPtr<CGColorSpaceRef> cgColorSpace;
+    switch (dataType) {
+    case Null:
+        break;
+    case ColorSpaceName: {
+        std::optional<String> colorSpaceName;
+        decoder >> colorSpaceName;
+        ASSERT(colorSpaceName);
+        if (!colorSpaceName)
+            return std::nullopt;
+
+        cgColorSpace = adoptCF(CGColorSpaceCreateWithName(colorSpaceName->createCFString().get()));
+        break;
+    }
+    case ColorSpaceData: {
+        std::optional<Vector<uint8_t>> iccData;
+        decoder >> iccData;
+        ASSERT(iccData);
+        if (!iccData)
+            return std::nullopt;
+
+        auto colorSpaceData = adoptCF(CFDataCreate(kCFAllocatorDefault, iccData->data(), iccData->size()));
+        // FIXME: <http://webkit.org/b/184358> We should switch to CGColorSpaceCreateICCBased.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+        cgColorSpace = adoptCF(CGColorSpaceCreateWithICCProfile(colorSpaceData.get()));
+#pragma clang diagnostic pop
+        break;
+    }
+    }
+
+    return { { WTFMove(*screenAvailableRect), WTFMove(*screenRect), WTFMove(cgColorSpace), WTFMove(*screenDepth), WTFMove(*screenDepthPerComponent), WTFMove(*screenHasInvertedColors), WTFMove(*screenIsMonochrome) } };
 }
 
 } // namespace WebCore
 
+#endif // PLATFORM(MAC)

trunk/Source/WebCore/platform/mac/PlatformScreenMac.mm

@@ -49 +49 @@
 static PlatformDisplayID displayID(NSScreen *screen)
 {
-    // FIXME: <http://webkit.org/b/184343> We should assert here if in WebContent process.
+    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
     return [[[screen deviceDescription] objectForKey:@"NSScreenNumber"] intValue];
 }
@@ -72 +72 @@
 static NSScreen *firstScreen()
 {
-    // FIXME: <http://webkit.org/b/184343> We should assert here if in WebContent process.
+    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
     NSArray *screens = [NSScreen screens];
     if (![screens count])
@@ -103 +103 @@
 void getScreenProperties(HashMap<PlatformDisplayID, ScreenProperties>& screenProperties)
 {
+    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
     for (NSScreen *screen in [NSScreen screens]) {
-        FloatRect screenAvailableRect = [screen visibleFrame];
-        screenAvailableRect.setY(NSMaxY([screen frame]) - (screenAvailableRect.y() + screenAvailableRect.height())); // flip
-        FloatRect screenRect = [screen frame];
+        FloatRect screenAvailableRect = screen.visibleFrame;
+        screenAvailableRect.setY(NSMaxY(screen.frame) - (screenAvailableRect.y() + screenAvailableRect.height())); // flip
+        FloatRect screenRect = screen.frame;
+
+        RetainPtr<CGColorSpaceRef> colorSpace = screen.colorSpace.CGColorSpace;
+
         int screenDepth = NSBitsPerPixelFromDepth(screen.depth);
         int screenDepthPerComponent = NSBitsPerSampleFromDepth(screen.depth);
@@ -112 +116 @@
         bool screenIsMonochrome = CGDisplayUsesForceToGray();
 
-        screenProperties.set(WebCore::displayID(screen), ScreenProperties { screenAvailableRect, screenRect, screenDepth, screenDepthPerComponent, screenHasInvertedColors, screenIsMonochrome });
+        screenProperties.set(WebCore::displayID(screen), ScreenProperties { screenAvailableRect, screenRect, colorSpace, screenDepth, screenDepthPerComponent, screenHasInvertedColors, screenIsMonochrome });
     }
 }
@@ -214 +218 @@
 CGColorSpaceRef screenColorSpace(Widget* widget)
 {
-    // FIXME: <http://webkit.org/b/184343> We should assert here if in WebContent process.
+    if (!screenProperties().isEmpty())
+        return getScreenProperties(widget).colorSpace.get();
+
+    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
     return screen(widget).colorSpace.CGColorSpace;
 }