Skip to content

Commit

Permalink
Revert of Reland 2:Geolocation: move from content/browser to device/ …
Browse files Browse the repository at this point in the history
…(patchset chromium#2 id:20001 of https://codereview.chromium.org/2192683002/ )

Reason for revert:
Looks like this is still breaking builders:

http://build.chromium.org/p/chromium.memory/builders/Mac%20ASan%2064%20Builder/builds/40523

https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/112343/steps/generate_build_files%20%28with%20patch%29/logs/stdio

Original issue's description:
> Reland 2:Geolocation: move from content/browser to device/
>
> Reland was reverted due to a build failure in an official win64 builder
> that hit the infamous size_t to int conversion warning [1]:
>
> FAILED: obj/device/geolocation/device_geolocation.network_location_request.obj
> ninja -t msvc -e environment.x64 -- "C:\b\depot_tools\win_toolchain\vs_files\95ddda401ec5678f15eeed01d2bee08fcbc5ee97\VC\bin\amd64\cl.exe" /nologo /showIncludes /FC @obj\device\geolocation\device_geolocation.network_location_request.obj.rsp /c ..\..\device\geolocation\network_location_request.cc /Foobj\device\geolocation\device_geolocation.network_location_request.obj /Fdobj\device\geolocation\device_geolocation.cc.pdb
> c:\b\build\slave\win64_trunk\build\src\device\geolocation\network_location_request.cc(119): error C2220: warning treated as error - no 'object' file generated
> c:\b\build\slave\win64_trunk\build\src\device\geolocation\network_location_request.cc(119): warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of data
>
> [1] https://uberchromegw.corp.google.com/i/official.desktop.continuous/builders/win64%20trunk/builds/1125/steps/compile/logs/stdio/text
>
> Original Reland description --------------------------------------------
>
> Original CL was reverted because it broke Win Gyp Component-build
> bot(s). This CL (PS2) defines the geolocation gyp component
> correctly.
>
> Original CL description ------------------------------------------------
> Geolocation: move from content/browser to device/
>
> This CL:
>
> - moves all of content/browser/geolocation to device/geolocation
> - the public geolocation files in content/common/{browser, public}
>  are also relocated to device/geolocation
> - the geolocation-specific unittests are compiled into
>  (already existing) device_unittests
> - adds new fancy new device/geolocation BUILD.gn  and
>  geolocation.gyp as well
> - makes a component of geolocation (at least for gn) and that forces
>  adding geolocation_export.h (like other //device/ folders).
> - Java Geolocation files are moved as well, and a new
>  geolocation_jni_registrar is added.
> - classes are moved to device namespace.
>
> All paths and include/call sites updated, DEPS, BUILD.gn files,
> gypi files etc.
>
> Some tricks:
> - can't use BrowserThread::CurrentlyOn, etc; instead, the
>  task runner is cached on constructor and used for both thread
>  checking and PostTask()ing (a few unittest and wifi_data_provider*
>  needed that substitution).
> - GeolocationServiceContext is moved to public/cpp so it can
>  be referenced from WebContentsImpl.
> - MockLocationProvider.java is also moved to device/geolocation.
>
> BUG=612334
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
>
> TBR=pstew@chromium.org
> rationale:device/geolocation depends on dbus and this triggers
> a DEPS presubmit rule -- however, this CL adds no new dependencies,
> hence moving on in the interest of speed (and avoiding more rebases).
>
> Committed: https://crrev.com/67553346ccc4dcb3c2da968bd98f1b15136d2d08
> Cr-Commit-Position: refs/heads/master@{#408393}

TBR=pstew@chromium.org,mcasas@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=612334

Review-Url: https://codereview.chromium.org/2187923003
Cr-Commit-Position: refs/heads/master@{#408396}
  • Loading branch information
dpranke authored and Commit bot committed Jul 28, 2016
1 parent 3dde872 commit c921b9b
Show file tree
Hide file tree
Showing 152 changed files with 2,576 additions and 985 deletions.
2 changes: 0 additions & 2 deletions android_webview/browser/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ include_rules = [
"+content/public/browser",
"+content/public/test",

"+device/geolocation",

# Explicitly disallow using SyncMessageFilter to prevent browser from
# sending synchronous IPC messages on non-UI threads.
"-ipc/ipc_sync_message_filter.h",
Expand Down
16 changes: 8 additions & 8 deletions android_webview/browser/aw_browser_main_parts.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@
#include "base/i18n/rtl.h"
#include "base/path_service.h"
#include "components/crash/content/browser/crash_micro_dump_manager_android.h"
#include "content/public/browser/access_token_store.h"
#include "content/public/browser/android/synchronous_compositor.h"
#include "content/public/browser/geolocation_delegate.h"
#include "content/public/browser/geolocation_provider.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/common/content_client.h"
#include "content/public/common/content_switches.h"
#include "content/public/common/result_codes.h"
#include "device/geolocation/access_token_store.h"
#include "device/geolocation/geolocation_delegate.h"
#include "device/geolocation/geolocation_provider.h"
#include "net/android/network_change_notifier_factory_android.h"
#include "net/base/network_change_notifier.h"
#include "ui/base/l10n/l10n_util.h"
Expand All @@ -41,11 +41,11 @@
namespace android_webview {
namespace {

class AwAccessTokenStore : public device::AccessTokenStore {
class AwAccessTokenStore : public content::AccessTokenStore {
public:
AwAccessTokenStore() { }

// device::AccessTokenStore implementation
// content::AccessTokenStore implementation
void LoadAccessTokens(const LoadAccessTokensCallback& request) override {
AccessTokenStore::AccessTokenMap access_token_map;
// AccessTokenMap and net::URLRequestContextGetter not used on Android,
Expand All @@ -62,11 +62,11 @@ class AwAccessTokenStore : public device::AccessTokenStore {
};

// A provider of Geolocation services to override AccessTokenStore.
class AwGeolocationDelegate : public device::GeolocationDelegate {
class AwGeolocationDelegate : public content::GeolocationDelegate {
public:
AwGeolocationDelegate() = default;

scoped_refptr<device::AccessTokenStore> CreateAccessTokenStore() final {
scoped_refptr<content::AccessTokenStore> CreateAccessTokenStore() final {
return new AwAccessTokenStore();
}

Expand Down Expand Up @@ -127,7 +127,7 @@ int AwBrowserMainParts::PreCreateThreads() {
void AwBrowserMainParts::PreMainMessageLoopRun() {
browser_context_->PreMainMessageLoopRun();

device::GeolocationProvider::SetGeolocationDelegate(
content::GeolocationProvider::SetGeolocationDelegate(
new AwGeolocationDelegate());

AwDevToolsDiscoveryProvider::Install();
Expand Down
2 changes: 0 additions & 2 deletions android_webview/browser/aw_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@
#include "content/public/common/content_switches.h"
#include "content/public/common/url_constants.h"
#include "content/public/common/web_preferences.h"
#include "device/geolocation/access_token_store.h"
#include "device/geolocation/geolocation_delegate.h"
#include "net/android/network_library.h"
#include "net/ssl/ssl_cert_request_info.h"
#include "net/ssl/ssl_info.h"
Expand Down
1 change: 0 additions & 1 deletion android_webview/javatests/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,4 @@ include_rules = [
"+components/policy/android/javatests",
"+content/public/android/java",
"+content/public/test/android/javatests",
"+device/geolocation/android/java",
]
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
import org.chromium.android_webview.AwSettings;
import org.chromium.base.annotations.SuppressFBWarnings;
import org.chromium.base.test.util.Feature;
import org.chromium.device.geolocation.LocationProviderFactory;
import org.chromium.device.geolocation.MockLocationProvider;
import org.chromium.content.browser.LocationProviderFactory;
import org.chromium.content.browser.test.util.MockLocationProvider;

import java.util.concurrent.Callable;

Expand Down
2 changes: 0 additions & 2 deletions android_webview/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,6 @@ instrumentation_test_apk("android_webview_test_apk") {
"//components/web_contents_delegate_android:web_contents_delegate_android_java",
"//content/public/android:content_java",
"//content/public/test/android:content_java_test_support",
"//device/geolocation:geolocation_java",
"//device/geolocation:geolocation_java_test_support",
"//net/android:net_java",
"//net/android:net_java_test_support",
"//third_party/android_tools:legacy_http_javalib",
Expand Down
4 changes: 0 additions & 4 deletions blimp/engine/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ source_set("app") {
"//content/public/common",
"//content/public/renderer",
"//content/public/utility",
"//device/geolocation:device_geolocation",
"//net",
]
}
Expand Down Expand Up @@ -280,7 +279,6 @@ source_set("feature") {
"//blimp/net",
"//content/public/browser",
"//content/public/common",
"//device/geolocation:device_geolocation",
"//net",
"//ui/base",
"//ui/base/ime",
Expand Down Expand Up @@ -342,7 +340,6 @@ source_set("session") {
"//blimp/common/proto",
"//blimp/net",
"//content/public/browser",
"//device/geolocation:device_geolocation",
"//net",
"//ui/aura",
"//ui/base/ime",
Expand Down Expand Up @@ -467,7 +464,6 @@ source_set("feature_unit_tests") {
"//blimp/net:test_support",
"//content",
"//content/public/browser",
"//device/geolocation:device_geolocation",
"//net",
"//net:test_support",
"//testing/gmock",
Expand Down
1 change: 0 additions & 1 deletion blimp/engine/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ include_rules = [
"+components/version_info",
"+components/web_cache/renderer",
"+content/public",
"+device/geolocation",
"+mojo/public",
"+net",
"+services/shell/public/cpp",
Expand Down
2 changes: 1 addition & 1 deletion blimp/engine/app/blimp_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "blimp/engine/app/blimp_browser_main_parts.h"
#include "blimp/engine/app/blimp_content_browser_client.h"
#include "blimp/engine/app/blimp_browser_main_parts.h"
#include "blimp/engine/app/settings_manager.h"
#include "blimp/engine/mojo/blob_channel_service.h"
#include "services/shell/public/cpp/interface_registry.h"
Expand Down
4 changes: 2 additions & 2 deletions blimp/engine/feature/geolocation/blimp_location_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/memory/weak_ptr.h"
#include "device/geolocation/geoposition.h"
#include "content/public/common/geoposition.h"

namespace blimp {
namespace engine {
Expand Down Expand Up @@ -44,7 +44,7 @@ void BlimpLocationProvider::StopProvider() {
}
}

void BlimpLocationProvider::GetPosition(device::Geoposition* position) {
void BlimpLocationProvider::GetPosition(content::Geoposition* position) {
*position = cached_position_;
}

Expand Down
14 changes: 7 additions & 7 deletions blimp/engine/feature/geolocation/blimp_location_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,20 @@

#include "base/memory/weak_ptr.h"
#include "blimp/common/proto/geolocation.pb.h"
#include "device/geolocation/geoposition.h"
#include "device/geolocation/location_provider.h"
#include "content/public/browser/location_provider.h"
#include "content/public/common/geoposition.h"

namespace blimp {
namespace engine {

// Location provider for Blimp using the device's provider over the network.
class BlimpLocationProvider : public device::LocationProvider {
class BlimpLocationProvider : public content::LocationProvider {
public:
// A delegate that implements a subset of LocationProvider's functions.
class Delegate {
public:
using GeopositionReceivedCallback =
base::Callback<void(const device::Geoposition&)>;
base::Callback<void(const content::Geoposition&)>;

virtual ~Delegate() {}

Expand All @@ -34,10 +34,10 @@ class BlimpLocationProvider : public device::LocationProvider {
explicit BlimpLocationProvider(base::WeakPtr<Delegate> delegate);
~BlimpLocationProvider() override;

// device::LocationProvider implementation.
// content::LocationProvider implementation.
bool StartProvider(bool high_accuracy) override;
void StopProvider() override;
void GetPosition(device::Geoposition* position) override;
void GetPosition(content::Geoposition* position) override;
void RequestRefresh() override;
void OnPermissionGranted() override;
void SetUpdateCallback(
Expand All @@ -47,7 +47,7 @@ class BlimpLocationProvider : public device::LocationProvider {
// This delegate handles a subset of the LocationProvider functionality.
base::WeakPtr<Delegate> delegate_;

device::Geoposition cached_position_;
content::Geoposition cached_position_;

// True if a successful StartProvider call has occured.
bool is_started_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ class BlimpLocationProviderTest : public testing::Test {
void SetUp() override {}

MOCK_METHOD2(OnLocationUpdate,
void(const device::LocationProvider* provider,
const device::Geoposition& geoposition));
void(const content::LocationProvider* provider,
const content::Geoposition& geoposition));

protected:
device::LocationProvider::LocationProviderUpdateCallback mock_callback_;
content::LocationProvider::LocationProviderUpdateCallback mock_callback_;
std::unique_ptr<MockBlimpLocationProviderDelegate> delegate_;
std::unique_ptr<BlimpLocationProvider> location_provider_;

Expand Down Expand Up @@ -137,12 +137,12 @@ TEST_F(BlimpLocationProviderTest, OnPermissionGrantedHandlesNullDelegate) {
}

TEST_F(BlimpLocationProviderTest, SetUpdateCallbackPropagatesCallback) {
base::Callback<void(const device::Geoposition&)> callback;
base::Callback<void(const content::Geoposition&)> callback;
EXPECT_CALL(*delegate_, SetUpdateCallback(_)).WillOnce(SaveArg<0>(&callback));
EXPECT_CALL(*this, OnLocationUpdate(location_provider_.get(), _)).Times(1);

location_provider_->SetUpdateCallback(mock_callback_);
callback.Run(device::Geoposition());
callback.Run(content::Geoposition());
}

TEST_F(BlimpLocationProviderTest, SetUpdateCallbackHandlesNullDelegate) {
Expand Down
32 changes: 16 additions & 16 deletions blimp/engine/feature/geolocation/engine_geolocation_feature.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@
#include "blimp/common/create_blimp_message.h"
#include "blimp/common/proto/blimp_message.pb.h"
#include "blimp/common/proto/geolocation.pb.h"
#include "device/geolocation/geolocation_delegate.h"
#include "device/geolocation/location_provider.h"
#include "device/geolocation/geoposition.h"
#include "content/public/browser/geolocation_delegate.h"
#include "content/public/browser/location_provider.h"
#include "content/public/common/geoposition.h"
#include "net/base/net_errors.h"

namespace blimp {
namespace engine {
namespace {
class BlimpGeolocationDelegate : public device::GeolocationDelegate {
class BlimpGeolocationDelegate : public content::GeolocationDelegate {
public:
explicit BlimpGeolocationDelegate(
base::WeakPtr<BlimpLocationProvider::Delegate> feature_delegate) {
Expand All @@ -29,7 +29,7 @@ class BlimpGeolocationDelegate : public device::GeolocationDelegate {

bool UseNetworkLocationProviders() final { return false; }

std::unique_ptr<device::LocationProvider> OverrideSystemLocationProvider()
std::unique_ptr<content::LocationProvider> OverrideSystemLocationProvider()
final {
return base::WrapUnique(new BlimpLocationProvider(feature_delegate_));
}
Expand All @@ -40,21 +40,21 @@ class BlimpGeolocationDelegate : public device::GeolocationDelegate {
DISALLOW_COPY_AND_ASSIGN(BlimpGeolocationDelegate);
};

device::Geoposition::ErrorCode ConvertErrorCode(
content::Geoposition::ErrorCode ConvertErrorCode(
const GeolocationErrorMessage::ErrorCode& error_code) {
switch (error_code) {
case GeolocationErrorMessage::PERMISSION_DENIED:
return device::Geoposition::ErrorCode::ERROR_CODE_PERMISSION_DENIED;
return content::Geoposition::ErrorCode::ERROR_CODE_PERMISSION_DENIED;
case GeolocationErrorMessage::POSITION_UNAVAILABLE:
return device::Geoposition::ErrorCode::ERROR_CODE_POSITION_UNAVAILABLE;
return content::Geoposition::ErrorCode::ERROR_CODE_POSITION_UNAVAILABLE;
case GeolocationErrorMessage::TIMEOUT:
return device::Geoposition::ErrorCode::ERROR_CODE_TIMEOUT;
return content::Geoposition::ErrorCode::ERROR_CODE_TIMEOUT;
}
}

device::Geoposition ConvertLocationMessage(
content::Geoposition ConvertLocationMessage(
const GeolocationCoordinatesMessage& coordinates) {
device::Geoposition output;
content::Geoposition output;
output.latitude = coordinates.latitude();
output.longitude = coordinates.longitude();
output.altitude = coordinates.altitude();
Expand All @@ -63,7 +63,7 @@ device::Geoposition ConvertLocationMessage(
output.heading = coordinates.heading();
output.speed = coordinates.speed();
output.timestamp = base::Time::Now();
output.error_code = device::Geoposition::ErrorCode::ERROR_CODE_NONE;
output.error_code = content::Geoposition::ErrorCode::ERROR_CODE_NONE;
return output;
}

Expand All @@ -79,7 +79,7 @@ void EngineGeolocationFeature::set_outgoing_message_processor(
outgoing_message_processor_ = std::move(message_processor);
}

device::GeolocationDelegate*
content::GeolocationDelegate*
EngineGeolocationFeature::CreateGeolocationDelegate() {
return new BlimpGeolocationDelegate(weak_factory_.GetWeakPtr());
}
Expand All @@ -95,14 +95,14 @@ void EngineGeolocationFeature::ProcessMessage(
case GeolocationMessage::kCoordinates: {
const GeolocationCoordinatesMessage& location =
geolocation_message.coordinates();
device::Geoposition output = ConvertLocationMessage(location);
content::Geoposition output = ConvertLocationMessage(location);
NotifyCallback(output);
break;
}
case GeolocationMessage::kError: {
const GeolocationErrorMessage& error_message =
geolocation_message.error();
device::Geoposition output;
content::Geoposition output;
output.error_message = error_message.error_message();
output.error_code = ConvertErrorCode(error_message.error_code());
NotifyCallback(output);
Expand All @@ -119,7 +119,7 @@ void EngineGeolocationFeature::ProcessMessage(
}

void EngineGeolocationFeature::NotifyCallback(
const device::Geoposition& position) {
const content::Geoposition& position) {
geoposition_received_callback_.Run(position);
}

Expand Down
12 changes: 5 additions & 7 deletions blimp/engine/feature/geolocation/engine_geolocation_feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,9 @@
#include "base/memory/weak_ptr.h"
#include "blimp/engine/feature/geolocation/blimp_location_provider.h"
#include "blimp/net/blimp_message_processor.h"

namespace device {
class GeolocationDelegate;
struct Geoposition;
}
#include "content/public/browser/geolocation_delegate.h"
#include "content/public/browser/location_provider.h"
#include "content/public/common/geoposition.h"

namespace blimp {
namespace engine {
Expand All @@ -30,14 +28,14 @@ class EngineGeolocationFeature : public BlimpMessageProcessor,
void set_outgoing_message_processor(
std::unique_ptr<BlimpMessageProcessor> message_processor);

device::GeolocationDelegate* CreateGeolocationDelegate();
content::GeolocationDelegate* CreateGeolocationDelegate();

// BlimpMessageProcessor implementation.
void ProcessMessage(std::unique_ptr<BlimpMessage> message,
const net::CompletionCallback& callback) override;

private:
void NotifyCallback(const device::Geoposition& position);
void NotifyCallback(const content::Geoposition& position);

// BlimpLocationProvider::Delegate implementation.
void RequestAccuracy(
Expand Down
Loading

0 comments on commit c921b9b

Please sign in to comment.