Commit 261fc18b authored by Michael Ershov's avatar Michael Ershov Committed by Commit Bot

Revert "[WebLayer] Switch geolocation tests to use permission prompt"

This reverts commit 6966bd53.

Reason for revert: The tests became flaky, see crbug.com/1064500.

Original change's description:
> [WebLayer] Switch geolocation tests to use permission prompt
> 
> These tests were previously using the fake permission manager which
> always grants permissions. Now we can use the real permission manager,
> and the fake permission manager can be removed.
> 
> Fixes a strict mode violation in permission dialogs (similar to fixes
> from https://crrev.com/c/2108603).
> 
> Bug: 1025625, 1025609
> Change-Id: I8b756c61ee213151e53d68a375ffa4d3f8fd7643
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2111061
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#752494}

TBR=jam@chromium.org,cduvall@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1025625, 1025609, 1064500
Change-Id: I881f788accb3968955410e1ea413949a72688ecd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2128069Reviewed-by: default avatarMichael Ershov <miersh@google.com>
Commit-Queue: Michael Ershov <miersh@google.com>
Cr-Commit-Position: refs/heads/master@{#754859}
parent 22bd480c
......@@ -12,7 +12,6 @@ import android.widget.TextView;
import androidx.core.widget.TextViewCompat;
import org.chromium.base.StrictModeContext;
import org.chromium.components.browser_ui.modaldialog.R;
import org.chromium.ui.modaldialog.ModalDialogProperties;
import org.chromium.ui.modelutil.PropertyModel;
......@@ -25,7 +24,8 @@ class PermissionDialogModel {
ModalDialogProperties.Controller controller, PermissionDialogDelegate delegate) {
Context context = delegate.getWindow().getContext().get();
assert context != null;
View customView = loadDialogView(context);
LayoutInflater inflater = LayoutInflater.from(context);
View customView = inflater.inflate(R.layout.permission_dialog, null);
String messageText = delegate.getMessageText();
assert !TextUtils.isEmpty(messageText);
......@@ -44,12 +44,4 @@ class PermissionDialogModel {
.with(ModalDialogProperties.FILTER_TOUCH_FOR_SECURITY, true)
.build();
}
private static View loadDialogView(Context context) {
// LayoutInflater may access the disk.
try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
LayoutInflater inflater = LayoutInflater.from(context);
return inflater.inflate(R.layout.permission_dialog, null);
}
}
}
......@@ -121,6 +121,8 @@ source_set("weblayer_lib_base") {
"browser/download_impl.h",
"browser/download_manager_delegate_impl.cc",
"browser/download_manager_delegate_impl.h",
"browser/fake_permission_controller_delegate.cc",
"browser/fake_permission_controller_delegate.h",
"browser/feature_list_creator.cc",
"browser/feature_list_creator.h",
"browser/file_select_helper.cc",
......
......@@ -14,6 +14,7 @@ import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.content_public.browser.test.util.CriteriaHelper;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.net.test.util.TestWebServer;
......@@ -26,6 +27,7 @@ import org.chromium.weblayer.shell.InstrumentationActivity;
* Tests that Geolocation Web API works as expected.
*/
@RunWith(WebLayerJUnit4ClassRunner.class)
@CommandLineFlags.Add("weblayer-fake-permissions")
public final class GeolocationTest {
@Rule
public InstrumentationActivityTestRule mActivityTestRule =
......@@ -92,8 +94,6 @@ public final class GeolocationTest {
@MediumTest
public void testGeolocation_getPosition() throws Throwable {
mActivityTestRule.executeScriptSync("initiate_getCurrentPosition();", false);
waitForDialog();
mTestWebLayer.clickPermissionDialogButton(true);
waitForCountEqual("positionCount", 1);
mActivityTestRule.executeScriptSync("initiate_getCurrentPosition();", false);
waitForCountEqual("positionCount", 2);
......@@ -107,8 +107,6 @@ public final class GeolocationTest {
@MediumTest
public void testGeolocation_watchPosition() throws Throwable {
mActivityTestRule.executeScriptSync("initiate_watchPosition();", false);
waitForDialog();
mTestWebLayer.clickPermissionDialogButton(true);
waitForCountGreaterThan("positionCount", 1);
ensureGeolocationIsRunning(true);
Assert.assertEquals(0, getCountFromJS("errorCount"));
......@@ -121,8 +119,6 @@ public final class GeolocationTest {
@MediumTest
public void testGeolocation_destroyTabStopsGeolocation() throws Throwable {
mActivityTestRule.executeScriptSync("initiate_watchPosition();", false);
waitForDialog();
mTestWebLayer.clickPermissionDialogButton(true);
ensureGeolocationIsRunning(true);
TestThreadUtils.runOnUiThreadBlocking(() -> {
Browser browser = mActivity.getBrowser();
......@@ -147,16 +143,6 @@ public final class GeolocationTest {
Assert.assertEquals(0, getCountFromJS("positionCount"));
}
@Test
@MediumTest
public void testGeolocation_denyFromPrompt() throws Throwable {
mActivityTestRule.executeScriptSync("initiate_watchPosition();", false);
waitForDialog();
mTestWebLayer.clickPermissionDialogButton(false);
waitForCountEqual("errorCount", 1);
Assert.assertEquals(0, getCountFromJS("positionCount"));
}
// helper methods
private void waitForCountEqual(String variableName, int count) {
......@@ -164,11 +150,6 @@ public final class GeolocationTest {
() -> { return getCountFromJS(variableName) == count; });
}
private void waitForDialog() {
CriteriaHelper.pollInstrumentationThread(
() -> { return mTestWebLayer.isPermissionDialogShown(); });
}
private void waitForCountGreaterThan(String variableName, int count) {
CriteriaHelper.pollInstrumentationThread(
() -> { return getCountFromJS(variableName) > count; });
......
......@@ -23,6 +23,7 @@
#include "content/public/browser/device_service.h"
#include "content/public/browser/download_request_utils.h"
#include "content/public/browser/resource_context.h"
#include "weblayer/browser/fake_permission_controller_delegate.h"
#include "weblayer/browser/permissions/permission_manager_factory.h"
#include "weblayer/browser/stateful_ssl_host_state_delegate_factory.h"
#include "weblayer/public/common/switches.h"
......@@ -153,6 +154,14 @@ content::SSLHostStateDelegate* BrowserContextImpl::GetSSLHostStateDelegate() {
content::PermissionControllerDelegate*
BrowserContextImpl::GetPermissionControllerDelegate() {
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kWebLayerFakePermissions)) {
if (!permission_controller_delegate_) {
permission_controller_delegate_ =
std::make_unique<FakePermissionControllerDelegate>();
}
return permission_controller_delegate_.get();
}
return PermissionManagerFactory::GetForBrowserContext(this);
}
......
......@@ -86,6 +86,8 @@ class BrowserContextImpl : public content::BrowserContext {
resource_context_;
DownloadManagerDelegateImpl download_delegate_;
std::unique_ptr<PrefService> user_pref_service_;
std::unique_ptr<content::PermissionControllerDelegate>
permission_controller_delegate_;
std::unique_ptr<WebLayerVariationsClient> weblayer_variations_client_;
};
} // namespace weblayer
......
......@@ -207,6 +207,15 @@ ContentBrowserClientImpl::CreateBrowserMainParts(
return browser_main_parts;
}
void ContentBrowserClientImpl::AppendExtraCommandLineSwitches(
base::CommandLine* command_line,
int child_process_id) {
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kWebLayerFakePermissions)) {
command_line->AppendSwitch(switches::kWebLayerFakePermissions);
}
}
std::string ContentBrowserClientImpl::GetApplicationLocale() {
return i18n::GetApplicationLocale();
}
......
......@@ -29,6 +29,8 @@ class ContentBrowserClientImpl : public content::ContentBrowserClient {
// ContentBrowserClient overrides.
std::unique_ptr<content::BrowserMainParts> CreateBrowserMainParts(
const content::MainFunctionParams& parameters) override;
void AppendExtraCommandLineSwitches(base::CommandLine* command_line,
int child_process_id) override;
std::string GetApplicationLocale() override;
std::string GetAcceptLangs(content::BrowserContext* context) override;
content::WebContentsViewDelegate* GetWebContentsViewDelegate(
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "weblayer/browser/fake_permission_controller_delegate.h"
#include "base/callback.h"
#include "content/public/browser/permission_controller.h"
#include "content/public/browser/permission_type.h"
#include "content/public/browser/web_contents.h"
namespace weblayer {
FakePermissionControllerDelegate::FakePermissionControllerDelegate() = default;
FakePermissionControllerDelegate::~FakePermissionControllerDelegate() = default;
int FakePermissionControllerDelegate::RequestPermission(
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
bool user_gesture,
base::OnceCallback<void(blink::mojom::PermissionStatus)> callback) {
std::move(callback).Run(blink::mojom::PermissionStatus::GRANTED);
return content::PermissionController::kNoPendingOperation;
}
int FakePermissionControllerDelegate::RequestPermissions(
const std::vector<content::PermissionType>& permissions,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
bool user_gesture,
base::OnceCallback<void(const std::vector<blink::mojom::PermissionStatus>&)>
callback) {
std::vector<blink::mojom::PermissionStatus> result(
permissions.size(), blink::mojom::PermissionStatus::GRANTED);
std::move(callback).Run(result);
return content::PermissionController::kNoPendingOperation;
}
void FakePermissionControllerDelegate::ResetPermission(
content::PermissionType permission,
const GURL& requesting_origin,
const GURL& embedding_origin) {}
blink::mojom::PermissionStatus
FakePermissionControllerDelegate::GetPermissionStatus(
content::PermissionType permission,
const GURL& requesting_origin,
const GURL& embedding_origin) {
return blink::mojom::PermissionStatus::GRANTED;
}
blink::mojom::PermissionStatus
FakePermissionControllerDelegate::GetPermissionStatusForFrame(
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin) {
return GetPermissionStatus(
permission, requesting_origin,
content::WebContents::FromRenderFrameHost(render_frame_host)
->GetLastCommittedURL()
.GetOrigin());
}
int FakePermissionControllerDelegate::SubscribePermissionStatusChange(
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)> callback) {
return content::PermissionController::kNoPendingOperation;
}
void FakePermissionControllerDelegate::UnsubscribePermissionStatusChange(
int subscription_id) {}
} // namespace weblayer
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef WEBLAYER_BROWSER_FAKE_PERMISSION_CONTROLLER_DELEGATE_H_
#define WEBLAYER_BROWSER_FAKE_PERMISSION_CONTROLLER_DELEGATE_H_
#include "base/callback_forward.h"
#include "base/macros.h"
#include "content/public/browser/permission_controller_delegate.h"
namespace weblayer {
// Temporary permission controller delegate which grants all permissions. Once
// permissions have been implemented, this will be removed. This is only used if
// the --weblayer-fake-permissions switch is passed on the command line.
class FakePermissionControllerDelegate
: public content::PermissionControllerDelegate {
public:
FakePermissionControllerDelegate();
~FakePermissionControllerDelegate() override;
// PermissionControllerDelegate implementation.
int RequestPermission(content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
bool user_gesture,
base::OnceCallback<void(blink::mojom::PermissionStatus)>
callback) override;
int RequestPermissions(
const std::vector<content::PermissionType>& permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
bool user_gesture,
base::OnceCallback<
void(const std::vector<blink::mojom::PermissionStatus>&)> callback)
override;
void ResetPermission(content::PermissionType permission,
const GURL& requesting_origin,
const GURL& embedding_origin) override;
blink::mojom::PermissionStatus GetPermissionStatus(
content::PermissionType permission,
const GURL& requesting_origin,
const GURL& embedding_origin) override;
blink::mojom::PermissionStatus GetPermissionStatusForFrame(
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin) override;
int SubscribePermissionStatusChange(
content::PermissionType permission,
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
base::RepeatingCallback<void(blink::mojom::PermissionStatus)> callback)
override;
void UnsubscribePermissionStatusChange(int subscription_id) override;
private:
DISALLOW_COPY_AND_ASSIGN(FakePermissionControllerDelegate);
};
} // namespace weblayer
#endif // WEBLAYER_BROWSER_FAKE_PERMISSION_CONTROLLER_DELEGATE_H_
......@@ -156,12 +156,9 @@ android_library("test_java") {
sources = [ "org/chromium/weblayer_private/test/TestWebLayerImpl.java" ]
deps = [
":weblayer_test_resources",
"//components/permissions/android:java",
"//content/public/test/android:content_java_test_support",
"//net/android:net_java",
"//services/device/public/java:geolocation_java",
"//services/device/public/java:geolocation_java_test_support",
"//ui/android:ui_full_java",
]
srcjar_deps = [ ":test_aidl" ]
}
......
......@@ -7,16 +7,11 @@ package org.chromium.weblayer_private.test;
import android.os.IBinder;
import org.chromium.base.annotations.UsedByReflection;
import org.chromium.components.permissions.PermissionDialogController;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.device.geolocation.LocationProviderOverrider;
import org.chromium.device.geolocation.MockLocationProvider;
import org.chromium.net.NetworkChangeNotifier;
import org.chromium.ui.modaldialog.ModalDialogProperties;
import org.chromium.weblayer_private.test_interfaces.ITestWebLayer;
import java.util.concurrent.ExecutionException;
/**
* Root implementation class for TestWebLayer.
*/
......@@ -51,24 +46,4 @@ public final class TestWebLayerImpl extends ITestWebLayer.Stub {
public boolean isMockLocationProviderRunning() {
return mMockLocationProvider.isRunning();
}
@Override
public boolean isPermissionDialogShown() {
try {
return TestThreadUtils.runOnUiThreadBlocking(() -> {
return PermissionDialogController.getInstance().isDialogShownForTest();
});
} catch (ExecutionException e) {
return false;
}
}
@Override
public void clickPermissionDialogButton(boolean allow) {
TestThreadUtils.runOnUiThreadBlocking(() -> {
PermissionDialogController.getInstance().clickButtonForTest(allow
? ModalDialogProperties.ButtonType.POSITIVE
: ModalDialogProperties.ButtonType.NEGATIVE);
});
}
}
}
\ No newline at end of file
......@@ -10,10 +10,4 @@ interface ITestWebLayer {
// set mock location provider
void setMockLocationProvider(in boolean enable) = 2;
boolean isMockLocationProviderRunning() = 3;
// Whether or not a permission dialog is currently showing.
boolean isPermissionDialogShown() = 4;
// Clicks a button on the permission dialog.
void clickPermissionDialogButton(in boolean allow) = 5;
}
......@@ -9,4 +9,8 @@ namespace switches {
// Makes WebLayer Shell use the given path for its data directory.
const char kWebLayerUserDataDir[] = "weblayer-user-data-dir";
// Makes WebLayer use a fake permission controller delegate which grants all
// permissions.
const char kWebLayerFakePermissions[] = "weblayer-fake-permissions";
} // namespace switches
......@@ -8,6 +8,7 @@
namespace switches {
extern const char kWebLayerUserDataDir[];
extern const char kWebLayerFakePermissions[];
} // namespace switches
......
......@@ -63,12 +63,4 @@ public final class TestWebLayer {
public boolean isMockLocationProviderRunning() throws RemoteException {
return mITestWebLayer.isMockLocationProviderRunning();
}
public boolean isPermissionDialogShown() throws RemoteException {
return mITestWebLayer.isPermissionDialogShown();
}
public void clickPermissionDialogButton(boolean allow) throws RemoteException {
mITestWebLayer.clickPermissionDialogButton(allow);
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment