WebContent process is calling CGDisplayUsesInvertedPolarity
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Apr 2018 04:44:32 +0000 (04:44 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Apr 2018 04:44:32 +0000 (04:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184337
<rdar://problem/39215702>

Reviewed by Zalan Bujtas.

The PlatformScreenMac code is still calling display-related routines directly, specifically
CGDisplayUsesInvertedPolarity and CGDisplayUsesForceToGray. These should be brokered from
the UIProcess.

There's also no reason to avoid the brokering behavior on current WebKit builds. Remove
the compile guards so all macOS builds use this behavior.

Finally, add some ProcessPrivilege assertions to guard against accidentally calling these
routines in the future.

Source/WebCore:

Tested by existing regression tests.

* platform/PlatformScreen.h:
* platform/ScreenProperties.h:
(WebCore::ScreenProperties::encode const): Add new values.
(WebCore::ScreenProperties::decode):
* platform/mac/PlatformScreenMac.mm:
(WebCore::displayID): Add assertion that this is not calling display-related routines in
the WebContent process.
(WebCore::firstScreen): Ditto.
(WebCore::screenProperties): Moved higher in the file so it can be reused. Add calls to
CGDisplayUsesInvertedPolarity and CGDisplayUsesForceToGray.
(WebCore::getScreenProperties): Moved higher in the file so it can be reused. Stop
double-hashing displayID.
(WebCore::screenIsMonochrome): Use cached values in WebContent process. Assert if this
code attempts a display-related routine in the WebContent process.
(WebCore::screenHasInvertedColors): Ditto.
(WebCore::screenDepth): Add assertion that this is not calling display-related routines in
the WebContent process.
(WebCore::screenDepthPerComponent): Ditto.
(WebCore::screenRect): Ditto.
(WebCore::screenAvailableRect): Ditto.
(WebCore::screen): Ditto.
(WebCore::screenColorSpace): Ditto.
(WebCore::screenSupportsExtendedColor): Ditto.

Source/WebKit:

* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::initializeNewWebProcess): Activate screen brokering code for all builds.
* WebProcess/WebProcess.cpp: Ditto.
* WebProcess/WebProcess.h: Ditto.
* WebProcess/WebProcess.messages.in: Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230323 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/PlatformScreen.h
Source/WebCore/platform/ScreenProperties.h
Source/WebCore/platform/mac/PlatformScreenMac.mm
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebProcessPool.cpp
Source/WebKit/WebProcess/WebProcess.cpp
Source/WebKit/WebProcess/WebProcess.h
Source/WebKit/WebProcess/WebProcess.messages.in

index 7975d0a..3e5db17 100644 (file)
@@ -1,3 +1,47 @@
+2018-04-05  Brent Fulgham  <bfulgham@apple.com>
+
+        WebContent process is calling CGDisplayUsesInvertedPolarity
+        https://bugs.webkit.org/show_bug.cgi?id=184337
+        <rdar://problem/39215702>
+
+        Reviewed by Zalan Bujtas.
+
+        The PlatformScreenMac code is still calling display-related routines directly, specifically
+        CGDisplayUsesInvertedPolarity and CGDisplayUsesForceToGray. These should be brokered from
+        the UIProcess.
+        
+        There's also no reason to avoid the brokering behavior on current WebKit builds. Remove
+        the compile guards so all macOS builds use this behavior.
+        
+        Finally, add some ProcessPrivilege assertions to guard against accidentally calling these
+        routines in the future.
+
+        Tested by existing regression tests.
+
+        * platform/PlatformScreen.h:
+        * platform/ScreenProperties.h:
+        (WebCore::ScreenProperties::encode const): Add new values.
+        (WebCore::ScreenProperties::decode):
+        * platform/mac/PlatformScreenMac.mm:
+        (WebCore::displayID): Add assertion that this is not calling display-related routines in
+        the WebContent process.
+        (WebCore::firstScreen): Ditto.
+        (WebCore::screenProperties): Moved higher in the file so it can be reused. Add calls to
+        CGDisplayUsesInvertedPolarity and CGDisplayUsesForceToGray.
+        (WebCore::getScreenProperties): Moved higher in the file so it can be reused. Stop
+        double-hashing displayID.
+        (WebCore::screenIsMonochrome): Use cached values in WebContent process. Assert if this
+        code attempts a display-related routine in the WebContent process.
+        (WebCore::screenHasInvertedColors): Ditto.
+        (WebCore::screenDepth): Add assertion that this is not calling display-related routines in
+        the WebContent process.
+        (WebCore::screenDepthPerComponent): Ditto.
+        (WebCore::screenRect): Ditto.
+        (WebCore::screenAvailableRect): Ditto.
+        (WebCore::screen): Ditto.
+        (WebCore::screenColorSpace): Ditto.
+        (WebCore::screenSupportsExtendedColor): Ditto.
+
 2018-04-05  John Wilander  <wilander@apple.com>
 
         Resource Load Statistics: Apply cookie blocking to setCookiesFromDOM()
index 9e84680..f54f79a 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006 Apple Inc.  All rights reserved.
+ * Copyright (C) 2006-2018 Apple Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -89,10 +89,8 @@ WEBCORE_EXPORT NSRect toDeviceSpace(const FloatRect&, NSWindow *source);
 
 NSPoint flipScreenPoint(const NSPoint&, NSScreen *);
 
-#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
 WEBCORE_EXPORT void getScreenProperties(HashMap<PlatformDisplayID, ScreenProperties>&);
 WEBCORE_EXPORT void setScreenProperties(const HashMap<PlatformDisplayID, ScreenProperties>&);
-#endif
 
 #endif
 
index a1eeb88..d537d23 100644 (file)
@@ -34,6 +34,8 @@ struct ScreenProperties {
     FloatRect screenRect;
     int screenDepth { 0 };
     int screenDepthPerComponent { 0 };
+    bool screenHasInvertedColors { false };
+    bool screenIsMonochrome { false };
 
     template<class Encoder> void encode(Encoder&) const;
     template<class Decoder> static std::optional<ScreenProperties> decode(Decoder&);
@@ -42,7 +44,7 @@ struct ScreenProperties {
 template<class Encoder>
 void ScreenProperties::encode(Encoder& encoder) const
 {
-    encoder << screenAvailableRect << screenRect << screenDepth << screenDepthPerComponent;
+    encoder << screenAvailableRect << screenRect << screenDepth << screenDepthPerComponent << screenHasInvertedColors << screenIsMonochrome;
 }
 
 template<class Decoder>
@@ -67,8 +69,18 @@ std::optional<ScreenProperties> ScreenProperties::decode(Decoder& decoder)
     decoder >> screenDepthPerComponent;
     if (!screenDepthPerComponent)
         return std::nullopt;
-    
-    return { { WTFMove(*screenAvailableRect), WTFMove(*screenRect), WTFMove(*screenDepth), WTFMove(*screenDepthPerComponent) } };
+
+    std::optional<bool> screenHasInvertedColors;
+    decoder >> screenHasInvertedColors;
+    if (!screenHasInvertedColors)
+        return std::nullopt;
+
+    std::optional<bool> screenIsMonochrome;
+    decoder >> screenIsMonochrome;
+    if (!screenIsMonochrome)
+        return std::nullopt;
+
+    return { { WTFMove(*screenAvailableRect), WTFMove(*screenRect), WTFMove(*screenDepth), WTFMove(*screenDepthPerComponent), WTFMove(*screenHasInvertedColors), WTFMove(*screenIsMonochrome) } };
 }
 
 } // namespace WebCore
index f1af29b..4c9fe70 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006 Apple Inc.  All rights reserved.
+ * Copyright (C) 2006-2018 Apple Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -34,6 +34,7 @@
 #import "ScreenProperties.h"
 #import <ColorSync/ColorSync.h>
 #import <pal/spi/cg/CoreGraphicsSPI.h>
+#import <wtf/ProcessPrivilege.h>
 
 extern "C" {
 bool CGDisplayUsesInvertedPolarity(void);
@@ -47,6 +48,7 @@ namespace WebCore {
 
 static PlatformDisplayID displayID(NSScreen *screen)
 {
+    // FIXME: <http://webkit.org/b/184343> We should assert here if in WebContent process.
     return [[[screen deviceDescription] objectForKey:@"NSScreenNumber"] intValue];
 }
 
@@ -69,6 +71,7 @@ static PlatformDisplayID displayID(Widget* widget)
 // Screen containing the menubar.
 static NSScreen *firstScreen()
 {
+    // FIXME: <http://webkit.org/b/184343> We should assert here if in WebContent process.
     NSArray *screens = [NSScreen screens];
     if (![screens count])
         return nil;
@@ -91,19 +94,12 @@ static NSScreen *screen(Widget* widget)
     return screen(displayID(widget));
 }
 
-bool screenIsMonochrome(Widget*)
-{
-    // This is a system-wide accessibility setting, same on all screens.
-    return CGDisplayUsesForceToGray();
-}
-
-bool screenHasInvertedColors()
+static HashMap<PlatformDisplayID, ScreenProperties>& screenProperties()
 {
-    // This is a system-wide accessibility setting, same on all screens.
-    return CGDisplayUsesInvertedPolarity();
+    static NeverDestroyed<HashMap<PlatformDisplayID, ScreenProperties>> screenProperties;
+    return screenProperties;
 }
 
-#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
 void getScreenProperties(HashMap<PlatformDisplayID, ScreenProperties>& screenProperties)
 {
     for (NSScreen *screen in [NSScreen screens]) {
@@ -112,14 +108,11 @@ void getScreenProperties(HashMap<PlatformDisplayID, ScreenProperties>& screenPro
         FloatRect screenRect = [screen frame];
         int screenDepth = NSBitsPerPixelFromDepth(screen.depth);
         int screenDepthPerComponent = NSBitsPerSampleFromDepth(screen.depth);
-        screenProperties.set(WebCore::displayID(screen), ScreenProperties { screenAvailableRect, screenRect, screenDepth, screenDepthPerComponent });
-    }
-}
+        bool screenHasInvertedColors = CGDisplayUsesInvertedPolarity();
+        bool screenIsMonochrome = CGDisplayUsesForceToGray();
 
-static HashMap<PlatformDisplayID, ScreenProperties>& screenProperties()
-{
-    static NeverDestroyed<HashMap<PlatformDisplayID, ScreenProperties>> screenProperties;
-    return screenProperties;
+        screenProperties.set(WebCore::displayID(screen), ScreenProperties { screenAvailableRect, screenRect, screenDepth, screenDepthPerComponent, screenHasInvertedColors, screenIsMonochrome });
+    }
 }
 
 void setScreenProperties(const HashMap<PlatformDisplayID, ScreenProperties>& properties)
@@ -130,62 +123,87 @@ void setScreenProperties(const HashMap<PlatformDisplayID, ScreenProperties>& pro
 static ScreenProperties getScreenProperties(Widget* widget)
 {
     auto displayIDForWidget = displayID(widget);
-    if (displayIDForWidget && screenProperties().contains(displayIDForWidget))
-        return screenProperties().get(displayIDForWidget);
+    if (displayIDForWidget) {
+        auto screenPropertiesForDisplay = screenProperties().find(displayIDForWidget);
+        if (screenPropertiesForDisplay != screenProperties().end())
+            return screenPropertiesForDisplay->value;
+    }
+
     // Return property of the first screen if the screen is not found in the map.
-    auto iter = screenProperties().begin();
-    return screenProperties().get(iter->key);
+    return screenProperties().begin()->value;
+}
+
+bool screenIsMonochrome(Widget* widget)
+{
+    if (!screenProperties().isEmpty())
+        return getScreenProperties(widget).screenIsMonochrome;
+
+    // This is a system-wide accessibility setting, same on all screens.
+    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
+    return CGDisplayUsesForceToGray();
+}
+
+bool screenHasInvertedColors()
+{
+    if (!screenProperties().isEmpty())
+        return screenProperties().begin()->value.screenHasInvertedColors;
+
+    // This is a system-wide accessibility setting, same on all screens.
+    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
+    return CGDisplayUsesInvertedPolarity();
 }
-#endif
 
 int screenDepth(Widget* widget)
 {
-#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
     if (!screenProperties().isEmpty()) {
-        ASSERT(getScreenProperties(widget).screenDepth);
-        return getScreenProperties(widget).screenDepth;
+        auto screenDepth = getScreenProperties(widget).screenDepth;
+        ASSERT(screenDepth);
+        return screenDepth;
     }
-#endif
+
+    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
     return NSBitsPerPixelFromDepth(screen(widget).depth);
 }
 
 int screenDepthPerComponent(Widget* widget)
 {
-#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
     if (!screenProperties().isEmpty()) {
-        ASSERT(getScreenProperties(widget).screenDepthPerComponent);
-        return getScreenProperties(widget).screenDepthPerComponent;
+        auto depthPerComponent = getScreenProperties(widget).screenDepthPerComponent;
+        ASSERT(depthPerComponent);
+        return depthPerComponent;
     }
-#endif
+
+    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
     return NSBitsPerSampleFromDepth(screen(widget).depth);
 }
 
 FloatRect screenRect(Widget* widget)
 {
-#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
     if (!screenProperties().isEmpty())
         return getScreenProperties(widget).screenRect;
-#endif
+
+    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
     return toUserSpace([screen(widget) frame], window(widget));
 }
 
 FloatRect screenAvailableRect(Widget* widget)
 {
-#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
-    if (!screenProperties().isEmpty()) {
+    if (!screenProperties().isEmpty())
         return getScreenProperties(widget).screenAvailableRect;
-    }
-#endif
+
+    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
     return toUserSpace([screen(widget) visibleFrame], window(widget));
 }
 
 NSScreen *screen(NSWindow *window)
 {
+    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
     return [window screen] ?: firstScreen();
 }
 
 NSScreen *screen(PlatformDisplayID displayID)
 {
+    // FIXME: <http://webkit.org/b/184344> We should assert here if in WebContent process.
     for (NSScreen *screen in [NSScreen screens]) {
         if (WebCore::displayID(screen) == displayID)
             return screen;
@@ -195,6 +213,7 @@ NSScreen *screen(PlatformDisplayID displayID)
 
 CGColorSpaceRef screenColorSpace(Widget* widget)
 {
+    // FIXME: <http://webkit.org/b/184343> We should assert here if in WebContent process.
     return screen(widget).colorSpace.CGColorSpace;
 }
 
@@ -203,6 +222,7 @@ bool screenSupportsExtendedColor(Widget* widget)
     if (!widget)
         return false;
 
+    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
     return [screen(widget) canRepresentDisplayGamut:NSDisplayGamutP3];
 }
 
index 436217f..a279931 100644 (file)
@@ -1,3 +1,27 @@
+2018-04-05  Brent Fulgham  <bfulgham@apple.com>
+
+        WebContent process is calling CGDisplayUsesInvertedPolarity
+        https://bugs.webkit.org/show_bug.cgi?id=184337
+        <rdar://problem/39215702>
+
+        Reviewed by Zalan Bujtas.
+
+        The PlatformScreenMac code is still calling display-related routines directly, specifically
+        CGDisplayUsesInvertedPolarity and CGDisplayUsesForceToGray. These should be brokered from
+        the UIProcess.
+        
+        There's also no reason to avoid the brokering behavior on current WebKit builds. Remove
+        the compile guards so all macOS builds use this behavior.
+        
+        Finally, add some ProcessPrivilege assertions to guard against accidentally calling these
+        routines in the future.
+
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::initializeNewWebProcess): Activate screen brokering code for all builds.
+        * WebProcess/WebProcess.cpp: Ditto.
+        * WebProcess/WebProcess.h: Ditto.
+        * WebProcess/WebProcess.messages.in: Ditto.
+
 2018-04-05  Brady Eidson  <beidson@apple.com>
 
         Process Swap on Navigation causes many webpages to hang due to attempted process swap for iframe navigations.
index 40168ad..70eb7b2 100644 (file)
@@ -746,7 +746,7 @@ WebProcessProxy& WebProcessPool::createNewWebProcess(WebsiteDataStore& websiteDa
     return process;
 }
 
-#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
+#if PLATFORM(MAC)
 static void displayReconfigurationCallBack(CGDirectDisplayID display, CGDisplayChangeSummaryFlags flags, void *userInfo)
 {
     HashMap<PlatformDisplayID, ScreenProperties> screenProperties;
@@ -915,7 +915,7 @@ void WebProcessPool::initializeNewWebProcess(WebProcessProxy& process, WebsiteDa
     Inspector::RemoteInspector::singleton(); 
 #endif
 
-#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
+#if PLATFORM(MAC)
     registerDisplayConfigurationCallback();
 
     HashMap<PlatformDisplayID, ScreenProperties> screenProperties;
index e007764..4ec594b 100644 (file)
@@ -1698,7 +1698,7 @@ void WebProcess::registerServiceWorkerClients()
 
 #endif
 
-#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
+#if PLATFORM(MAC)
 void WebProcess::setScreenProperties(const HashMap<uint32_t, WebCore::ScreenProperties>& properties)
 {
     WebCore::setScreenProperties(properties);
index 02353e1..e573936 100644 (file)
@@ -34,7 +34,7 @@
 #include "ViewUpdateDispatcher.h"
 #include "WebInspectorInterruptDispatcher.h"
 #include <WebCore/ActivityState.h>
-#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
+#if PLATFORM(MAC)
 #include <WebCore/ScreenProperties.h>
 #endif
 #include <WebCore/Timer.h>
@@ -371,10 +371,12 @@ private:
     void didReceiveWebProcessMessage(IPC::Connection&, IPC::Decoder&);
     void didReceiveSyncWebProcessMessage(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder>&);
 
-#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
+#if PLATFORM(MAC)
     void setScreenProperties(const HashMap<uint32_t, WebCore::ScreenProperties>&);
+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
     void scrollerStylePreferenceChanged(bool useOverlayScrollbars);
 #endif
+#endif
 
     RefPtr<WebConnectionToUIProcess> m_webConnection;
 
index 3fb4c0b..3bd2779 100644 (file)
@@ -1,4 +1,4 @@
-# Copyright (C) 2010, 2016 Apple Inc. All rights reserved.
+# Copyright (C) 2010-2018 Apple Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions
@@ -128,8 +128,10 @@ messages -> WebProcess LegacyReceiver {
     CheckProcessLocalPortForActivity(struct WebCore::MessagePortIdentifier port, uint64_t callbackIdentifier)
     MessagesAvailableForPort(struct WebCore::MessagePortIdentifier port)
 
-#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
+#if PLATFORM(MAC)
     SetScreenProperties(HashMap<uint32_t, WebCore::ScreenProperties> screenProperties)
+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
     ScrollerStylePreferenceChanged(bool useOvelayScrollbars)
 #endif
+#endif
 }