Bug 135035 – Clean up WKOriginDataManager and get it messaging to the DatabaseProcess

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brady Eidson]

Clean up WKOriginDataManager and get it messaging to the DatabaseProcess

First functional step of https://bugs.webkit.org/show_bug.cgi?id=135029

[Brady Eidson]

Created attachment 235104 [details]
Patch v1

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.h:57:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.h:58:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:111:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:144:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 4 in 18 files


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

[Brady Eidson]

(In reply to comment #2)
> Attachment 235104 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.h:57:  Extra space before ( in function call  [whitespace/parens] [4]
> ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.h:58:  Extra space before ( in function call  [whitespace/parens] [4]
> ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:111:  Extra space before ( in function call  [whitespace/parens] [4]
> ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:144:  Extra space before ( in function call  [whitespace/parens] [4]
> Total errors found: 4 in 18 files
> 
> 

These are the same old "check-webkit-style doesn't understand our style for C++ lambdas" which has been filed already

[Brady Eidson]

Created attachment 235143 [details]
Patch v2 - Also added a new API we'll need very soon anyways

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.h:57:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.h:58:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.h:59:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:111:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:138:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:171:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 6 in 18 files


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

[Brady Eidson]

Comment on attachment 235143 [details]
Patch v2 - Also added a new API we'll need very soon anyways

NM this, v3 coming soon

[Brady Eidson]

Created attachment 235145 [details]
Patch v3 - Okay for real this time

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.h:57:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.h:58:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.h:59:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:111:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:138:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:166:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 6 in 18 files


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

[Brady Eidson]

Comment on attachment 235145 [details]
Patch v3 - Okay for real this time

Nevermind...  one more change.  *sigh*

[Brady Eidson]

NM, yes let's do it this way for now.

[Tim Horton]

Comment on attachment 235145 [details]
Patch v3 - Okay for real this time

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

> Source/WebKit2/DatabaseProcess/DatabaseProcess.cpp:55
> +    m_webOriginDataManager = std::make_unique<WebOriginDataManager>(this);

why not in the initializer list?

> Source/WebKit2/DatabaseProcess/DatabaseProcess.messages.in:25
> +messages -> DatabaseProcess LegacyReceiver {

why are we adding *new* "Legacy" message receivers?

[Brady Eidson]

(In reply to comment #11)
> (From update of attachment 235145 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=235145&action=review
> 
> > Source/WebKit2/DatabaseProcess/DatabaseProcess.cpp:55
> > +    m_webOriginDataManager = std::make_unique<WebOriginDataManager>(this);
> 
> why not in the initializer list?

Leftover from an earlier iteration where it wasn't feasible for some reason.  Will fix.

> > Source/WebKit2/DatabaseProcess/DatabaseProcess.messages.in:25
> > +messages -> DatabaseProcess LegacyReceiver {
> 
> why are we adding *new* "Legacy" message receivers?

It appears that the only way to get your ChildProcessSupplements to have a chance at handling messages is to be a LegacyReceiver and have your custom didReceiveMessage check the messageReceiverMap() first.

This is one reason why I really really want Anders to review this patch, because I can find no alternative, but if there is one he'd know.

[Sam Weinig]

Comment on attachment 235145 [details]
Patch v3 - Okay for real this time

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

> Source/WebKit2/UIProcess/API/C/WKOriginDataManager.h:-78
> -typedef struct WKOriginDataManagerChangeClient {
> -    int                                                                 version;
> -    const void *                                                        clientInfo;
> -
> -    // Version 0.
> -    WKOriginDataManagerChangeCallback                                   didChange;
> -} WKOriginDataManagerChangeClient WK_DEPRECATED("Use an explicit versioned struct instead");

Why is it okay to get rid of this client?

[Brady Eidson]

(In reply to comment #13)
> (From update of attachment 235145 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=235145&action=review
> 
> > Source/WebKit2/UIProcess/API/C/WKOriginDataManager.h:-78
> > -typedef struct WKOriginDataManagerChangeClient {
> > -    int                                                                 version;
> > -    const void *                                                        clientInfo;
> > -
> > -    // Version 0.
> > -    WKOriginDataManagerChangeCallback                                   didChange;
> > -} WKOriginDataManagerChangeClient WK_DEPRECATED("Use an explicit versioned struct instead");
> 
> Why is it okay to get rid of this client?

It was never used outside the project, and we're not positive it needs to be around.
It was never even used within the project - just a big no-op.

It clutters up things right now, and can be brought back whenever it's actually needed and someone has time to make it actually do something.

[Brady Eidson]

http://trac.webkit.org/changeset/171622