<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:mcatanzaro@igalia.com" title="Michael Catanzaro <mcatanzaro@igalia.com>"> <span class="fn">Michael Catanzaro</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK] Add API to WebKitWebsiteDataManager to handle cached data"
href="https://bugs.webkit.org/show_bug.cgi?id=146589">bug 146589</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #257641 Flags</td>
<td>review?
</td>
<td>review+
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK] Add API to WebKitWebsiteDataManager to handle cached data"
href="https://bugs.webkit.org/show_bug.cgi?id=146589#c13">Comment # 13</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK] Add API to WebKitWebsiteDataManager to handle cached data"
href="https://bugs.webkit.org/show_bug.cgi?id=146589">bug 146589</a>
from <span class="vcard"><a class="email" href="mailto:mcatanzaro@igalia.com" title="Michael Catanzaro <mcatanzaro@igalia.com>"> <span class="fn">Michael Catanzaro</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=257641&action=diff" name="attach_257641" title="Updated patch">attachment 257641</a> <a href="attachment.cgi?id=257641&action=edit" title="Updated patch">[details]</a></span>
Updated patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=257641&action=review">https://bugs.webkit.org/attachment.cgi?id=257641&action=review</a>
You'll need to update all the Since tags, obviously.
And it'd be nice to see implementation in Ephy before landing, since it's not great to add API that no apps are using, but I guess the MB implementation is enough to show the API is sufficient for the task.
<span class="quote">> Source/WebKit2/UIProcess/API/gtk/WebKitSecurityOrigin.cpp:46
> +struct _WebKitSecurityOrigin {
> + _WebKitSecurityOrigin(WebCore::SecurityOrigin* coreSecurityOrigin)
> + : securityOrigin(coreSecurityOrigin)
> + {
> + }</span >
Why is it useful to create WebKitSecurityOrigin objects with a null WebCore::SecurityOrigin? I would have expected the constructor to receive a reference, and to use a Ref rather than RefPtr member variable.
<span class="quote">> Source/WebKit2/UIProcess/API/gtk/WebKitSecurityOrigin.cpp:79
> +WebCore::SecurityOrigin& webkitSecurityOriginGetSecurityOrigin(WebKitSecurityOrigin* origin)
> +{
> + ASSERT(origin);
> + return *origin->securityOrigin;
> +}</span >
This ASSERT indicates to me that allowing null WebCore::SecurityOrigins is probably not appropriate.
<span class="quote">> Source/WebKit2/UIProcess/API/gtk/WebKitWebsiteDataManager.cpp:652
> + * Note that this is not supported for #WebKitWebContext<!-- -->s using
> + * %WEBKIT_PROCESS_MODEL_SHARED_SECONDARY_PROCESS process model; an empty list will
> + * always be returned in such case.</span >
Were we still using the soup cache in shared secondary process mode when you wrote this patch? Hopefully this comment is obsolete and should be removed?
<span class="quote">> Tools/MiniBrowser/gtk/main.c:396
> + WebKitWebsiteDataManager* manager = webkit_web_context_get_website_data_manager(webkit_web_context_get_default());</span >
WebKitWebsiteDataManager *manager
<span class="quote">> Tools/MiniBrowser/gtk/main.c:424
> + AboutDataRequest *dataRequest = aboutDataRequestNew(request);
> + WebKitWebsiteDataManager* manager = webkit_web_context_get_website_data_manager(webkit_web_context_get_default());</span >
WebKitWebsiteDataManager *manager
<span class="quote">> Tools/MiniBrowser/gtk/main.c:493
> + WebKitUserContentManager *userContentManager = webkit_user_content_manager_new();</span >
Looks like it's leaked?
<span class="quote">> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebContext.cpp:736
> + // Disk cache delays the storing of initial resources for 1 second to avoid
> + // affecting early page load. So, wait 1 second here to make sure resources
> + // have already been stored.
> + test->wait(1);</span >
This is unfortunate.
<span class="quote">> Tools/TestWebKitAPI/gtk/WebKit2Gtk/WebViewTest.cpp:42
> +WebViewTest::WebViewTest(WebKitProcessModel processModel)
> + : Test(processModel)
> + , m_webView(WEBKIT_WEB_VIEW(g_object_ref_sink(webkit_web_view_new_with_context(m_webContext.get()))))
> + , m_mainLoop(g_main_loop_new(nullptr, TRUE))
> {
> assertObjectIsDeletedWhenTestFinishes(G_OBJECT(m_webView));
> g_signal_connect(m_webView, "web-process-crashed", G_CALLBACK(WebViewTest::webProcessCrashed), this);</span >
This is not good. Use delegating constructors to avoid the need to reimplement the constructor body. Make sure there is a constructor that can take both a WebKitUserContentManager and a WebKitProcessModel.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>