Bug 135035 - Clean up WKOriginDataManager and get it messaging to the DatabaseProcess
Summary: Clean up WKOriginDataManager and get it messaging to the DatabaseProcess
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks: 135029
  Show dependency treegraph
 
Reported: 2014-07-17 18:05 PDT by Brady Eidson
Modified: 2014-07-25 15:35 PDT (History)
6 users (show)

See Also:


Attachments
Patch v1 (41.85 KB, patch)
2014-07-17 18:10 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch v2 - Also added a new API we'll need very soon anyways (45.05 KB, patch)
2014-07-18 13:27 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch v3 - Okay for real this time (44.44 KB, patch)
2014-07-18 13:46 PDT, Brady Eidson
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2014-07-17 18:05:18 PDT
Clean up WKOriginDataManager and get it messaging to the DatabaseProcess

First functional step of https://bugs.webkit.org/show_bug.cgi?id=135029
Comment 1 Brady Eidson 2014-07-17 18:10:46 PDT
Created attachment 235104 [details]
Patch v1
Comment 2 WebKit Commit Bot 2014-07-17 18:12:18 PDT
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.
Comment 3 Brady Eidson 2014-07-18 09:42:46 PDT
(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
Comment 4 Brady Eidson 2014-07-18 13:27:34 PDT
Created attachment 235143 [details]
Patch v2 - Also added a new API we'll need very soon anyways
Comment 5 WebKit Commit Bot 2014-07-18 13:29:33 PDT
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.
Comment 6 Brady Eidson 2014-07-18 13:32:51 PDT
Comment on attachment 235143 [details]
Patch v2 - Also added a new API we'll need very soon anyways

NM this, v3 coming soon
Comment 7 Brady Eidson 2014-07-18 13:46:11 PDT
Created attachment 235145 [details]
Patch v3 - Okay for real this time
Comment 8 WebKit Commit Bot 2014-07-18 13:47:26 PDT
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.
Comment 9 Brady Eidson 2014-07-18 16:16:07 PDT
Comment on attachment 235145 [details]
Patch v3 - Okay for real this time

Nevermind...  one more change.  *sigh*
Comment 10 Brady Eidson 2014-07-18 16:50:35 PDT
NM, yes let's do it this way for now.
Comment 11 Tim Horton 2014-07-24 14:44:50 PDT
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?
Comment 12 Brady Eidson 2014-07-24 15:45:14 PDT
(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.
Comment 13 Sam Weinig 2014-07-25 13:55:17 PDT
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?
Comment 14 Brady Eidson 2014-07-25 14:22:56 PDT
(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.
Comment 15 Brady Eidson 2014-07-25 15:35:58 PDT
http://trac.webkit.org/changeset/171622