Alex Plaskett Reporter
	Description • 6 years ago

The plugin-container content processes will never terminate the connection to the WindowServer using CGSSetDenyWindowServerConnections and CGSShutdownServerConnections. 

XRE_UseNativeEventProcessing will always return true as the default preference dom.ipc.useNativeEventProcessing.content is set to true by default on macOS (https://searchfox.org/mozilla-central/source/modules/libpref/init/all.js#3218).

We can also confirm this under lldb, attached to a plugin content process by printing the value of deny_future_windowserver_connections, which will always be 0. 

It is recommended this is reviewed.  

Annotated code as follows:

https://searchfox.org/mozilla-central/source/dom/ipc/ContentChild.cpp#1612

static bool
StartMacOSContentSandbox()
{
  int sandboxLevel = GetEffectiveContentSandboxLevel();
  if (sandboxLevel < 1) {
    return false;
  }

  if (!XRE_UseNativeEventProcessing()) {
    // If we've opened a connection to the window server, shut it down now. Forbid
    // future connections as well. We do this for sandboxing, but it also ensures
    // that the Activity Monitor will not label the content process as "Not
    // responding" because it's not running a native event loop. See bug 1384336.
    CGSSetDenyWindowServerConnections(true);
    CGSShutdownServerConnections();
  }

bool XRE_UseNativeEventProcessing()
{
  if (XRE_IsContentProcess()) {                                                                               // In the case of the content process
    static bool sInited = false;
    static bool sUseNativeEventProcessing = false;
    if (!sInited) {
      Preferences::AddBoolVarCache(&sUseNativeEventProcessing,
                                   "dom.ipc.useNativeEventProcessing.content");                               // This pulls back existing true cache value if its not already set.
      sInited = true;
    }

    return sUseNativeEventProcessing;                                                                         // Will return true here as well based on default pref value.
  }

  return true;                                                                                            
}


//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

  // Adds the aVariable to cache table. |aVariable| must be a pointer for a
  // static variable. The value will be modified when the pref value is changed
  // but note that even if you modified it, the value isn't assigned to the
  // pref.
  static nsresult AddBoolVarCache(bool* aVariable,
                                  const char* aPref,
                                  bool aDefault = false,
                                  bool aSkipAssignment = false);


 /* static */ nsresult
Preferences::AddBoolVarCache(bool* aCache,
                             const char* aPref,
                             bool aDefault,
                             bool aSkipAssignment)
{
  AssertNotAlreadyCached("bool", aPref, aCache);
  if (!aSkipAssignment) {                                               // aSkipAssignment is false by default 
    *aCache = GetBool(aPref, aDefault);                                 // GetBool retrieves true and sets sUseNativeEventProcessing to true. 
  }
  CacheData* data = new CacheData();
  data->mCacheLocation = aCache;
  data->mDefaultValueBool = aDefault;
  CacheDataAppendElement(data);
  Preferences::RegisterCallback(BoolVarChanged,
                                aPref,
                                data,
                                Preferences::ExactMatch,
                                /* isPriority */ true);
  return NS_OK;
}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

	Alex Gaynor [:Alex_Gaynor]
	Comment 2 • 6 years ago

To summarize the results of the research in bug 1426100, and the state of this bug:

Originally we thought the blocker to disconnecting the windowserver was removing native event processing (which needed some performance work to be landable). However, on implementing that and digging in, I found that windowserver connections were still hanging around.

Disassembly of the macOS system libraries showed that we (along with every other browser) were misusing the (undocumented, private) disconnect-windowserver APIs. Correcting our usage of them produced (a) a browser with no windowserver connections, (b) a browser where OpenGL didn't work.

Digging into the OpenGL code I found that many of these APIs require a windowserver connection. Therefore disconnecting from the windowserver will require WebGL's OpenGL interactions to be remoted.

Therefore the route to go is (a) land the removal of native event processing without touching the windowserver code (this is what the latest patch on bug 1426100 does), (b) once the WebGL remoting is completed, turn off the windowserver connection.

(Both Chromium and WebKit were notified that they had the same bug as we did with disconnecting from the windowserver.)

	Pulsebot
	Comment 9 • 3 years ago

Pushed by haftandilian@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5aaf8123b97a
WindowServer connection never being terminated r=spohl