Revert "Cherry Pick https://codereview.chromium.org/65273002/ PS 8"
Bug 11668445
There are object lifetime issues with RenderProcessHost and
geolation_dispatcher(), and rolling the patch out is safest path
at this point.
This reverts commit 2a3dd3fe8352b8d59e69e1db33b93034effb23e7.
Change-Id: I9ce22bb47b854f875ca8a10e1457ae079e2d7b91
diff --git a/android_webview/native/aw_contents.cc b/android_webview/native/aw_contents.cc
index e1351bd..6670432 100644
--- a/android_webview/native/aw_contents.cc
+++ b/android_webview/native/aw_contents.cc
@@ -710,13 +710,11 @@
void AwContents::SetIsPaused(JNIEnv* env, jobject obj, bool paused) {
browser_view_renderer_->SetIsPaused(paused);
- ContentViewCore* cvc =
- ContentViewCore::FromWebContents(web_contents_.get());
- if (cvc) {
- cvc->PauseOrResumeGeolocation(paused);
- if (paused) {
+ if (paused) {
+ ContentViewCore* cvc =
+ ContentViewCore::FromWebContents(web_contents_.get());
+ if (cvc)
cvc->PauseVideo();
- }
}
}
diff --git a/content/browser/android/content_view_core_impl.cc b/content/browser/android/content_view_core_impl.cc
index a28c2dc..e43a656 100644
--- a/content/browser/android/content_view_core_impl.cc
+++ b/content/browser/android/content_view_core_impl.cc
@@ -351,20 +351,6 @@
host->Send(new ViewMsg_PauseVideo(host->GetRoutingID()));
}
-void ContentViewCoreImpl::PauseOrResumeGeolocation(bool should_pause) {
- RenderViewHostImpl* rvh =
- static_cast<RenderViewHostImpl*>(web_contents_->GetRenderViewHost());
- if (rvh) {
- BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
- base::Bind(&GeolocationDispatcherHost::PauseOrResume,
- static_cast<RenderProcessHostImpl*>(
- web_contents_->GetRenderProcessHost())->
- geolocation_dispatcher_host(),
- rvh->GetRoutingID(),
- should_pause));
- }
-}
-
void ContentViewCoreImpl::OnTabCrashed() {
JNIEnv* env = AttachCurrentThread();
ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
diff --git a/content/browser/android/content_view_core_impl.h b/content/browser/android/content_view_core_impl.h
index 9a70c07..837a644 100644
--- a/content/browser/android/content_view_core_impl.h
+++ b/content/browser/android/content_view_core_impl.h
@@ -61,7 +61,6 @@
virtual void RequestContentClipping(const gfx::Rect& clipping,
const gfx::Size& content_size) OVERRIDE;
virtual void PauseVideo() OVERRIDE;
- virtual void PauseOrResumeGeolocation(bool should_pause) OVERRIDE;
// --------------------------------------------------------------------------
// Methods called from Java via JNI
diff --git a/content/browser/geolocation/geolocation_dispatcher_host.cc b/content/browser/geolocation/geolocation_dispatcher_host.cc
index 8784a56..53f8e4f 100644
--- a/content/browser/geolocation/geolocation_dispatcher_host.cc
+++ b/content/browser/geolocation/geolocation_dispatcher_host.cc
@@ -67,29 +67,23 @@
bool enable_high_accuracy);
void OnStopUpdating(int render_view_id);
- virtual void PauseOrResume(int render_view_id, bool should_pause) OVERRIDE;
-
// Updates the |location_arbitrator_| with the currently required update
- // options.
- void RefreshGeolocationOptions();
+ // options, based on |renderer_high_accuracy_|.
+ void RefreshHighAccuracy();
void OnLocationUpdate(const Geoposition& position);
int render_process_id_;
scoped_refptr<GeolocationPermissionContext> geolocation_permission_context_;
- struct RendererGeolocationOptions {
- bool high_accuracy;
- bool is_paused;
- };
-
- // Used to keep track of the renderers in this process that are using
- // geolocation and the options associated with them. The map is iterated
- // when a location update is available and the fan out to individual bridge
- // IDs happens renderer side, in order to minimize context switches.
+ // Iterated when sending location updates to renderer processes. The fan out
+ // to individual bridge IDs happens renderer side, in order to minimize
+ // context switches.
// Only used on the IO thread.
- std::map<int, RendererGeolocationOptions> geolocation_renderers_;
-
+ std::set<int> geolocation_renderer_ids_;
+ // Maps renderer_id to whether high accuracy is requestd for this particular
+ // bridge.
+ std::map<int, bool> renderer_high_accuracy_;
// Only set whilst we are registered with the arbitrator.
GeolocationProviderImpl* location_provider_;
@@ -136,11 +130,9 @@
void GeolocationDispatcherHostImpl::OnLocationUpdate(
const Geoposition& geoposition) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
- for (std::map<int, RendererGeolocationOptions>::iterator it =
- geolocation_renderers_.begin();
- it != geolocation_renderers_.end(); ++it) {
- if (!(it->second.is_paused))
- Send(new GeolocationMsg_PositionUpdated(it->first, geoposition));
+ for (std::set<int>::iterator it = geolocation_renderer_ids_.begin();
+ it != geolocation_renderer_ids_.end(); ++it) {
+ Send(new GeolocationMsg_PositionUpdated(*it, geoposition));
}
}
@@ -191,58 +183,44 @@
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
DVLOG(1) << __FUNCTION__ << " " << render_process_id_ << ":"
<< render_view_id;
- RendererGeolocationOptions opts = { enable_high_accuracy, false };
- geolocation_renderers_[render_view_id] = opts;
- RefreshGeolocationOptions();
+ if (!geolocation_renderer_ids_.count(render_view_id))
+ geolocation_renderer_ids_.insert(render_view_id);
+
+ renderer_high_accuracy_[render_view_id] = enable_high_accuracy;
+ RefreshHighAccuracy();
}
void GeolocationDispatcherHostImpl::OnStopUpdating(int render_view_id) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
DVLOG(1) << __FUNCTION__ << " " << render_process_id_ << ":"
<< render_view_id;
- DCHECK_EQ(1U, geolocation_renderers_.count(render_view_id));
- geolocation_renderers_.erase(render_view_id);
- RefreshGeolocationOptions();
+ if (renderer_high_accuracy_.erase(render_view_id))
+ RefreshHighAccuracy();
+
+ DCHECK_EQ(1U, geolocation_renderer_ids_.count(render_view_id));
+ geolocation_renderer_ids_.erase(render_view_id);
}
-void GeolocationDispatcherHostImpl::PauseOrResume(int render_view_id,
- bool should_pause) {
+void GeolocationDispatcherHostImpl::RefreshHighAccuracy() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
- std::map<int, RendererGeolocationOptions>::iterator it =
- geolocation_renderers_.find(render_view_id);
- if (it == geolocation_renderers_.end())
- return;
-
- RendererGeolocationOptions* opts = &(it->second);
- if (opts->is_paused != should_pause) {
- opts->is_paused = should_pause;
- RefreshGeolocationOptions();
- }
-}
-
-void GeolocationDispatcherHostImpl::RefreshGeolocationOptions() {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-
- bool needs_updates = false;
- bool use_high_accuracy = false;
- std::map<int, RendererGeolocationOptions>::const_iterator i =
- geolocation_renderers_.begin();
- for (; i != geolocation_renderers_.end(); ++i) {
- needs_updates |= !(i->second.is_paused);
- use_high_accuracy |= i->second.high_accuracy;
- if (needs_updates && use_high_accuracy)
- break;
- }
- if (needs_updates) {
- if (!location_provider_)
- location_provider_ = GeolocationProviderImpl::GetInstance();
- // Re-add to re-establish our options, in case they changed.
- location_provider_->AddLocationUpdateCallback(
- callback_, use_high_accuracy);
- } else {
- if (location_provider_)
+ if (renderer_high_accuracy_.empty()) {
+ if (location_provider_) {
location_provider_->RemoveLocationUpdateCallback(callback_);
- location_provider_ = NULL;
+ location_provider_ = NULL;
+ }
+ } else {
+ if (!location_provider_)
+ location_provider_ = GeolocationProviderImpl::GetInstance();
+ // Re-add to re-establish our options, in case they changed.
+ bool use_high_accuracy = false;
+ std::map<int, bool>::iterator i = renderer_high_accuracy_.begin();
+ for (; i != renderer_high_accuracy_.end(); ++i) {
+ if (i->second) {
+ use_high_accuracy = true;
+ break;
+ }
+ }
+ location_provider_->AddLocationUpdateCallback(callback_, use_high_accuracy);
}
}
} // namespace
diff --git a/content/browser/geolocation/geolocation_dispatcher_host.h b/content/browser/geolocation/geolocation_dispatcher_host.h
index 963a4ec..0631c87 100644
--- a/content/browser/geolocation/geolocation_dispatcher_host.h
+++ b/content/browser/geolocation/geolocation_dispatcher_host.h
@@ -19,10 +19,6 @@
int render_process_id,
GeolocationPermissionContext* geolocation_permission_context);
- // Pause or resumes geolocation for the given |render_view_id|. Should
- // be called on the IO thread. Resuming when nothing is paused is a no-op.
- virtual void PauseOrResume(int render_view_id, bool should_pause) = 0;
-
protected:
GeolocationDispatcherHost();
virtual ~GeolocationDispatcherHost();
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index fcc7f1e..20ea4fc 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -51,6 +51,7 @@
#include "content/browser/download/mhtml_generation_manager.h"
#include "content/browser/fileapi/chrome_blob_storage_context.h"
#include "content/browser/fileapi/fileapi_message_filter.h"
+#include "content/browser/geolocation/geolocation_dispatcher_host.h"
#include "content/browser/gpu/gpu_data_manager_impl.h"
#include "content/browser/gpu/gpu_process_host.h"
#include "content/browser/gpu/shader_disk_cache.h"
@@ -635,19 +636,17 @@
channel_->AddFilter(new IndexedDBDispatcherHost(
GetID(),
storage_partition_impl_->GetIndexedDBContext()));
- geolocation_dispatcher_host_ = NULL;
if (IsGuest()) {
if (!g_browser_plugin_geolocation_context.Get().get()) {
g_browser_plugin_geolocation_context.Get() =
new BrowserPluginGeolocationPermissionContext();
}
- geolocation_dispatcher_host_ = GeolocationDispatcherHost::New(
- GetID(), g_browser_plugin_geolocation_context.Get().get());
+ channel_->AddFilter(GeolocationDispatcherHost::New(
+ GetID(), g_browser_plugin_geolocation_context.Get().get()));
} else {
- geolocation_dispatcher_host_ = GeolocationDispatcherHost::New(
- GetID(), browser_context->GetGeolocationPermissionContext());
+ channel_->AddFilter(GeolocationDispatcherHost::New(
+ GetID(), browser_context->GetGeolocationPermissionContext()));
}
- channel_->AddFilter(geolocation_dispatcher_host_);
gpu_message_filter_ = new GpuMessageFilter(GetID(), widget_helper_.get());
channel_->AddFilter(gpu_message_filter_);
#if defined(ENABLE_WEBRTC)
diff --git a/content/browser/renderer_host/render_process_host_impl.h b/content/browser/renderer_host/render_process_host_impl.h
index 985ef3b..72ec846 100644
--- a/content/browser/renderer_host/render_process_host_impl.h
+++ b/content/browser/renderer_host/render_process_host_impl.h
@@ -13,7 +13,6 @@
#include "base/process/process.h"
#include "base/timer/timer.h"
#include "content/browser/child_process_launcher.h"
-#include "content/browser/geolocation/geolocation_dispatcher_host.h"
#include "content/browser/power_monitor_message_broadcaster.h"
#include "content/common/content_export.h"
#include "content/public/browser/global_request_id.h"
@@ -34,7 +33,6 @@
}
namespace content {
-class GeolocationDispatcherHost;
class GpuMessageFilter;
class PeerConnectionTrackerHost;
class RendererMainThread;
@@ -146,10 +144,6 @@
scoped_ptr<RenderWidgetHostViewFrameSubscriber> subscriber);
void EndFrameSubscription(int route_id);
- GeolocationDispatcherHost* geolocation_dispatcher_host() {
- return geolocation_dispatcher_host_;
- }
-
// Register/unregister the host identified by the host id in the global host
// list.
static void RegisterHost(int host_id, RenderProcessHost* host);
@@ -330,9 +324,6 @@
// Forwards power state messages to the renderer process.
PowerMonitorMessageBroadcaster power_monitor_broadcaster_;
- // Message filter for geolocation messages.
- GeolocationDispatcherHost* geolocation_dispatcher_host_;
-
DISALLOW_COPY_AND_ASSIGN(RenderProcessHostImpl);
};
diff --git a/content/public/browser/android/content_view_core.h b/content/public/browser/android/content_view_core.h
index 9adaf4c..79be4c4 100644
--- a/content/public/browser/android/content_view_core.h
+++ b/content/public/browser/android/content_view_core.h
@@ -55,7 +55,6 @@
virtual void RequestContentClipping(const gfx::Rect& clipping,
const gfx::Size& content_size) = 0;
virtual void PauseVideo() = 0;
- virtual void PauseOrResumeGeolocation(bool should_pause) = 0;
// Observer callback for frame metadata updates.
typedef base::Callback<void(