Bug 173472 – [WK2] Add WKProcessPool SPI to efficiently reset all plugin load client policies

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

Add C API to efficiently reset all plugin load client policies.

[Chris Dumez]

<rdar://problem/28858817>

[Chris Dumez]

Created attachment 313078 [details]
Patch

[Build Bot]

Attachment 313078 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/WebProcessPool.cpp:1591:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit2/UIProcess/WebProcessPool.cpp:1592:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Sam Weinig]

Comment on attachment 313078 [details]
Patch

Why not make this SPI on WKProcessPool.  Given that we want the C-SPI to go away over time, and that you can cast a WKContextRef to a WKProcessPool, I don't see the value of adding new C-SPI here.

[Chris Dumez]

(In reply to Sam Weinig from comment #4)
> Comment on attachment 313078 [details]
> Patch
> 
> Why not make this SPI on WKProcessPool.  Given that we want the C-SPI to go
> away over time, and that you can cast a WKContextRef to a WKProcessPool, I
> don't see the value of adding new C-SPI here.

Oh, I guess the location does not matter to me. I was merely being consistent with all the other PluginLoadPolicy API is in WKContextPrivateMac.h. Wouldn't it look weird if 3 of them were in WKContextPrivateMac.h and one in WKProcessPool?

[Chris Dumez]

(In reply to Chris Dumez from comment #5)
> (In reply to Sam Weinig from comment #4)
> > Comment on attachment 313078 [details]
> > Patch
> > 
> > Why not make this SPI on WKProcessPool.  Given that we want the C-SPI to go
> > away over time, and that you can cast a WKContextRef to a WKProcessPool, I
> > don't see the value of adding new C-SPI here.
> 
> Oh, I guess the location does not matter to me. I was merely being
> consistent with all the other PluginLoadPolicy API is in
> WKContextPrivateMac.h. Wouldn't it look weird if 3 of them were in
> WKContextPrivateMac.h and one in WKProcessPool?

Also, it seems WKProcessPool is Cocoa, not C API. Wouldn't this make it much harder to adopt the new API in C API clients?

[Chris Dumez]

Created attachment 313082 [details]
Patch

[Build Bot]

Attachment 313082 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/WebProcessPool.cpp:1591:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit2/UIProcess/WebProcessPool.cpp:1592:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Comment on attachment 313082 [details]
Patch

Oh, I missed the part about being to cast from one to the other. I'll give it a shot.

[Chris Dumez]

Created attachment 313085 [details]
Patch

[Chris Dumez]

Created attachment 313086 [details]
Patch

[Chris Dumez]

(In reply to Sam Weinig from comment #4)
> Comment on attachment 313078 [details]
> Patch
> 
> Why not make this SPI on WKProcessPool.  Given that we want the C-SPI to go
> away over time, and that you can cast a WKContextRef to a WKProcessPool, I
> don't see the value of adding new C-SPI here.

Done in this latest iteration, thanks.

[Build Bot]

Attachment 313086 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/WebProcessPool.cpp:1591:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit2/UIProcess/WebProcessPool.cpp:1592:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brady Eidson]

Comment on attachment 313086 [details]
Patch

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

No API test?

> Source/WebKit2/UIProcess/WebProcessPool.cpp:1610
> +    sendToAllProcesses(Messages::WebProcess::ResetPluginLoadClientPolicies(pluginLoadClientPolicies));

pluginLoadClientPolicies ?  Or m_pluginLoadClientPolicies?

[Chris Dumez]

Comment on attachment 313086 [details]
Patch

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

I will look into adding an API test.

>> Source/WebKit2/UIProcess/WebProcessPool.cpp:1610
>> +    sendToAllProcesses(Messages::WebProcess::ResetPluginLoadClientPolicies(pluginLoadClientPolicies));
> 
> pluginLoadClientPolicies ?  Or m_pluginLoadClientPolicies?

Should be identical at this point.

[Chris Dumez]

Created attachment 313126 [details]
Patch

[Chris Dumez]

I added an API test.

[Build Bot]

Attachment 313126 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/WebProcessPool.cpp:1591:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit2/UIProcess/WebProcessPool.cpp:1592:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKProcessPool.mm:342:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 3 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 313135 [details]
Patch

[Build Bot]

Attachment 313135 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/WebProcessPool.cpp:1591:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit2/UIProcess/WebProcessPool.cpp:1592:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKProcessPool.mm:342:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 3 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 313139 [details]
Patch

[Build Bot]

Attachment 313139 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/WebProcessPool.cpp:1591:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit2/UIProcess/WebProcessPool.cpp:1592:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKProcessPool.mm:342:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 3 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Comment on attachment 313139 [details]
Patch

Clearing flags on attachment: 313139

Committed r218419: <http://trac.webkit.org/changeset/218419>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

Reopen to land follow-up improvement.

[Chris Dumez]

Created attachment 313170 [details]
Follow-up improvement

[WebKit Commit Bot]

Comment on attachment 313170 [details]
Follow-up improvement

Clearing flags on attachment: 313170

Committed r218499: <http://trac.webkit.org/changeset/218499>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.