Bug 184322 – Add ProcessPrivilege assertions to places that access NSApp

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Brent Fulgham]

Add new assertions to warn us if powerful API calls are used in the WbContent process.

We have refactored the WebContent code to avoid calling into these routines, but we need to guard against re-introducing them in the future.

[Radar WebKit Bug Importer]

<rdar://problem/39194560>

[Brent Fulgham]

Created attachment 337244 [details]
Patch

[EWS Watchlist]

Comment on attachment 337244 [details]
Patch

Attachment 337244 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/7210687

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 337250 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Brent Fulgham]

Created attachment 337276 [details]
Patch

[Brent Fulgham]

Created attachment 337559 [details]
Patch (Rebased to current)

[Per Arne Vollan]

Comment on attachment 337559 [details]
Patch (Rebased to current)

View in context: https://bugs.webkit.org/attachment.cgi?id=337559&action=review

R=me.

> Source/WebCore/platform/mac/PasteboardMac.mm:678
> +    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));

I believe the method Pasteboard::setDragImage is still called from WebContent process. I think we can move this assert into the if statement.

[Brent Fulgham]

Comment on attachment 337559 [details]
Patch (Rebased to current)

View in context: https://bugs.webkit.org/attachment.cgi?id=337559&action=review

>> Source/WebCore/platform/mac/PasteboardMac.mm:678
>> +    RELEASE_ASSERT(hasProcessPrivilege(ProcessPrivilege::CanCommunicateWithWindowServer));
> 
> I believe the method Pasteboard::setDragImage is still called from WebContent process. I think we can move this assert into the if statement.

OK!

[Brent Fulgham]

Committed r230468: <https://trac.webkit.org/changeset/230468>

[Saam Barati]

It seems like this change caused a 3% Kraken regression on Mac.

[Brent Fulgham]

(In reply to Saam Barati from comment #10)
> It seems like this change caused a 3% Kraken regression on Mac.

Oh no! Please roll it out!

Is it possible to identify which code paths are contributing to the slowdown?

I can relays with normal DEBUG assertions.

[Brent Fulgham]

Weakening some of these RELEASE_ASSERTS on hot code paths to normal ASSERTS:

Committed r230512: <https://trac.webkit.org/changeset/230512/webkit>

[Brent Fulgham]

Rolled out a change that still depends on Bug 184451:

Committed r230514: <https://trac.webkit.org/changeset/230514/webkit>