Move wifi_data_provider_chromeos -> content

Move wifi_data_provider_chromeos -> content

This also implements wifi based geolocation correctly on ChromeOS.

This CL is dependent upon https://codereview.chromium.org/11887008/
It enfolds the changes from https://codereview.chromium.org/11710002/

Testing: Visit http://html5demos.com/geo and grant permissions. The location should be based on wifi data, not just IP data.

BUG=98716, 156428, 167987
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=177320

[stevenjb, 2013-01-12 02:06:36]

Tested this on a ChromeOS device, so it both migrates the code to src/content,
and actually works.

[John Knottenbelt, 2013-01-14 14:12:30]

LGTM, with an inline comment / question.

https://codereview.chromium.org/11881011/diff/2001/content/browser/geolocatio...
File content/browser/geolocation/device_data_provider.h (left):

https://codereview.chromium.org/11881011/diff/2001/content/browser/geolocatio...
content/browser/geolocation/device_data_provider.h:129:
DCHECK(CalledOnClientThread());
According to jam@, there are some tests that hold a reference to the provider
causing the last reference to be lost on the UI thread instead of the
geolocation thread.

It seems a shame to remove the guarantee that this class will always be
destroyed on the same thread its used on. Can you take a look and see if there
are any obvious fixes?

[stevenjb, 2013-01-14 18:02:38]

https://codereview.chromium.org/11881011/diff/2001/content/browser/geolocatio...
File content/browser/geolocation/device_data_provider.h (left):

https://codereview.chromium.org/11881011/diff/2001/content/browser/geolocatio...
content/browser/geolocation/device_data_provider.h:129:
DCHECK(CalledOnClientThread());
On 2013/01/14 14:12:31, John Knottenbelt wrote:
> According to jam@, there are some tests that hold a reference to the provider
> causing the last reference to be lost on the UI thread instead of the
> geolocation thread.
> 
> It seems a shame to remove the guarantee that this class will always be
> destroyed on the same thread its used on. Can you take a look and see if there
> are any obvious fixes?

jam@, any thoughts? I just copied this, but I'm really not familiar wiht how
it's used. I could put the DCHECK back and see what fails...

That said, a DCHECK like this in the destructor is kind of unusual, and this
does derive from base::RefCountedThreadSafe, which implies to me that destroying
this on another thread should be valid?

[jam, 2013-01-14 18:57:21]

lgtm

https://codereview.chromium.org/11881011/diff/2001/content/browser/geolocatio...
File content/browser/geolocation/device_data_provider.h (left):

https://codereview.chromium.org/11881011/diff/2001/content/browser/geolocatio...
content/browser/geolocation/device_data_provider.h:129:
DCHECK(CalledOnClientThread());
On 2013/01/14 18:02:38, stevenjb (chromium) wrote:
> On 2013/01/14 14:12:31, John Knottenbelt wrote:
> > According to jam@, there are some tests that hold a reference to the
provider
> > causing the last reference to be lost on the UI thread instead of the
> > geolocation thread.
> > 
> > It seems a shame to remove the guarantee that this class will always be
> > destroyed on the same thread its used on. Can you take a look and see if
there
> > are any obvious fixes?
> 
> jam@, any thoughts? I just copied this, but I'm really not familiar wiht how
> it's used. I could put the DCHECK back and see what fails...
> 
> That said, a DCHECK like this in the destructor is kind of unusual, and this
> does derive from base::RefCountedThreadSafe, which implies to me that
destroying
> this on another thread should be valid?

this check is just invalid because as you say the destruction of this object can
happen on different threads because of the ref counted. normally if we want to
add a dcheck like this, a traits on the object would ensure that it destructs on
a sepecific message loop. that can be done, but it seems out of scope of this
change.

the issue is not with a specific test that holds a reference. it's just that we
never used the cros implementation in tests for a long time, and that one seems
to post tasks differently from other tests which changes the ref counting

[Greg Spencer (Chromium), 2013-01-15 21:42:54]

LGTM

[commit-bot: I haz the power, 2013-01-17 00:31:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevenjb@chromium.org/11881011/7001

[commit-bot: I haz the power, 2013-01-17 02:38:42]

Change committed as 177320