Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Cherry pick into M41 commit e10a825
Browse files Browse the repository at this point in the history 
"Original CL description:
Remove thread jumping from the geolocation permission context.

This fixes a crash in the browser when the context is freed before from the
callback.

We used to jump to the blocking pool to avoid strict mode violation on startup
but the code has been refactored since then and we no longer hit such strict mode violation.
"

BUG=447282

Review URL: https://codereview.chromium.org/836963004

Cr-Commit-Position: refs/branch-heads/2272@{#51}
Cr-Branched-From: 827a380-refs/heads/master@{#310958}
  • Loading branch information
@miguelgarciaarribas
miguelgarciaarribas committed Jan 19, 2015
1 parent 4578943 commit 819885c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 114 deletions.
83 changes: 10 additions & 73 deletions chrome/browser/geolocation/geolocation_permission_context_android.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,97 +4,34 @@

#include "chrome/browser/geolocation/geolocation_permission_context_android.h"

#include "base/prefs/pref_service.h"
#include "chrome/browser/android/google_location_settings_helper.h"
#include "chrome/browser/profiles/profile.h"
#include "content/public/browser/browser_thread.h"
#include "components/content_settings/core/common/permission_request_id.h"
#include "content/public/browser/web_contents.h"

GeolocationPermissionContextAndroid::
PermissionRequestInfo::PermissionRequestInfo()
: id(0, 0, 0, GURL()),
user_gesture(false) {
}
#include "url/gurl.h"

GeolocationPermissionContextAndroid::
GeolocationPermissionContextAndroid(Profile* profile)
: GeolocationPermissionContext(profile),
google_location_settings_helper_(
GoogleLocationSettingsHelper::Create()),
weak_factory_(this) {
GoogleLocationSettingsHelper::Create()) {
}

GeolocationPermissionContextAndroid::~GeolocationPermissionContextAndroid() {
}

void GeolocationPermissionContextAndroid::ProceedDecidePermission(
content::WebContents* web_contents,
const PermissionRequestInfo& info,
base::Callback<void(bool)> callback) {

// The super class implementation expects everything in UI thread.
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
GeolocationPermissionContext::RequestPermission(
web_contents, info.id,
info.requesting_origin,
info.user_gesture,
callback);
}


// UI thread
void GeolocationPermissionContextAndroid::RequestPermission(
content::WebContents* web_contents,
const PermissionRequestID& id,
const GURL& requesting_frame_origin,
bool user_gesture,
const BrowserPermissionCallback& callback) {
PermissionRequestInfo info;
info.id = id;
info.requesting_origin = requesting_frame_origin;
info.embedder_origin = web_contents->GetLastCommittedURL().GetOrigin();
info.user_gesture = user_gesture;

// Called on the UI thread. However, do the work on a separate thread
// to avoid strict mode violation.
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
content::BrowserThread::PostBlockingPoolTask(FROM_HERE,
base::Bind(
&GeolocationPermissionContextAndroid::CheckMasterLocation,
weak_factory_.GetWeakPtr(), web_contents, info, callback));
}

// Blocking pool thread
void GeolocationPermissionContextAndroid::CheckMasterLocation(
content::WebContents* web_contents,
const PermissionRequestInfo& info,
const BrowserPermissionCallback& callback) {
// Check to see if the feature in its entirety has been disabled.
// This must happen before other services (e.g. tabs, extensions)
// get an opportunity to allow the geolocation request.
bool enabled =
google_location_settings_helper_->IsSystemLocationEnabled();

base::Closure ui_closure;
if (enabled) {
ui_closure = base::Bind(
&GeolocationPermissionContextAndroid::ProceedDecidePermission,
base::Unretained(this), web_contents, info, callback);
} else {
ui_closure = base::Bind(
&GeolocationPermissionContextAndroid::PermissionDecided,
base::Unretained(this), info.id, info.requesting_origin,
info.embedder_origin, callback, false, false);
if (!google_location_settings_helper_->IsSystemLocationEnabled()) {
PermissionDecided(id, requesting_frame_origin,
web_contents->GetLastCommittedURL().GetOrigin(),
callback, false /* persist */, false /* granted */);
return;
}

// This method is executed from the BlockingPool, post the result
// back to the UI thread.
content::BrowserThread::PostTask(
content::BrowserThread::UI, FROM_HERE, ui_closure);
}


void GeolocationPermissionContextAndroid::InterceptPermissionCheck(
const BrowserPermissionCallback& callback, bool granted) {
callback.Run(granted);
GeolocationPermissionContext::RequestPermission(
web_contents, id, requesting_frame_origin, user_gesture, callback);
}
49 changes: 8 additions & 41 deletions chrome/browser/geolocation/geolocation_permission_context_android.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
// The flow for geolocation permissions on Android needs to take into account
// the global geolocation settings so it differs from the desktop one. It
// works as follows.
// GeolocationPermissionContextAndroid::DecidePermission intercepts the flow in
// the UI thread, and posts a task to the blocking pool to CheckSystemLocation.
// CheckSystemLocation will in fact check several possible settings
// GeolocationPermissionContextAndroid::RequestPermission intercepts the flow
// and proceeds to check the system location.
// This will in fact check several possible settings
// - The global system geolocation setting
// - The Google location settings on pre KK devices
// - An old internal Chrome setting on pre-JB MR1 devices
Expand All @@ -20,67 +20,34 @@
// infobars, etc.).
//
// Otherwise the permission is already decided.

// There is a bit of thread jumping since some of the permissions (like the
// per site settings) are queried on the UI thread while the system level
// permissions are considered I/O and thus checked in the blocking thread pool.

#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/geolocation/geolocation_permission_context.h"
#include "components/content_settings/core/common/permission_request_id.h"
#include "url/gurl.h"

namespace content {
class WebContents;
}

class GoogleLocationSettingsHelper;
class GURL;
class PermissionRequestID;

class GeolocationPermissionContextAndroid
: public GeolocationPermissionContext {
public:
explicit GeolocationPermissionContextAndroid(Profile* profile);
virtual ~GeolocationPermissionContextAndroid();
~GeolocationPermissionContextAndroid() override;

private:
struct PermissionRequestInfo {
PermissionRequestInfo();

PermissionRequestID id;
GURL requesting_origin;
GURL embedder_origin;
bool user_gesture;
};

// PermissionContextBase:
virtual void RequestPermission(
// GeolocationPermissionContext:
void RequestPermission(
content::WebContents* web_contents,
const PermissionRequestID& id,
const GURL& requesting_frame_origin,
bool user_gesture,
const BrowserPermissionCallback& callback) override;

void CheckMasterLocation(content::WebContents* web_contents,
const PermissionRequestInfo& info,
const BrowserPermissionCallback& callback);

void ProceedDecidePermission(content::WebContents* web_contents,
const PermissionRequestInfo& info,
base::Callback<void(bool)> callback);

// Will perform a final check on the system location settings before
// granting the permission.
void InterceptPermissionCheck(const BrowserPermissionCallback& callback,
bool granted);

scoped_ptr<GoogleLocationSettingsHelper> google_location_settings_helper_;
base::WeakPtrFactory<GeolocationPermissionContextAndroid> weak_factory_;

private:
void CheckSystemLocation(content::WebContents* web_contents,
const PermissionRequestInfo& info,
base::Callback<void(bool)> callback);

DISALLOW_COPY_AND_ASSIGN(GeolocationPermissionContextAndroid);
};
Expand Down

0 comments on commit 819885c

Please sign in to comment.