Make chrome communicate with gpsd through libgps/shared memory

Make chrome communicate with gpsd through libgps/shared memory

Update gps.h header to 3.1 version to have shared memory export
libgps APIs. Adapt libgps_wrapper_linux to use shared memory to
communicate with gpsd.

We enable the libgps support on ChromeOS and drop the support
on destkop linux for now due to the fact that we can't control
what gpsd version will be present on the desktop system and the gps
data written to shared memory by gpsd maybe not be compatible 
with what chrome/libgps is expecting.
    
If there is demand for desktop libgps support we can add it 
back and investigate on using socket interface between libgps and gpsd.

BUG=103751
TEST=run
"tools/checklicenses/checklicenses.py third_party/gpsd/"
and make sure no license violation.

content_unittests --gtest_filter="*Gps*"
and make sure all unittests pass


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=109736

[joth, 2011-11-11 18:40:40]

LG, thanks for handling this up.

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
File content/browser/geolocation/libgps_wrapper_linux.cc (right):

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:33: CHECK_EQ(0, err) <<
"Error closing dl handle: " << err;
nit: use DCHECK_EQ

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:40: LOG(WARNING) << "Could
not open " << LIBGPS_VERSION << ": " << dlerror();
this is only log warning for Chrome OS.

DLOG(WARNING) might actually make a good compromise here.

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:44: LOG(INFO) << "Loaded "
<< LIBGPS_VERSION;
again DLOG (or DVLOG).

chrome is trying to discourage production logging for 'normal' flows

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:50: LOG(WARNING) << "libgps
" << #function << " error: " << dlerror(); \
ditto, and throughout the file

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:71: if
(gps_open_(GPSD_SHARED_MEMORY, 0, &gps_data_) != 0) {
this is still applied for desktop linux?

I'm still not convinced about using shared mem only on desktop linux. it could
easily break and cause chrome to crash if user has an otherwise benignly
mis-configured system (i.e. 2 versions of libgps.so installed, and the 'wrong'
version of gpsd is running).
at least with the socket interface we'd just get json parse error at worse, not
a crash.

options for the linux build:
- disable it all
- set to "localhost"
- get it from a command-line switch (defaulting to one of the above)

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:126: DLOG(WARNING) <<
"Status_NO_FIX";
this looks like it will be very noisy. DVLOG(1) or even 2.

(are you familiar with using --v-module to enable verbose logging?)

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:138: if
(!isnan(gps_data_.fix.epx) && !isnan(gps_data_.fix.epy)) {
nit: i guess either epx or epy would probably do if the other were nan, but I
doubt that's a practical problem? if so no need to handle that

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:144: // So expecting that
accuracy is not valid most of the time.
maybe this comment should go onto the bug report. I'm not quite clear, does this
mean it's expected that accuracy will come and go from one second to the next?
if so we should code around it e.g. remember the accuracy when we get one, and
inject that in subsequent polls

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
File content/browser/geolocation/libgps_wrapper_linux.h (right):

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.h:14: #include
"third_party/gpsd/release-3.1/gps.h"
could we keep this #include in the .cc ? even if we're just using one gpds
version, it still seems nice to contain it in one place

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.h:16:
COMPILE_ASSERT(GPSD_API_MAJOR_VERSION == 5, GPSD_API_version_is_not_5);
...so this would have to go to the .cc too which should be fine

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.h:17: #define LIBGPS_VERSION 
"libgps.so.20"
use const char[] in anon namespace in the cc

http://codereview.chromium.org/8463022/diff/2001/content/content_browser.gypi
File content/content_browser.gypi (left):

http://codereview.chromium.org/8463022/diff/2001/content/content_browser.gypi...
content/content_browser.gypi:193:
'browser/geolocation/libgps_2_94_wrapper_linux.cc',
you can delete this file too, in the same patch

[Yufeng Shen (Slow to review), 2011-11-11 21:11:35]

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
File content/browser/geolocation/libgps_wrapper_linux.cc (right):

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:33: CHECK_EQ(0, err) <<
"Error closing dl handle: " << err;
On 2011/11/11 18:40:40, joth wrote:
> nit: use DCHECK_EQ

Done.

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:40: LOG(WARNING) << "Could
not open " << LIBGPS_VERSION << ": " << dlerror();
On 2011/11/11 18:40:40, joth wrote:
> this is only log warning for Chrome OS.
> 
> DLOG(WARNING) might actually make a good compromise here.

Done.

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:44: LOG(INFO) << "Loaded "
<< LIBGPS_VERSION;
On 2011/11/11 18:40:40, joth wrote:
> again DLOG (or DVLOG).
> 
> chrome is trying to discourage production logging for 'normal' flows

Done.

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:50: LOG(WARNING) << "libgps
" << #function << " error: " << dlerror(); \
On 2011/11/11 18:40:40, joth wrote:
> ditto, and throughout the file

Done.

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:71: if
(gps_open_(GPSD_SHARED_MEMORY, 0, &gps_data_) != 0) {
Drop the desktop linux for now

On 2011/11/11 18:40:40, joth wrote:
> this is still applied for desktop linux?
> 
> I'm still not convinced about using shared mem only on desktop linux. it could
> easily break and cause chrome to crash if user has an otherwise benignly
> mis-configured system (i.e. 2 versions of libgps.so installed, and the 'wrong'
> version of gpsd is running).
> at least with the socket interface we'd just get json parse error at worse,
not
> a crash.
> 
> options for the linux build:
> - disable it all
> - set to "localhost"
> - get it from a command-line switch (defaulting to one of the above)

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:126: DLOG(WARNING) <<
"Status_NO_FIX";
done

trying to get myself familiar with that

On 2011/11/11 18:40:40, joth wrote:
> this looks like it will be very noisy. DVLOG(1) or even 2.
> 
> (are you familiar with using --v-module to enable verbose logging?)

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:138: if
(!isnan(gps_data_.fix.epx) && !isnan(gps_data_.fix.epy)) {
On 2011/11/11 18:40:40, joth wrote:
> nit: i guess either epx or epy would probably do if the other were nan, but I
> doubt that's a practical problem? if so no need to handle that

Done.

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:144: // So expecting that
accuracy is not valid most of the time.
It means with most devices we won't get fix.epx and fix.epy
information. I updated http://crbug.com/99326   with the details.

On 2011/11/11 18:40:40, joth wrote:
> maybe this comment should go onto the bug report. I'm not quite clear, does
this
> mean it's expected that accuracy will come and go from one second to the next?
> if so we should code around it e.g. remember the accuracy when we get one, and
> inject that in subsequent polls

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
File content/browser/geolocation/libgps_wrapper_linux.h (right):

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.h:14: #include
"third_party/gpsd/release-3.1/gps.h"
On 2011/11/11 18:40:40, joth wrote:
> could we keep this #include in the .cc ? even if we're just using one gpds
> version, it still seems nice to contain it in one place

Done.

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.h:16:
COMPILE_ASSERT(GPSD_API_MAJOR_VERSION == 5, GPSD_API_version_is_not_5);
On 2011/11/11 18:40:40, joth wrote:
> ...so this would have to go to the .cc too which should be fine

Done.

http://codereview.chromium.org/8463022/diff/2001/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.h:17: #define LIBGPS_VERSION 
"libgps.so.20"
On 2011/11/11 18:40:40, joth wrote:
> use const char[] in anon namespace in the cc

Done.

http://codereview.chromium.org/8463022/diff/2001/content/content_browser.gypi
File content/content_browser.gypi (left):

http://codereview.chromium.org/8463022/diff/2001/content/content_browser.gypi...
content/content_browser.gypi:193:
'browser/geolocation/libgps_2_94_wrapper_linux.cc',
On 2011/11/11 18:40:40, joth wrote:
> you can delete this file too, in the same patch

Done.

[joth, 2011-11-11 21:24:17]

LGTM

http://codereview.chromium.org/8463022/diff/3010/content/browser/geolocation/...
File content/browser/geolocation/libgps_wrapper_linux.cc (right):

http://codereview.chromium.org/8463022/diff/3010/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:17: const char
LIBGPS_VERSION[] = "libgps.so.20";
write this is:

namespace {
const char kLibGpsName[] = "libgps.so.20";
}  // namespace

[Yufeng Shen (Slow to review), 2011-11-11 21:35:03]

http://codereview.chromium.org/8463022/diff/3010/content/browser/geolocation/...
File content/browser/geolocation/libgps_wrapper_linux.cc (right):

http://codereview.chromium.org/8463022/diff/3010/content/browser/geolocation/...
content/browser/geolocation/libgps_wrapper_linux.cc:17: const char
LIBGPS_VERSION[] = "libgps.so.20";
On 2011/11/11 21:24:17, joth wrote:
> write this is:
> 
> namespace {
> const char kLibGpsName[] = "libgps.so.20";
> }  // namespace

Done.

[commit-bot: I haz the power, 2011-11-11 21:38:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miletus@chromium.org/8463022/5004

[commit-bot: I haz the power, 2011-11-12 00:21:55]

Change committed as 109736