Summary: | [WK2] Add WKProcessPool SPI to efficiently reset all plugin load client policies | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||||||||||
Component: | WebKit2 | Assignee: | Chris Dumez <cdumez> | ||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||
Severity: | Normal | CC: | achristensen, andersca, beidson, buildbot, commit-queue, darin, ggaren, koivisto, sam | ||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||
Bug Blocks: | 173689, 173721 | ||||||||||||||||||||
Attachments: |
|
Description
Chris Dumez
2017-06-16 09:05:58 PDT
Created attachment 313078 [details]
Patch
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 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.
(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? (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? Created attachment 313082 [details]
Patch
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 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.
Created attachment 313085 [details]
Patch
Created attachment 313086 [details]
Patch
(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. 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 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 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. Created attachment 313126 [details]
Patch
I added an API test. 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.
Created attachment 313135 [details]
Patch
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.
Created attachment 313139 [details]
Patch
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 on attachment 313139 [details] Patch Clearing flags on attachment: 313139 Committed r218419: <http://trac.webkit.org/changeset/218419> All reviewed patches have been landed. Closing bug. Reopen to land follow-up improvement. Created attachment 313170 [details]
Follow-up improvement
Comment on attachment 313170 [details] Follow-up improvement Clearing flags on attachment: 313170 Committed r218499: <http://trac.webkit.org/changeset/218499> All reviewed patches have been landed. Closing bug. |