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

Commit 97d22bb

Browse files
michaelbaiCommit bot
michaelbai
authored and
Commit bot
committed
Deny the geolocation permission request by default.
This patch denies the request if there is no response when GeolocationPermission.Callback object is gc-ed. BUG=513752 Review URL: https://codereview.chromium.org/1267103003 Cr-Commit-Position: refs/heads/master@{#341760}
1 parent 1094ea7 commit 97d22bb

File tree

4 files changed

+144
-47
lines changed

4 files changed

+144
-47
lines changed

‎android_webview/java/src/org/chromium/android_webview/AwContents.java

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@
3737
import android.view.animation.AnimationUtils;
3838
import android.view.inputmethod.EditorInfo;
3939
import android.view.inputmethod.InputConnection;
40-
import android.webkit.GeolocationPermissions;
4140
import android.webkit.JavascriptInterface;
4241
import android.webkit.ValueCallback;
4342
import android.widget.OverScroller;
4443

44+
import org.chromium.android_webview.permission.AwGeolocationCallback;
4545
import org.chromium.android_webview.permission.AwPermissionRequest;
4646
import org.chromium.base.ThreadUtils;
4747
import org.chromium.base.TraceEvent;
@@ -2441,25 +2441,13 @@ private void onReceivedHttpAuthRequest(AwHttpAuthHandler handler, String host, S
24412441
mContentsClient.onReceivedHttpAuthRequest(handler, host, realm);
24422442
}
24432443

2444-
private class AwGeolocationCallback implements GeolocationPermissions.Callback {
2444+
public AwGeolocationPermissions getGeolocationPermissions() {
2445+
return mBrowserContext.getGeolocationPermissions();
2446+
}
24452447

2446-
@Override
2447-
public void invoke(final String origin, final boolean allow, final boolean retain) {
2448-
ThreadUtils.runOnUiThread(new Runnable() {
2449-
@Override
2450-
public void run() {
2451-
if (retain) {
2452-
if (allow) {
2453-
mBrowserContext.getGeolocationPermissions().allow(origin);
2454-
} else {
2455-
mBrowserContext.getGeolocationPermissions().deny(origin);
2456-
}
2457-
}
2458-
if (isDestroyed()) return;
2459-
nativeInvokeGeolocationCallback(mNativeAwContents, allow, origin);
2460-
}
2461-
});
2462-
}
2448+
public void invokeGeolocationCallback(boolean value, String requestingFrame) {
2449+
if (isDestroyed()) return;
2450+
nativeInvokeGeolocationCallback(mNativeAwContents, value, requestingFrame);
24632451
}
24642452

24652453
@CalledByNative
@@ -2478,7 +2466,7 @@ private void onGeolocationPermissionsShowPrompt(String origin) {
24782466
return;
24792467
}
24802468
mContentsClient.onGeolocationPermissionsShowPrompt(
2481-
origin, new AwGeolocationCallback());
2469+
origin, new AwGeolocationCallback(origin, this));
24822470
}
24832471

24842472
@CalledByNative

‎android_webview/java/src/org/chromium/android_webview/permission/AwGeolocationCallback.java

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
// Copyright 2015 The Chromium Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
package org.chromium.android_webview.permission;
6+
7+
import android.webkit.GeolocationPermissions;
8+
9+
import org.chromium.android_webview.AwContents;
10+
import org.chromium.base.Log;
11+
import org.chromium.base.ThreadUtils;
12+
import org.chromium.content.common.CleanupReference;
13+
14+
import java.lang.ref.WeakReference;
15+
16+
/**
17+
* This class implements GeolocationPermissions.Callback, and will be sent to
18+
* WebView applications through WebChromeClient.onGeolocationPermissionsShowPrompt().
19+
*/
20+
public class AwGeolocationCallback implements GeolocationPermissions.Callback {
21+
private static final String TAG = "cr.Geolocation";
22+
23+
private CleanupRunable mCleanupRunable;
24+
private CleanupReference mCleanupReference;
25+
26+
private static class CleanupRunable implements Runnable {
27+
private WeakReference<AwContents> mAwContents;
28+
private boolean mAllow;
29+
private boolean mRetain;
30+
private String mOrigin;
31+
32+
public CleanupRunable(AwContents awContents, String origin) {
33+
mAwContents = new WeakReference<AwContents>(awContents);
34+
mOrigin = origin;
35+
}
36+
37+
@Override
38+
public void run() {
39+
assert ThreadUtils.runningOnUiThread();
40+
AwContents awContents = mAwContents.get();
41+
if (awContents == null) return;
42+
if (mRetain) {
43+
if (mAllow) {
44+
awContents.getGeolocationPermissions().allow(mOrigin);
45+
} else {
46+
awContents.getGeolocationPermissions().deny(mOrigin);
47+
}
48+
}
49+
awContents.invokeGeolocationCallback(mAllow, mOrigin);
50+
}
51+
52+
public void setResponse(String origin, boolean allow, boolean retain) {
53+
mOrigin = origin;
54+
mAllow = allow;
55+
mRetain = retain;
56+
}
57+
}
58+
59+
public AwGeolocationCallback(String origin, AwContents awContents) {
60+
mCleanupRunable = new CleanupRunable(awContents, origin);
61+
mCleanupReference = new CleanupReference(this, mCleanupRunable);
62+
}
63+
64+
@Override
65+
public void invoke(String origin, boolean allow, boolean retain) {
66+
if (mCleanupRunable == null || mCleanupReference == null) {
67+
Log.w(TAG, "Response for this geolocation request has been received."
68+
+ " Ignoring subsequent responses");
69+
return;
70+
}
71+
mCleanupRunable.setResponse(origin, allow, retain);
72+
mCleanupReference.cleanupNow();
73+
mCleanupReference = null;
74+
mCleanupRunable = null;
75+
}
76+
}

‎android_webview/javatests/src/org/chromium/android_webview/test/GeolocationTest.java

Lines changed: 59 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66

77
import android.os.Build;
88
import android.test.suitebuilder.annotation.MediumTest;
9+
import android.test.suitebuilder.annotation.SmallTest;
910
import android.webkit.GeolocationPermissions;
1011

1112
import org.chromium.android_webview.AwContents;
13+
import org.chromium.base.annotations.SuppressFBWarnings;
1214
import org.chromium.base.test.util.Feature;
1315
import org.chromium.base.test.util.MinAndroidSdkLevel;
1416
import org.chromium.content.browser.LocationProviderFactory;
@@ -37,52 +39,65 @@ public class GeolocationTest extends AwTestBase {
3739
+ " function gotPos(position) {\n"
3840
+ " positionCount++;\n"
3941
+ " }\n"
42+
+ " function errorCallback(error){"
43+
+ " window.document.title = 'deny';"
44+
+ " console.log('navigator.getCurrentPosition error: ', error);"
45+
+ " }"
4046
+ " function initiate_getCurrentPosition() {\n"
4147
+ " navigator.geolocation.getCurrentPosition(\n"
42-
+ " gotPos, function() { }, { });\n"
48+
+ " gotPos, errorCallback, { });\n"
4349
+ " }\n"
4450
+ " function initiate_watchPosition() {\n"
4551
+ " navigator.geolocation.watchPosition(\n"
46-
+ " gotPos, function() { }, { });\n"
52+
+ " gotPos, errorCallback, { });\n"
4753
+ " }\n"
4854
+ " </script>\n"
4955
+ " </head>\n"
5056
+ " <body>\n"
5157
+ " </body>\n"
5258
+ "</html>";
5359

54-
@Override
55-
public void setUp() throws Exception {
56-
super.setUp();
57-
mContentsClient = new TestAwContentsClient() {
58-
@Override
59-
public void onGeolocationPermissionsShowPrompt(String origin,
60-
GeolocationPermissions.Callback callback) {
61-
callback.invoke(origin, true, true);
62-
}
63-
};
64-
mAwContents = createAwTestContainerViewOnMainSync(mContentsClient).getAwContents();
65-
enableJavaScriptOnUiThread(mAwContents);
66-
setupGeolocation();
60+
private static class GrantPermisionAwContentClient extends TestAwContentsClient {
61+
@Override
62+
public void onGeolocationPermissionsShowPrompt(String origin,
63+
GeolocationPermissions.Callback callback) {
64+
callback.invoke(origin, true, true);
65+
}
6766
}
6867

69-
@Override
70-
public void tearDown() throws Exception {
71-
mMockLocationProvider.stopUpdates();
72-
super.tearDown();
68+
private static class DefaultPermisionAwContentClient extends TestAwContentsClient {
69+
@Override
70+
public void onGeolocationPermissionsShowPrompt(String origin,
71+
GeolocationPermissions.Callback callback) {
72+
// This method is empty intentionally to simulate callback is not referenced.
73+
}
7374
}
7475

75-
private void setupGeolocation() {
76+
private void initAwContents(TestAwContentsClient contentsClient) throws Exception {
77+
mContentsClient = contentsClient;
78+
mAwContents = createAwTestContainerViewOnMainSync(mContentsClient).getAwContents();
79+
enableJavaScriptOnUiThread(mAwContents);
7680
getInstrumentation().runOnMainSync(new Runnable() {
7781
@Override
7882
public void run() {
7983
mAwContents.getSettings().setGeolocationEnabled(true);
8084
}
8185
});
86+
}
87+
88+
@Override
89+
public void setUp() throws Exception {
90+
super.setUp();
8291
mMockLocationProvider = new MockLocationProvider();
8392
LocationProviderFactory.setLocationProviderImpl(mMockLocationProvider);
8493
}
8594

95+
@Override
96+
public void tearDown() throws Exception {
97+
mMockLocationProvider.stopUpdates();
98+
super.tearDown();
99+
}
100+
86101
private int getPositionCountFromJS() {
87102
int result = -1;
88103
try {
@@ -110,6 +125,7 @@ public Boolean call() throws Exception {
110125
@MediumTest
111126
@Feature({"AndroidWebView"})
112127
public void testGetPosition() throws Throwable {
128+
initAwContents(new GrantPermisionAwContentClient());
113129
loadDataSync(mAwContents, mContentsClient.getOnPageFinishedHelper(),
114130
RAW_HTML, "text/html", false);
115131

@@ -137,6 +153,7 @@ public Boolean call() throws Exception {
137153
@MediumTest
138154
@Feature({"AndroidWebView"})
139155
public void testWatchPosition() throws Throwable {
156+
initAwContents(new GrantPermisionAwContentClient());
140157
loadDataSync(mAwContents, mContentsClient.getOnPageFinishedHelper(),
141158
RAW_HTML, "text/html", false);
142159

@@ -153,6 +170,7 @@ public Boolean call() throws Exception {
153170
@MediumTest
154171
@Feature({"AndroidWebView"})
155172
public void testPauseGeolocationOnPause() throws Throwable {
173+
initAwContents(new GrantPermisionAwContentClient());
156174
// Start a watch going.
157175
loadDataSync(mAwContents, mContentsClient.getOnPageFinishedHelper(),
158176
RAW_HTML, "text/html", false);
@@ -204,6 +222,7 @@ public Boolean call() throws Exception {
204222
@MediumTest
205223
@Feature({"AndroidWebView"})
206224
public void testPauseAwContentsBeforeNavigating() throws Throwable {
225+
initAwContents(new GrantPermisionAwContentClient());
207226
getInstrumentation().runOnMainSync(new Runnable() {
208227
@Override
209228
public void run() {
@@ -241,6 +260,7 @@ public Boolean call() throws Exception {
241260
@MediumTest
242261
@Feature({"AndroidWebView"})
243262
public void testResumeWhenNotStarted() throws Throwable {
263+
initAwContents(new GrantPermisionAwContentClient());
244264
getInstrumentation().runOnMainSync(new Runnable() {
245265
@Override
246266
public void run() {
@@ -261,4 +281,23 @@ public void run() {
261281
ensureGeolocationRunning(false);
262282
}
263283

284+
@Feature({"AndroidWebView"})
285+
@SmallTest
286+
public void testDenyAccessByDefault() throws Throwable {
287+
initAwContents(new DefaultPermisionAwContentClient());
288+
loadDataSync(mAwContents, mContentsClient.getOnPageFinishedHelper(),
289+
RAW_HTML, "text/html", false);
290+
291+
mAwContents.evaluateJavaScriptForTests("initiate_getCurrentPosition();", null);
292+
293+
poll(new Callable<Boolean>() {
294+
@SuppressFBWarnings("DM_GC")
295+
@Override
296+
public Boolean call() throws Exception {
297+
Runtime.getRuntime().gc();
298+
return "deny".equals(getTitleOnUiThread(mAwContents));
299+
}
300+
});
301+
}
302+
264303
}

‎android_webview/native/aw_contents.cc

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -491,11 +491,8 @@ void AwContents::InvokeGeolocationCallback(JNIEnv* env,
491491
jboolean value,
492492
jstring origin) {
493493
DCHECK_CURRENTLY_ON(BrowserThread::UI);
494-
if (pending_geolocation_prompts_.empty()) {
495-
LOG(WARNING) << "Response for this geolocation request has been received."
496-
<< " Ignoring subsequent responses";
494+
if (pending_geolocation_prompts_.empty())
497495
return;
498-
}
499496

500497
GURL callback_origin(base::android::ConvertJavaStringToUTF16(env, origin));
501498
if (callback_origin.GetOrigin() ==
@@ -506,9 +503,6 @@ void AwContents::InvokeGeolocationCallback(JNIEnv* env,
506503
ShowGeolocationPromptHelper(java_ref_,
507504
pending_geolocation_prompts_.front().first);
508505
}
509-
} else {
510-
LOG(WARNING) << "Response for this geolocation request has been received."
511-
<< " Ignoring subsequent responses";
512506
}
513507
}
514508

0 commit comments

Comments
 (0)