diff --git a/components/permissions/android/java/src/org/chromium/components/permissions/PermissionDialogModel.java b/components/permissions/android/java/src/org/chromium/components/permissions/PermissionDialogModel.java index 0b6c34276f9dd5..d9bc61cbb047d9 100644 --- a/components/permissions/android/java/src/org/chromium/components/permissions/PermissionDialogModel.java +++ b/components/permissions/android/java/src/org/chromium/components/permissions/PermissionDialogModel.java @@ -12,6 +12,7 @@ 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; @@ -24,8 +25,7 @@ public static PropertyModel getModel( ModalDialogProperties.Controller controller, PermissionDialogDelegate delegate) { Context context = delegate.getWindow().getContext().get(); assert context != null; - LayoutInflater inflater = LayoutInflater.from(context); - View customView = inflater.inflate(R.layout.permission_dialog, null); + View customView = loadDialogView(context); String messageText = delegate.getMessageText(); assert !TextUtils.isEmpty(messageText); @@ -44,4 +44,12 @@ public static PropertyModel getModel( .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); + } + } } diff --git a/weblayer/BUILD.gn b/weblayer/BUILD.gn index ad1e1a342babf7..2152e8215b1380 100644 --- a/weblayer/BUILD.gn +++ b/weblayer/BUILD.gn @@ -108,8 +108,6 @@ 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", diff --git a/weblayer/browser/android/javatests/src/org/chromium/weblayer/test/GeolocationTest.java b/weblayer/browser/android/javatests/src/org/chromium/weblayer/test/GeolocationTest.java index 8be2e7d56f7b26..2bcf0a32d81690 100644 --- a/weblayer/browser/android/javatests/src/org/chromium/weblayer/test/GeolocationTest.java +++ b/weblayer/browser/android/javatests/src/org/chromium/weblayer/test/GeolocationTest.java @@ -14,7 +14,6 @@ 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; @@ -27,7 +26,6 @@ * Tests that Geolocation Web API works as expected. */ @RunWith(WebLayerJUnit4ClassRunner.class) -@CommandLineFlags.Add("weblayer-fake-permissions") public final class GeolocationTest { @Rule public InstrumentationActivityTestRule mActivityTestRule = @@ -94,6 +92,8 @@ public void tearDown() throws Throwable { @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,6 +107,8 @@ public void testGeolocation_getPosition() throws Throwable { @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")); @@ -119,6 +121,8 @@ public void testGeolocation_watchPosition() throws Throwable { @MediumTest public void testGeolocation_destroyTabStopsGeolocation() throws Throwable { mActivityTestRule.executeScriptSync("initiate_watchPosition();", false); + waitForDialog(); + mTestWebLayer.clickPermissionDialogButton(true); ensureGeolocationIsRunning(true); TestThreadUtils.runOnUiThreadBlocking(() -> { Browser browser = mActivity.getBrowser(); @@ -143,6 +147,16 @@ public void testGeolocation_denyOnInsecureOrigins() throws Throwable { 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) { @@ -150,6 +164,11 @@ private void waitForCountEqual(String variableName, int count) { () -> { 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; }); diff --git a/weblayer/browser/browser_context_impl.cc b/weblayer/browser/browser_context_impl.cc index f53516fc0ba372..07f58b384f483f 100644 --- a/weblayer/browser/browser_context_impl.cc +++ b/weblayer/browser/browser_context_impl.cc @@ -23,7 +23,6 @@ #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,14 +152,6 @@ 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(); - } - return permission_controller_delegate_.get(); - } return PermissionManagerFactory::GetForBrowserContext(this); } diff --git a/weblayer/browser/browser_context_impl.h b/weblayer/browser/browser_context_impl.h index c416405f02db39..90d65f562904e4 100644 --- a/weblayer/browser/browser_context_impl.h +++ b/weblayer/browser/browser_context_impl.h @@ -86,8 +86,6 @@ class BrowserContextImpl : public content::BrowserContext { resource_context_; DownloadManagerDelegateImpl download_delegate_; std::unique_ptr user_pref_service_; - std::unique_ptr - permission_controller_delegate_; std::unique_ptr weblayer_variations_client_; }; } // namespace weblayer diff --git a/weblayer/browser/content_browser_client_impl.cc b/weblayer/browser/content_browser_client_impl.cc index d223e8e832e903..f6b8011c96d0dc 100644 --- a/weblayer/browser/content_browser_client_impl.cc +++ b/weblayer/browser/content_browser_client_impl.cc @@ -176,15 +176,6 @@ 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(); } diff --git a/weblayer/browser/content_browser_client_impl.h b/weblayer/browser/content_browser_client_impl.h index 254e2721224850..f3641d797503f5 100644 --- a/weblayer/browser/content_browser_client_impl.h +++ b/weblayer/browser/content_browser_client_impl.h @@ -29,8 +29,6 @@ class ContentBrowserClientImpl : public content::ContentBrowserClient { // ContentBrowserClient overrides. std::unique_ptr 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( diff --git a/weblayer/browser/fake_permission_controller_delegate.cc b/weblayer/browser/fake_permission_controller_delegate.cc deleted file mode 100644 index bc9564d16e2e0f..00000000000000 --- a/weblayer/browser/fake_permission_controller_delegate.cc +++ /dev/null @@ -1,77 +0,0 @@ -// 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 callback) { - std::move(callback).Run(blink::mojom::PermissionStatus::GRANTED); - return content::PermissionController::kNoPendingOperation; -} - -int FakePermissionControllerDelegate::RequestPermissions( - const std::vector& permissions, - content::RenderFrameHost* render_frame_host, - const GURL& requesting_origin, - bool user_gesture, - base::OnceCallback&)> - callback) { - std::vector 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 callback) { - return content::PermissionController::kNoPendingOperation; -} - -void FakePermissionControllerDelegate::UnsubscribePermissionStatusChange( - int subscription_id) {} - -} // namespace weblayer diff --git a/weblayer/browser/fake_permission_controller_delegate.h b/weblayer/browser/fake_permission_controller_delegate.h deleted file mode 100644 index 018ffd99d6b42f..00000000000000 --- a/weblayer/browser/fake_permission_controller_delegate.h +++ /dev/null @@ -1,63 +0,0 @@ -// 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 - callback) override; - int RequestPermissions( - const std::vector& permission, - content::RenderFrameHost* render_frame_host, - const GURL& requesting_origin, - bool user_gesture, - base::OnceCallback< - void(const std::vector&)> 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 callback) - override; - void UnsubscribePermissionStatusChange(int subscription_id) override; - - private: - DISALLOW_COPY_AND_ASSIGN(FakePermissionControllerDelegate); -}; - -} // namespace weblayer - -#endif // WEBLAYER_BROWSER_FAKE_PERMISSION_CONTROLLER_DELEGATE_H_ diff --git a/weblayer/browser/java/BUILD.gn b/weblayer/browser/java/BUILD.gn index e67d4cbad69bbb..aef77fc7b7c52e 100644 --- a/weblayer/browser/java/BUILD.gn +++ b/weblayer/browser/java/BUILD.gn @@ -143,9 +143,12 @@ 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" ] } diff --git a/weblayer/browser/java/org/chromium/weblayer_private/test/TestWebLayerImpl.java b/weblayer/browser/java/org/chromium/weblayer_private/test/TestWebLayerImpl.java index 1fc308463dfbe7..9b6c3b0aa888a3 100644 --- a/weblayer/browser/java/org/chromium/weblayer_private/test/TestWebLayerImpl.java +++ b/weblayer/browser/java/org/chromium/weblayer_private/test/TestWebLayerImpl.java @@ -7,11 +7,16 @@ 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. */ @@ -46,4 +51,24 @@ public void setMockLocationProvider(boolean enable) { public boolean isMockLocationProviderRunning() { return mMockLocationProvider.isRunning(); } -} \ No newline at end of file + + @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); + }); + } +} diff --git a/weblayer/browser/java/org/chromium/weblayer_private/test_interfaces/ITestWebLayer.aidl b/weblayer/browser/java/org/chromium/weblayer_private/test_interfaces/ITestWebLayer.aidl index 6a2a36bd26d8d2..7000fdcba929e8 100644 --- a/weblayer/browser/java/org/chromium/weblayer_private/test_interfaces/ITestWebLayer.aidl +++ b/weblayer/browser/java/org/chromium/weblayer_private/test_interfaces/ITestWebLayer.aidl @@ -10,4 +10,10 @@ 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; } diff --git a/weblayer/public/common/switches.cc b/weblayer/public/common/switches.cc index 3cf247589cc362..21fdccb938d211 100644 --- a/weblayer/public/common/switches.cc +++ b/weblayer/public/common/switches.cc @@ -9,8 +9,4 @@ 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 diff --git a/weblayer/public/common/switches.h b/weblayer/public/common/switches.h index 05f3c69639dc24..208d09f0ec1391 100644 --- a/weblayer/public/common/switches.h +++ b/weblayer/public/common/switches.h @@ -8,7 +8,6 @@ namespace switches { extern const char kWebLayerUserDataDir[]; -extern const char kWebLayerFakePermissions[]; } // namespace switches diff --git a/weblayer/public/javatestutil/org/chromium/weblayer/TestWebLayer.java b/weblayer/public/javatestutil/org/chromium/weblayer/TestWebLayer.java index 7e6c44a620101e..4e0126e950434e 100644 --- a/weblayer/public/javatestutil/org/chromium/weblayer/TestWebLayer.java +++ b/weblayer/public/javatestutil/org/chromium/weblayer/TestWebLayer.java @@ -63,4 +63,12 @@ public void setMockLocationProvider(boolean enabled) throws RemoteException { 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); + } }