[Webkit-unassigned] [Bug 139491] [GTK] Add initial database process support

[bugzilla-daemon at webkit.org]

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

--- Comment #19 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #17)
> Comment on attachment 245132 [details]
> Rebased patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=245132&action=review
> 
> Great that the problems with the tests are solved.

Thanks for the review.

> > Source/WebKit2/DatabaseProcess/EntryPoint/unix/DatabaseProcessMain.cpp:2
> > + * Copyright (C) 2014 Igalia S.L.
> 
> Nit: 2015

Sure.

> 
> > Source/WebKit2/Shared/gtk/KeyedDecoder.cpp:30
> > +
> 
> Is this going/meant to be used for anything else apart from the
> DatabaseProcess? If not we should add the proper guards.

I don't know, but neither the WebCore base classes nor the mac implementation have DatabaseProcess guards. In any case these are GTK+ specific files and I plan to enable indexeddb by default in bug #98932, so it will be always built anyway.

> > Source/WebKit2/Shared/gtk/KeyedDecoder.cpp:38
> > +    buildDictionaryFromGVariant(variant.get(), dictionary);
> 
> As we do this trice, and knowing that in none of those cases we use the
> dictionary for anything else, I think we can create and return the
> dictionary from buildDictionaryFromGVariant (renamed to perhaps
> createDictionary... or convertGVariantToDictionary). That would allow us to
> remove those unneeded local variables.

Good idea.

> > Source/WebKit2/Shared/gtk/KeyedDecoder.cpp:47
> > +}
> 
> What about enclosing the destructor in a #if !ASSERT_DISABLED block?

Isn't ASSERT noop in non debug builds? Or do you mean not defining the destructor at all? I think I need to define the destructor, since it's virtual in the parent class.

> > 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) ...

I'll use a template as Zan suggested.

-- 
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/3a81db49/attachment-0002.html>