Changeset 230323 in webkit


Ignore:
Timestamp:
Apr 5, 2018 9:44:32 PM (6 years ago)
Author:
Brent Fulgham
Message:

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.

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.
Location:
trunk/Source
Files:
9 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r230319 r230323  
     12018-04-05  Brent Fulgham  <bfulgham@apple.com>
     2
     3        WebContent process is calling CGDisplayUsesInvertedPolarity
     4        https://bugs.webkit.org/show_bug.cgi?id=184337
     5        <rdar://problem/39215702>
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        The PlatformScreenMac code is still calling display-related routines directly, specifically
     10        CGDisplayUsesInvertedPolarity and CGDisplayUsesForceToGray. These should be brokered from
     11        the UIProcess.
     12       
     13        There's also no reason to avoid the brokering behavior on current WebKit builds. Remove
     14        the compile guards so all macOS builds use this behavior.
     15       
     16        Finally, add some ProcessPrivilege assertions to guard against accidentally calling these
     17        routines in the future.
     18
     19        Tested by existing regression tests.
     20
     21        * platform/PlatformScreen.h:
     22        * platform/ScreenProperties.h:
     23        (WebCore::ScreenProperties::encode const): Add new values.
     24        (WebCore::ScreenProperties::decode):
     25        * platform/mac/PlatformScreenMac.mm:
     26        (WebCore::displayID): Add assertion that this is not calling display-related routines in
     27        the WebContent process.
     28        (WebCore::firstScreen): Ditto.
     29        (WebCore::screenProperties): Moved higher in the file so it can be reused. Add calls to
     30        CGDisplayUsesInvertedPolarity and CGDisplayUsesForceToGray.
     31        (WebCore::getScreenProperties): Moved higher in the file so it can be reused. Stop
     32        double-hashing displayID.
     33        (WebCore::screenIsMonochrome): Use cached values in WebContent process. Assert if this
     34        code attempts a display-related routine in the WebContent process.
     35        (WebCore::screenHasInvertedColors): Ditto.
     36        (WebCore::screenDepth): Add assertion that this is not calling display-related routines in
     37        the WebContent process.
     38        (WebCore::screenDepthPerComponent): Ditto.
     39        (WebCore::screenRect): Ditto.
     40        (WebCore::screenAvailableRect): Ditto.
     41        (WebCore::screen): Ditto.
     42        (WebCore::screenColorSpace): Ditto.
     43        (WebCore::screenSupportsExtendedColor): Ditto.
     44
    1452018-04-05  John Wilander  <wilander@apple.com>
    246
  • trunk/Source/WebCore/platform/PlatformScreen.h

    r230059 r230323  
    11/*
    2  * Copyright (C) 2006 Apple Inc.  All rights reserved.
     2 * Copyright (C) 2006-2018 Apple Inc.  All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    9090NSPoint flipScreenPoint(const NSPoint&, NSScreen *);
    9191
    92 #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
    9392WEBCORE_EXPORT void getScreenProperties(HashMap<PlatformDisplayID, ScreenProperties>&);
    9493WEBCORE_EXPORT void setScreenProperties(const HashMap<PlatformDisplayID, ScreenProperties>&);
    95 #endif
    9694
    9795#endif
  • trunk/Source/WebCore/platform/ScreenProperties.h

    r228940 r230323  
    3535    int screenDepth { 0 };
    3636    int screenDepthPerComponent { 0 };
     37    bool screenHasInvertedColors { false };
     38    bool screenIsMonochrome { false };
    3739
    3840    template<class Encoder> void encode(Encoder&) const;
     
    4345void ScreenProperties::encode(Encoder& encoder) const
    4446{
    45     encoder << screenAvailableRect << screenRect << screenDepth << screenDepthPerComponent;
     47    encoder << screenAvailableRect << screenRect << screenDepth << screenDepthPerComponent << screenHasInvertedColors << screenIsMonochrome;
    4648}
    4749
     
    6870    if (!screenDepthPerComponent)
    6971        return std::nullopt;
    70    
    71     return { { WTFMove(*screenAvailableRect), WTFMove(*screenRect), WTFMove(*screenDepth), WTFMove(*screenDepthPerComponent) } };
     72
     73    std::optional<bool> screenHasInvertedColors;
     74    decoder >> screenHasInvertedColors;
     75    if (!screenHasInvertedColors)
     76        return std::nullopt;
     77
     78    std::optional<bool> screenIsMonochrome;
     79    decoder >> screenIsMonochrome;
     80    if (!screenIsMonochrome)
     81        return std::nullopt;
     82
     83    return { { WTFMove(*screenAvailableRect), WTFMove(*screenRect), WTFMove(*screenDepth), WTFMove(*screenDepthPerComponent), WTFMove(*screenHasInvertedColors), WTFMove(*screenIsMonochrome) } };
    7284}
    7385
  • trunk/Source/WebCore/platform/mac/PlatformScreenMac.mm

    r228940 r230323  
    11/*
    2  * Copyright (C) 2006 Apple Inc.  All rights reserved.
     2 * Copyright (C) 2006-2018 Apple Inc.  All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    3535#import <ColorSync/ColorSync.h>
    3636#import <pal/spi/cg/CoreGraphicsSPI.h>
     37#import <wtf/ProcessPrivilege.h>
    3738
    3839extern "C" {
     
    4849static PlatformDisplayID displayID(NSScreen *screen)
    4950{
     51    // FIXME: <http://webkit.org/b/184343> We should assert here if in WebContent process.
    5052    return [[[screen deviceDescription] objectForKey:@"NSScreenNumber"] intValue];
    5153}
     
    7072static NSScreen *firstScreen()
    7173{
     74    // FIXME: <http://webkit.org/b/184343> We should assert here if in WebContent process.
    7275    NSArray *screens = [NSScreen screens];
    7376    if (![screens count])
     
    9295}
    9396
    94 bool screenIsMonochrome(Widget*)
    95 {
    96     // This is a system-wide accessibility setting, same on all screens.
    97     return CGDisplayUsesForceToGray();
    98 }
    99 
    100 bool screenHasInvertedColors()
    101 {
    102     // This is a system-wide accessibility setting, same on all screens.
    103     return CGDisplayUsesInvertedPolarity();
    104 }
    105 
    106 #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
     97static HashMap<PlatformDisplayID, ScreenProperties>& screenProperties()
     98{
     99    static NeverDestroyed<HashMap<PlatformDisplayID, ScreenProperties>> screenProperties;
     100    return screenProperties;
     101}
     102
    107103void getScreenProperties(HashMap<PlatformDisplayID, ScreenProperties>& screenProperties)
    108104{
     
    113109        int screenDepth = NSBitsPerPixelFromDepth(screen.depth);
    114110        int screenDepthPerComponent = NSBitsPerSampleFromDepth(screen.depth);
    115         screenProperties.set(WebCore::displayID(screen), ScreenProperties { screenAvailableRect, screenRect, screenDepth, screenDepthPerComponent });
    116     }
    117 }
    118 
    119 static HashMap<PlatformDisplayID, ScreenProperties>& screenProperties()
    120 {
    121     static NeverDestroyed<HashMap<PlatformDisplayID, ScreenProperties>> screenProperties;
    122     return screenProperties;
     111        bool screenHasInvertedColors = CGDisplayUsesInvertedPolarity();
     112        bool screenIsMonochrome = CGDisplayUsesForceToGray();
     113
     114        screenProperties.set(WebCore::displayID(screen), ScreenProperties { screenAvailableRect, screenRect, screenDepth, screenDepthPerComponent, screenHasInvertedColors, screenIsMonochrome });
     115    }
    123116}
    124117
     
    131124{
    132125    auto displayIDForWidget = displayID(widget);
    133     if (displayIDForWidget && screenProperties().contains(displayIDForWidget))
    134         return screenProperties().get(displayIDForWidget);
     126    if (displayIDForWidget) {
     127        auto screenPropertiesForDisplay = screenProperties().find(displayIDForWidget);
     128        if (screenPropertiesForDisplay != screenProperties().end())
     129            return screenPropertiesForDisplay->value;
     130    }
     131
    135132    // Return property of the first screen if the screen is not found in the map.
    136     auto iter = screenProperties().begin();
    137     return screenProperties().get(iter->key);
    138 }
    139 #endif
     133    return screenProperties().begin()->value;
     134}
     135
     136bool screenIsMonochrome(Widget* widget)
     137{
     138    if (!screenProperties().isEmpty())
     139        return getScreenProperties(widget).screenIsMonochrome;
     140
     141    // This is a system-wide accessibility setting, same on all screens.
     142    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
     143    return CGDisplayUsesForceToGray();
     144}
     145
     146bool screenHasInvertedColors()
     147{
     148    if (!screenProperties().isEmpty())
     149        return screenProperties().begin()->value.screenHasInvertedColors;
     150
     151    // This is a system-wide accessibility setting, same on all screens.
     152    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
     153    return CGDisplayUsesInvertedPolarity();
     154}
    140155
    141156int screenDepth(Widget* widget)
    142157{
    143 #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
    144158    if (!screenProperties().isEmpty()) {
    145         ASSERT(getScreenProperties(widget).screenDepth);
    146         return getScreenProperties(widget).screenDepth;
    147     }
    148 #endif
     159        auto screenDepth = getScreenProperties(widget).screenDepth;
     160        ASSERT(screenDepth);
     161        return screenDepth;
     162    }
     163
     164    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
    149165    return NSBitsPerPixelFromDepth(screen(widget).depth);
    150166}
     
    152168int screenDepthPerComponent(Widget* widget)
    153169{
    154 #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
    155170    if (!screenProperties().isEmpty()) {
    156         ASSERT(getScreenProperties(widget).screenDepthPerComponent);
    157         return getScreenProperties(widget).screenDepthPerComponent;
    158     }
    159 #endif
     171        auto depthPerComponent = getScreenProperties(widget).screenDepthPerComponent;
     172        ASSERT(depthPerComponent);
     173        return depthPerComponent;
     174    }
     175
     176    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
    160177    return NSBitsPerSampleFromDepth(screen(widget).depth);
    161178}
     
    163180FloatRect screenRect(Widget* widget)
    164181{
    165 #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
    166182    if (!screenProperties().isEmpty())
    167183        return getScreenProperties(widget).screenRect;
    168 #endif
     184
     185    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
    169186    return toUserSpace([screen(widget) frame], window(widget));
    170187}
     
    172189FloatRect screenAvailableRect(Widget* widget)
    173190{
    174 #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
    175     if (!screenProperties().isEmpty()) {
     191    if (!screenProperties().isEmpty())
    176192        return getScreenProperties(widget).screenAvailableRect;
    177     }
    178 #endif
     193
     194    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
    179195    return toUserSpace([screen(widget) visibleFrame], window(widget));
    180196}
     
    182198NSScreen *screen(NSWindow *window)
    183199{
     200    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
    184201    return [window screen] ?: firstScreen();
    185202}
     
    187204NSScreen *screen(PlatformDisplayID displayID)
    188205{
     206    // FIXME: <http://webkit.org/b/184344> We should assert here if in WebContent process.
    189207    for (NSScreen *screen in [NSScreen screens]) {
    190208        if (WebCore::displayID(screen) == displayID)
     
    196214CGColorSpaceRef screenColorSpace(Widget* widget)
    197215{
     216    // FIXME: <http://webkit.org/b/184343> We should assert here if in WebContent process.
    198217    return screen(widget).colorSpace.CGColorSpace;
    199218}
     
    204223        return false;
    205224
     225    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
    206226    return [screen(widget) canRepresentDisplayGamut:NSDisplayGamutP3];
    207227}
  • trunk/Source/WebKit/ChangeLog

    r230315 r230323  
     12018-04-05  Brent Fulgham  <bfulgham@apple.com>
     2
     3        WebContent process is calling CGDisplayUsesInvertedPolarity
     4        https://bugs.webkit.org/show_bug.cgi?id=184337
     5        <rdar://problem/39215702>
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        The PlatformScreenMac code is still calling display-related routines directly, specifically
     10        CGDisplayUsesInvertedPolarity and CGDisplayUsesForceToGray. These should be brokered from
     11        the UIProcess.
     12       
     13        There's also no reason to avoid the brokering behavior on current WebKit builds. Remove
     14        the compile guards so all macOS builds use this behavior.
     15       
     16        Finally, add some ProcessPrivilege assertions to guard against accidentally calling these
     17        routines in the future.
     18
     19        * UIProcess/WebProcessPool.cpp:
     20        (WebKit::WebProcessPool::initializeNewWebProcess): Activate screen brokering code for all builds.
     21        * WebProcess/WebProcess.cpp: Ditto.
     22        * WebProcess/WebProcess.h: Ditto.
     23        * WebProcess/WebProcess.messages.in: Ditto.
     24
    1252018-04-05  Brady Eidson  <beidson@apple.com>
    226
  • trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

    r230293 r230323  
    747747}
    748748
    749 #if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
     749#if PLATFORM(MAC)
    750750static void displayReconfigurationCallBack(CGDirectDisplayID display, CGDisplayChangeSummaryFlags flags, void *userInfo)
    751751{
     
    916916#endif
    917917
    918 #if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
     918#if PLATFORM(MAC)
    919919    registerDisplayConfigurationCallback();
    920920
  • trunk/Source/WebKit/WebProcess/WebProcess.cpp

    r230290 r230323  
    16991699#endif
    17001700
    1701 #if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
     1701#if PLATFORM(MAC)
    17021702void WebProcess::setScreenProperties(const HashMap<uint32_t, WebCore::ScreenProperties>& properties)
    17031703{
  • trunk/Source/WebKit/WebProcess/WebProcess.h

    r230290 r230323  
    3535#include "WebInspectorInterruptDispatcher.h"
    3636#include <WebCore/ActivityState.h>
    37 #if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
     37#if PLATFORM(MAC)
    3838#include <WebCore/ScreenProperties.h>
    3939#endif
     
    372372    void didReceiveSyncWebProcessMessage(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder>&);
    373373
    374 #if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
     374#if PLATFORM(MAC)
    375375    void setScreenProperties(const HashMap<uint32_t, WebCore::ScreenProperties>&);
     376#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
    376377    void scrollerStylePreferenceChanged(bool useOverlayScrollbars);
     378#endif
    377379#endif
    378380
  • trunk/Source/WebKit/WebProcess/WebProcess.messages.in

    r229917 r230323  
    1 # Copyright (C) 2010, 2016 Apple Inc. All rights reserved.
     1# Copyright (C) 2010-2018 Apple Inc. All rights reserved.
    22#
    33# Redistribution and use in source and binary forms, with or without
     
    129129    MessagesAvailableForPort(struct WebCore::MessagePortIdentifier port)
    130130
    131 #if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
     131#if PLATFORM(MAC)
    132132    SetScreenProperties(HashMap<uint32_t, WebCore::ScreenProperties> screenProperties)
     133#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
    133134    ScrollerStylePreferenceChanged(bool useOvelayScrollbars)
    134135#endif
     136#endif
    135137}
Note: See TracChangeset for help on using the changeset viewer.