[Webkit-unassigned] [Bug 139491] [GTK] Add initial database process support
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jan 22 07:09:17 PST 2015
https://bugs.webkit.org/show_bug.cgi?id=139491
--- Comment #17 from Sergio Villar Senin <svillar at igalia.com> ---
Comment on attachment 245132
--> https://bugs.webkit.org/attachment.cgi?id=245132
Rebased patch
View in context: https://bugs.webkit.org/attachment.cgi?id=245132&action=review
Great that the problems with the tests are solved.
> Source/WebKit2/DatabaseProcess/EntryPoint/unix/DatabaseProcessMain.cpp:2
> + * Copyright (C) 2014 Igalia S.L.
Nit: 2015
> Source/WebKit2/DatabaseProcess/gtk/DatabaseProcessMainGtk.cpp:2
> + * Copyright (C) 2014 Igalia S.L.
Ditto.
> Source/WebKit2/Shared/gtk/KeyedDecoder.cpp:2
> + * Copyright (C) 2014 Igalia S.L.
Ditto.
> 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.
> 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.
> Source/WebKit2/Shared/gtk/KeyedDecoder.cpp:47
> +}
What about enclosing the destructor in a #if !ASSERT_DISABLED block?
> 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) ...
> Source/WebKit2/Shared/gtk/KeyedDecoder.h:2
> + * Copyright (C) 2014 Igaia S.L.
Nit: 2015
> Source/WebKit2/Shared/gtk/KeyedDecoder.h:35
> +
Is this going/meant to be used for anything else apart from the DatabaseProcess? If not we should add the proper guards.
> Source/WebKit2/Shared/gtk/KeyedEncoder.cpp:2
> + * Copyright (C) 2014 Igalia S.L.
Ditto.
> Source/WebKit2/Shared/gtk/KeyedEncoder.cpp:31
> +
Is this going/meant to be used for anything else apart from the DatabaseProcess? If not we should add the proper guards.
> Source/WebKit2/Shared/gtk/KeyedEncoder.cpp:48
> +}
Same comment about the destructor full of ASSERTs.
> Source/WebKit2/Shared/gtk/KeyedEncoder.h:2
> + * Copyright (C) 2014 Igalia S.L.
Nit: 2015
> Source/WebKit2/Shared/gtk/KeyedEncoder.h:34
> +
Is this going/meant to be used for anything else apart from the DatabaseProcess? If not we should add the proper guards.
--
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/20150122/6743ee9e/attachment-0002.html>
More information about the webkit-unassigned
mailing list