[Webkit-unassigned] [Bug 146589] [GTK] Add API to WebKitWebsiteDataManager to handle cached data

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=146589

Michael Catanzaro <mcatanzaro at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mcatanzaro at igalia.com

--- Comment #9 from Michael Catanzaro <mcatanzaro at igalia.com> ---
(In reply to comment #7)
> 
> This is a common thing we do in the API just to make it more convenient to
> use. The host and protocol are stored in WebCore::SecurityOrigin as String,
> but all our APIs return always string in utf8 (like all other GNOME platform
> APIs). If we return the converted string directly, the returned value should
> be freed by the user. So, we cache the converted strings to be able to
> return a const char *, and we do that only on demand, so if you never call
> get_host we don't have a cached value for nothing.

Surely if we were to take a String s and return s.utf8().data(), that does not need to be freed by the caller. It is a pointer to the CString's internal CStringBuffer. And the CString has a reference-counted CStringBuffer, so the data should be freed automatically when... er, when the function returns. Ah, OK, so that would be bad. WebKit's strings are confusing; I wish it hadn't taken a year for me to realize this.

> > > Source/WebKit2/UIProcess/API/gtk/WebKitSecurityOrigin.cpp:77
> > > +    ASSERT(origin);
> > 
> > When you would write an assertion like this, I think it's better to pass by
> > reference instead. I think we've been trying to avoid pass by pointer except
> > in C API and when the pointer is expected to be null.
> 
> WebKitSecurityOrigin is a boxed type not a C++ class, we always use pointers
> with GObjects or boxed types.

I am not sure why, though. I think it would still work fine if we were to do pass by reference.

> > > Source/WebKit2/UIProcess/API/gtk/WebKitSecurityOrigin.cpp:166
> I thought nullable/allow-none was assumed for char *, I think we don't use
> that in any of our APIs, we will have to update them all.

I think that is wrong, unfortunately. I found [1] which says:

gchar* means (type utf8) (transfer full)
const gchar* means (type utf8) (transfer none)

Where (nullable) is conspicuously missing. And it includes this example:

/**
 * gtk_link_button_new_with_label: 
 * @uri: A URI
 * @label: (nullable): A piece of text or NULL
 */
GtkWidget *
gtk_link_button_new_with_label (const gchar *uri,
                                const gchar *label);

So I think we've been doing it wrong.

Looking at [1], it does look like no annotations are needed for GError, so you're right about that. I was looking at some Gio code that used (out), but I guess it was redundant.

[1] https://wiki.gnome.org/Projects/GObjectIntrospection/Annotations

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150728/fb2afe65/attachment-0001.html>