Bug 92484 - [EFL][WK2] Add API to Ewk_Cookie_Manager to watch for cookie changes
Summary: [EFL][WK2] Add API to Ewk_Cookie_Manager to watch for cookie changes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks: 61838
  Show dependency treegraph
 
Reported: 2012-07-27 03:32 PDT by Chris Dumez
Modified: 2012-07-27 06:40 PDT (History)
9 users (show)

See Also:


Attachments
Patch (11.14 KB, patch)
2012-07-27 03:38 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (11.22 KB, patch)
2012-07-27 05:45 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 2012-07-27 03:32:51 PDT
There is currently no way for the client to get notified whenever cookies are added, modified or removed.
We need to provide such functionality in Ewk_Cookie_Manager.
Comment 1 Chris Dumez 2012-07-27 03:38:38 PDT
Created attachment 154895 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2012-07-27 04:47:34 PDT
Comment on attachment 154895 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/ewk_cookie_manager.cpp:44
> +struct Watch_Changes_Callback_Data {
> +    Ewk_Cookie_Manager_Changes_Watch_Cb callback;

I dont think the Data here is such a good addition given that it is containing an actual callback. Is this stuct even needed. It seems as you can only have one callback with user data

> Source/WebKit2/UIProcess/API/efl/ewk_cookie_manager.cpp:66
> +    Watch_Changes_Callback_Data changeWatcher;

The naming seems a bit weird here given the name of the variable and the name of the struct. A watcher indicates something active
Comment 3 Chris Dumez 2012-07-27 05:33:48 PDT
Comment on attachment 154895 [details]
Patch

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

>> Source/WebKit2/UIProcess/API/efl/ewk_cookie_manager.cpp:44
>> +    Ewk_Cookie_Manager_Changes_Watch_Cb callback;
> 
> I dont think the Data here is such a good addition given that it is containing an actual callback. Is this stuct even needed. It seems as you can only have one callback with user data

I would prefer to keep the struct since this is the approach I have used in order files for callbacks + userData. I think it is more scalable.
How about renaming _Data with _Handler then?

>> Source/WebKit2/UIProcess/API/efl/ewk_cookie_manager.cpp:66
>> +    Watch_Changes_Callback_Data changeWatcher;
> 
> The naming seems a bit weird here given the name of the variable and the name of the struct. A watcher indicates something active

You're right. How about changeHandler then?
Comment 4 Kenneth Rohde Christiansen 2012-07-27 05:40:10 PDT
ok fine
Comment 5 Chris Dumez 2012-07-27 05:45:47 PDT
Created attachment 154913 [details]
Patch

Take Kenneth's feedback into consideration.
Comment 6 WebKit Review Bot 2012-07-27 06:40:01 PDT
Comment on attachment 154913 [details]
Patch

Clearing flags on attachment: 154913

Committed r123870: <http://trac.webkit.org/changeset/123870>
Comment 7 WebKit Review Bot 2012-07-27 06:40:08 PDT
All reviewed patches have been landed.  Closing bug.