Bug 173472 - [WK2] Add WKProcessPool SPI to efficiently reset all plugin load client policies
Summary: [WK2] Add WKProcessPool SPI to efficiently reset all plugin load client policies
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks: 173689 173721
  Show dependency treegraph
 
Reported: 2017-06-16 09:05 PDT by Chris Dumez
Modified: 2017-06-22 09:41 PDT (History)
9 users (show)

See Also:


Attachments
Patch (14.90 KB, patch)
2017-06-16 09:13 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (14.98 KB, text/plain)
2017-06-16 09:48 PDT, Chris Dumez
no flags Details
Patch (15.73 KB, patch)
2017-06-16 10:34 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (15.75 KB, patch)
2017-06-16 10:35 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (28.84 KB, patch)
2017-06-16 13:57 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (28.98 KB, patch)
2017-06-16 14:27 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (28.90 KB, patch)
2017-06-16 14:36 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Follow-up improvement (3.46 KB, patch)
2017-06-16 18:11 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2017-06-16 09:05:58 PDT
Add C API to efficiently reset all plugin load client policies.
Comment 1 Chris Dumez 2017-06-16 09:06:07 PDT
<rdar://problem/28858817>
Comment 2 Chris Dumez 2017-06-16 09:13:44 PDT
Created attachment 313078 [details]
Patch
Comment 3 Build Bot 2017-06-16 09:15:15 PDT
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.
Comment 4 Sam Weinig 2017-06-16 09:39:39 PDT
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.
Comment 5 Chris Dumez 2017-06-16 09:45:08 PDT
(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?
Comment 6 Chris Dumez 2017-06-16 09:47:31 PDT
(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?
Comment 7 Chris Dumez 2017-06-16 09:48:00 PDT
Created attachment 313082 [details]
Patch
Comment 8 Build Bot 2017-06-16 09:50:42 PDT
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.
Comment 9 Chris Dumez 2017-06-16 09:52:22 PDT
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.
Comment 10 Chris Dumez 2017-06-16 10:34:22 PDT
Created attachment 313085 [details]
Patch
Comment 11 Chris Dumez 2017-06-16 10:35:18 PDT
Created attachment 313086 [details]
Patch
Comment 12 Chris Dumez 2017-06-16 10:35:44 PDT
(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.
Comment 13 Build Bot 2017-06-16 10:38:00 PDT
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.
Comment 14 Brady Eidson 2017-06-16 11:50:43 PDT
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?
Comment 15 Chris Dumez 2017-06-16 11:56:21 PDT
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.
Comment 16 Chris Dumez 2017-06-16 13:57:08 PDT
Created attachment 313126 [details]
Patch
Comment 17 Chris Dumez 2017-06-16 13:57:21 PDT
I added an API test.
Comment 18 Build Bot 2017-06-16 13:59:49 PDT
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.
Comment 19 Chris Dumez 2017-06-16 14:27:59 PDT
Created attachment 313135 [details]
Patch
Comment 20 Build Bot 2017-06-16 14:30:07 PDT
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.
Comment 21 Chris Dumez 2017-06-16 14:36:47 PDT
Created attachment 313139 [details]
Patch
Comment 22 Build Bot 2017-06-16 14:39:20 PDT
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.
Comment 23 Chris Dumez 2017-06-16 15:32:39 PDT
Comment on attachment 313139 [details]
Patch

Clearing flags on attachment: 313139

Committed r218419: <http://trac.webkit.org/changeset/218419>
Comment 24 Chris Dumez 2017-06-16 15:32:41 PDT
All reviewed patches have been landed.  Closing bug.
Comment 25 Chris Dumez 2017-06-16 18:10:25 PDT
Reopen to land follow-up improvement.
Comment 26 Chris Dumez 2017-06-16 18:11:43 PDT
Created attachment 313170 [details]
Follow-up improvement
Comment 27 WebKit Commit Bot 2017-06-19 11:22:02 PDT
Comment on attachment 313170 [details]
Follow-up improvement

Clearing flags on attachment: 313170

Committed r218499: <http://trac.webkit.org/changeset/218499>
Comment 28 WebKit Commit Bot 2017-06-19 11:22:04 PDT
All reviewed patches have been landed.  Closing bug.