RESOLVED FIXED 173472
[WK2] Add WKProcessPool SPI to efficiently reset all plugin load client policies
https://bugs.webkit.org/show_bug.cgi?id=173472
Summary [WK2] Add WKProcessPool SPI to efficiently reset all plugin load client policies
Chris Dumez
Reported 2017-06-16 09:05:58 PDT
Add C API to efficiently reset all plugin load client policies.
Attachments
Patch (14.90 KB, patch)
2017-06-16 09:13 PDT, Chris Dumez
no flags
Patch (14.98 KB, text/plain)
2017-06-16 09:48 PDT, Chris Dumez
no flags
Patch (15.73 KB, patch)
2017-06-16 10:34 PDT, Chris Dumez
no flags
Patch (15.75 KB, patch)
2017-06-16 10:35 PDT, Chris Dumez
no flags
Patch (28.84 KB, patch)
2017-06-16 13:57 PDT, Chris Dumez
no flags
Patch (28.98 KB, patch)
2017-06-16 14:27 PDT, Chris Dumez
no flags
Patch (28.90 KB, patch)
2017-06-16 14:36 PDT, Chris Dumez
no flags
Follow-up improvement (3.46 KB, patch)
2017-06-16 18:11 PDT, Chris Dumez
no flags
Chris Dumez
Comment 1 2017-06-16 09:06:07 PDT
Chris Dumez
Comment 2 2017-06-16 09:13:44 PDT
Build Bot
Comment 3 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.
Sam Weinig
Comment 4 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.
Chris Dumez
Comment 5 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?
Chris Dumez
Comment 6 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?
Chris Dumez
Comment 7 2017-06-16 09:48:00 PDT
Build Bot
Comment 8 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.
Chris Dumez
Comment 9 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.
Chris Dumez
Comment 10 2017-06-16 10:34:22 PDT
Chris Dumez
Comment 11 2017-06-16 10:35:18 PDT
Chris Dumez
Comment 12 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.
Build Bot
Comment 13 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.
Brady Eidson
Comment 14 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?
Chris Dumez
Comment 15 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.
Chris Dumez
Comment 16 2017-06-16 13:57:08 PDT
Chris Dumez
Comment 17 2017-06-16 13:57:21 PDT
I added an API test.
Build Bot
Comment 18 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.
Chris Dumez
Comment 19 2017-06-16 14:27:59 PDT
Build Bot
Comment 20 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.
Chris Dumez
Comment 21 2017-06-16 14:36:47 PDT
Build Bot
Comment 22 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.
Chris Dumez
Comment 23 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>
Chris Dumez
Comment 24 2017-06-16 15:32:41 PDT
All reviewed patches have been landed. Closing bug.
Chris Dumez
Comment 25 2017-06-16 18:10:25 PDT
Reopen to land follow-up improvement.
Chris Dumez
Comment 26 2017-06-16 18:11:43 PDT
Created attachment 313170 [details] Follow-up improvement
WebKit Commit Bot
Comment 27 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>
WebKit Commit Bot
Comment 28 2017-06-19 11:22:04 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.