Ubuntu
chromium-browser package

  1. Bug #1100326
  2. Comment #86

Comment 86 for bug 1100326

Revision history for this message
In , Mrobinson-d (mrobinson-d) wrote :

Comment on attachment 225788
Patch proposal

View in context: https://bugs.webkit.org/attachment.cgi?id=225788&action=review

Looks good to me, in general, but Carlos should probably do a review as well since he's managing the stable branch.

> Source/Platform/GNUmakefile.am:16
> - -I$(top_builddir)/DerivedSources/Platform
> + -I$(top_builddir)/DerivedSources/Platform \
> + -I$(top_builddir)/DerivedSources/WebCore
>

It probably makes sense to generate the files in DerivedSources/Platform for the autotools port.

> Source/WebCore/PlatformGTK.cmake:279
> + COMMAND gdbus-codegen --interface-prefix org.freedesktop.GeoClue2. --c-namespace GClue --generate-c-code ${DERIVED_SOURCES_WEBCORE_DIR}/Geoclue2Interface ${GEOCLUE_DBUS_INTERFACE}

So, if possible it would be could to use GeoClue as the C namespace.

> Source/WebCore/platform/geoclue/GeolocationProviderGeoclue2.cpp:36
> +const char* GeoClueBusName = "org.freedesktop.GeoClue2";
> +const char* GeoClueManagerPath = "/org/freedesktop/GeoClue2/Manager";

These should be named like gGeoClueBusName and gGeoClueManagerPath.

> Source/WebCore/platform/geoclue/GeolocationProviderGeoclue2.cpp:117
> + // The provider will take ownershipt of the managerProxy.

ownershipt->ownership