Cleanup: MockGeolocationArbitrator for GeolocationProvider tests

Cleanup: use MockGeolocationArbitrator to test GeolocationProvider

This patch introduces a MockGeolocationArbitrator class for use
with the GeolocationProvider unittests. This avoids having to
instantiate dependencies of the GeolocationArbitrator to test the
GeolocationProvider.

This will make it easier to add a unit test for 
https://codereview.chromium.org/11312210/

TEST=Existing
BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=170444

[bulach, 2012-11-28 19:35:59]

lgtm, thanks! you'll need OWNERS for the gyp..

one question for the test below (although your patch hasn't changed the
condition...), and a few nits:

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
File content/browser/geolocation/geolocation_provider_unittest.cc (right):

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
content/browser/geolocation/geolocation_provider_unittest.cc:114: {
nit: { on the previous line

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
content/browser/geolocation/geolocation_provider_unittest.cc:171:
EXPECT_TRUE(provider()->IsRunning());
shouldn't the provider be stopped at this stage?

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
File content/browser/geolocation/mock_location_arbitrator.cc (right):

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
content/browser/geolocation/mock_location_arbitrator.cc:34: }
nit: // namespace content

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
File content/browser/geolocation/mock_location_arbitrator.h (right):

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
content/browser/geolocation/mock_location_arbitrator.h:29: 
nit: remove \n

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
content/browser/geolocation/mock_location_arbitrator.h:31: bool
providers_started_;
nit: \n DISALLOW_COPY_AND_ASSIGN(MockGeolocationArbitrator);

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
content/browser/geolocation/mock_location_arbitrator.h:34: };
nit: s/;/  // namespace content

[John Knottenbelt, 2012-11-29 17:31:01]

Thanks for review, Marcus!

Brett, please could you review the changes to the gypi files.

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
File content/browser/geolocation/geolocation_provider_unittest.cc (right):

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
content/browser/geolocation/geolocation_provider_unittest.cc:114: {
On 2012/11/28 19:35:59, bulach wrote:
> nit: { on the previous line

Done.

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
content/browser/geolocation/geolocation_provider_unittest.cc:171:
EXPECT_TRUE(provider()->IsRunning());
On 2012/11/28 19:35:59, bulach wrote:
> shouldn't the provider be stopped at this stage?

This may be something we want to look into for the future. 

I think, at one stage, we did have the provider starting and stopping the
geolocation thread. The details are a bit hazy, but I think this made things a
little awkward if there were any 'long running' providers still executing on the
thread. We don't want to wait for them to finish we can pronounce the thread
dead. 

I also believe that repeatedly starting / stopping the thread  stopped working
after some time. I didn't have time to investigate the reason behind this, but
it seemed simpler to have a long running geolocation thread.

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
File content/browser/geolocation/mock_location_arbitrator.cc (right):

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
content/browser/geolocation/mock_location_arbitrator.cc:34: }
On 2012/11/28 19:35:59, bulach wrote:
> nit: // namespace content

Done.

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
File content/browser/geolocation/mock_location_arbitrator.h (right):

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
content/browser/geolocation/mock_location_arbitrator.h:29: 
On 2012/11/28 19:35:59, bulach wrote:
> nit: remove \n

Done.

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
content/browser/geolocation/mock_location_arbitrator.h:31: bool
providers_started_;
On 2012/11/28 19:35:59, bulach wrote:
> nit: \n DISALLOW_COPY_AND_ASSIGN(MockGeolocationArbitrator);

Done.

https://codereview.chromium.org/11412194/diff/7002/content/browser/geolocatio...
content/browser/geolocation/mock_location_arbitrator.h:34: };
On 2012/11/28 19:35:59, bulach wrote:
> nit: s/;/  // namespace content

Done.

[brettw, 2012-11-29 23:55:30]

.gypi lgtm

[commit-bot: I haz the power, 2012-11-30 10:24:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jknotten@chromium.org/11412194/8003

[commit-bot: I haz the power, 2012-11-30 13:00:41]

Change committed as 170444