Bug 124331 – Implement WebOriginDataManager

• Component: WebKit2
• Status: RESOLVED DUPLICATE

[Martin Hock]

Continuation of 122781.

Implements the functionality in WebOriginDataManager and convert the WK*Manager C APIs to use WebOriginDataManager, where * is one of:
ApplicationCache, Cookie, Database, KeyValueStorage, MediaCache, PluginSiteData, ResourceCache.

[Martin Hock]

Created attachment 216962 [details]
patch

[EFL EWS Bot]

Comment on attachment 216962 [details]
patch

Attachment 216962 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/23608039

[Build Bot]

Comment on attachment 216962 [details]
patch

Attachment 216962 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/23748056

New failing tests:
http/tests/cookies/third-party-cookie-relaxing.html

[Build Bot]

Created attachment 216978 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Martin Hock]

Created attachment 217011 [details]
fix InjectedBundle + build

[EFL EWS Bot]

Comment on attachment 217011 [details]
fix InjectedBundle + build

Attachment 217011 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/23768005

[Martin Hock]

Created attachment 217056 [details]
fix build

[Martin Hock]

Created attachment 217062 [details]
merge

[EFL EWS Bot]

Comment on attachment 217062 [details]
merge

Attachment 217062 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/23698189

[Tim Horton]

Comment on attachment 217062 [details]
merge

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

Wayyyyyyy too big for me to make heads or tails out of, so I just glanced through.

> Source/WebKit2/Shared/SecurityOriginDataHash.h:4
> + * Redistribution and use in source and binary forms, with or without

This looks like the old license. Unless this file is copied from somewhere and I haven't gotten there yet?

> Source/WebKit2/Shared/SecurityOriginDataHash.h:101
> +

extra space

> Source/WebKit2/Shared/StoredSecurityOriginData.cpp:4
> + * Redistribution and use in source and binary forms, with or without

this is the right license

> Source/WebKit2/Shared/StoredSecurityOriginData.cpp:96
> +    if (!callback) {
> +        // FIXME: Log error or assert.
> +        return;
> +    }

ASSERT(callback)?

> Source/WebKit2/Shared/WebStoredSecurityOrigin.h:66
> +    // FIXME: concat types?

? more explanation/maybe a bugzilla link in the fixme? I don't know what this means, as it stands.

> Source/WebKit2/Shared/WebStoredSecurityOrigin.h:73
> +        : m_securityOrigin(securityOrigin), m_types(types)

separate lines, comma leading

> Source/WebKit2/Shared/WebStoredSecurityOrigin.h:79
> +

extra space

> Source/WebKit2/UIProcess/API/C/WKContext.cpp:217
> +    return reinterpret_cast<WKDatabaseManagerRef>(toAPI(toImpl(contextRef)->supplement<WebOriginDataManagerProxy>()));

scariest_cast

> Source/WebKit2/UIProcess/API/C/WKCookieManager.cpp:48
> +    toImpl(originDataManager)->getOrigins(kWKCookieOriginData, ArrayCallback::create(ArrayCallback::create(context, callback).leakRef(), WKOriginDataManagerStoredOriginsToHostnames));

leakRef!

> Source/WebKit2/UIProcess/API/C/WKOriginDataManager.cpp:78
> +    API::Array *array = toImpl(arrayRef);

Star on the wrong side.

> Source/WebKit2/UIProcess/API/C/WKOriginDataManager.cpp:79
> +    RefPtr<ArrayCallback> cb = adoptRef(reinterpret_cast<ArrayCallback *>(callback));

Extra space before the star.

> Source/WebKit2/UIProcess/API/C/WKOriginDataManager.cpp:84
> +        origins[i] = WebSecurityOrigin::create(reinterpret_cast<WebStoredSecurityOrigin *>(array->at(i))->securityOrigin());

ditto (and so on, I assume you can apply this where necessary)

> Source/WebKit2/UIProcess/Plugins/WebPluginSiteDataManager.cpp:60
> +        // FIXME: functionality moved to WebOriginDataManager
> +        // PluginProcessManager::shared().getSitesWithData(m_plugins.last(), m_webPluginSiteDataManager, m_callbackID);

Wut.

> Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:100
> +        // setting flags to 0 clears all data
> +        // setting maxAge to max int means no max age
> +        // setting callback to 0 sets an empty callback

http://www.webkit.org/coding/coding-style.html#comments-sentences

though these are kind of odd to begin with

> Source/WebKit2/UIProcess/WebOriginDataManagerProxy.cpp:227
> +        for (auto i = origins->begin(), end = origins->end(); i != end; ++i)

use range-based-for, maybe? and some other places (like right above)

> Source/WebKit2/WebProcess/OriginData/WebOriginDataManager.cpp:195
> +        UNUSED_PARAM(hostname);

this doesn't look right anymore.

[Martin Hock]

Comment on attachment 217062 [details]
merge

Thanks for the review.  I created 124617 to break this patch up into (somewhat) smaller pieces, and I have tried to implement the suggestions there.

[Martin Hock]

*** This bug has been marked as a duplicate of bug 124617 ***