[Webkit-unassigned] [Bug 139491] [GTK] Add initial database process support
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jan 23 02:47:09 PST 2015
https://bugs.webkit.org/show_bug.cgi?id=139491
--- Comment #20 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #18)
> Comment on attachment 245132 [details]
> Rebased patch
>
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=245132&action=review
Thanks for the review.
> > Source/WTF/wtf/gobject/GRefPtr.cpp:102
> > +template <> void derefGPtr(GVariantBuilder* ptr)
> > +{
> > + g_variant_builder_unref(ptr);
> > +}
>
> Safe to call g_variant_builder_unref() without a null check on ptr, like in
> refGPtr() for GVariantBuilder?
Good catch, it's indeed needed, and I made the mistake because I copy-pasted the GVariant implementation, which means it also needs to be fixed. I've noticed that GHashTable impl has the same issue so I'll fix both GVariant and GHashTable in a separate patch.
> > Source/WebKit2/DatabaseProcess/DatabaseProcess.cpp:196
> > + RefPtr<DatabaseToWebProcessConnection> connection = DatabaseToWebProcessConnection::create(socketPair.server);
> > + m_databaseToWebProcessConnections.append(connection.release());
>
> Can you inline the DatabaseToWebProcessConnection::create() call?
Yes
> > Source/WebKit2/DatabaseProcess/DatabaseProcess.cpp:199
> > + IPC::Attachment clientSocket(socketPair.client);
> > + parentProcessConnection()->send(Messages::DatabaseProcessProxy::DidCreateDatabaseToWebProcessConnection(clientSocket), 0);
>
> Can you inline the IPC::Attachment construction?
Yes
> > Source/WebKit2/DatabaseProcess/unix/DatabaseProcessMainUnix.h:43
> > +#endif // DatabaseProcessMainUnix_h
> > +
> > +#endif // ENABLE(DATABASE_PROCESS)
>
> The comments for the build guards are reversed here.
Oops
> >> Source/WebKit2/Shared/gtk/KeyedDecoder.cpp:79
> >> +
> >
> > This and the following 4 decodeXXX look almost the same, I think we can create a macro with this code, the unique difference is the g_variant_get_XXX call, which can be generated by the macro using a passed in argument. So the body of those methods would be simply something like DECODE(key, result, boolean) or DECODE(key, result, int32) ...
>
> You get a golden r+ for using templates instead of macros.
Let's try
> > Source/WebKit2/Shared/gtk/KeyedEncoder.h:52
> > + virtual void encodeBytes(const String& key, const uint8_t*, size_t) override;
> > + virtual void encodeBool(const String& key, bool) override;
> > + virtual void encodeUInt32(const String& key, uint32_t) override;
> > + virtual void encodeInt32(const String& key, int32_t) override;
> > + virtual void encodeInt64(const String& key, int64_t) override;
> > + virtual void encodeFloat(const String& key, float) override;
> > + virtual void encodeDouble(const String& key, double) override;
> > + virtual void encodeString(const String& key, const String&) override;
>
> Same here, it might be possible to implement these through macros or
> templates.
In the case of the encoder, the implementation of those methods is already a single line, what would be the benefit of a template?
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150123/afcb367e/attachment-0002.html>
More information about the webkit-unassigned
mailing list