Skip to content

Commit 0215844

Browse files
ntfschr-chromiumChromium LUCI CQ
authored andcommitted
Revert "Reland "[permissions] Add browsertest for approximate geolocation flow""
This reverts commit 1de8507. Reason for revert: test failures in crbug.com/452645984 Original change's description: > Reland "[permissions] Add browsertest for approximate geolocation flow" > > This is a reland of commit 9d3515e > > Original change's description: > > [permissions] Add browsertest for approximate geolocation flow > > > > This CL adds a browsertest that verifies the flow for requesting > > geolocation permission with the approximate/precise option when only > > approximate location is granted. > > > > The test needs to be on Android because the feature is not implemented > > on desktop yet. > > > > Bug: 393053278 > > Change-Id: Ic6fbc6ecf1d0e77e92392a73d5e83cf940009edd > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7035572 > > Reviewed-by: Andy Paicu <[email protected]> > > Commit-Queue: Antonio Sartori <[email protected]> > > Reviewed-by: Christian Dullweber <[email protected]> > > Cr-Commit-Position: refs/heads/main@{#1530773} > > Bug: 393053278 > Change-Id: I3a1cc994a993669309fe5125e6b2f5dcacfd514e > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7052722 > Reviewed-by: Andy Paicu <[email protected]> > Commit-Queue: Antonio Sartori <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1532195} Bug: 393053278 Bug: 452645984 No-Presubmit: true No-Tree-Checks: true No-Try: true Change-Id: I90ce98cad75acb541d685e1c876768d8fd01e825 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7062057 Bot-Commit: Rubber Stamper <[email protected]> Auto-Submit: Nate Fischer <[email protected]> Commit-Queue: Rubber Stamper <[email protected]> Owners-Override: Nate Fischer <[email protected]> Cr-Commit-Position: refs/heads/main@{#1532369}
1 parent df406fd commit 0215844

File tree

7 files changed

+4
-195
lines changed

7 files changed

+4
-195
lines changed

chrome/browser/permissions/BUILD.gn

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -331,22 +331,6 @@ if (!is_android) {
331331
}
332332
}
333333

334-
if (is_android) {
335-
source_set("android_browser_tests") {
336-
testonly = true
337-
338-
defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ]
339-
340-
sources = [ "android_permission_request_manager_browsertest.cc" ]
341-
342-
deps = [
343-
":permissions",
344-
":test_support",
345-
"//chrome/test:test_support",
346-
]
347-
}
348-
}
349-
350334
if (!is_android && !is_chromeos_device) {
351335
source_set("interactive_ui_tests") {
352336
testonly = true

chrome/browser/permissions/android_permission_request_manager_browsertest.cc

Lines changed: 0 additions & 150 deletions
This file was deleted.

chrome/test/BUILD.gn

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1903,7 +1903,6 @@ if (is_android) {
19031903
"//chrome/browser/password_manager/android:test_support",
19041904
"//chrome/browser/password_manager/factories:password_factory_headers",
19051905
"//chrome/browser/payments:browser_tests",
1906-
"//chrome/browser/permissions:android_browser_tests",
19071906
"//chrome/browser/policy:browser_tests",
19081907
"//chrome/browser/policy:test_support",
19091908
"//chrome/browser/preloading:test_support",

components/permissions/contexts/geolocation_permission_context_android.cc

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,9 @@ void GeolocationPermissionContextAndroid::RequestPermission(
143143
.status;
144144
if (!request_data->IsEmbeddedPermissionElementInitiated() &&
145145
(status == PermissionStatus::GRANTED) &&
146-
ShouldRepromptUser(web_contents)) {
146+
ShouldRepromptUserForPermissions(web_contents,
147+
{content_settings_type()}) ==
148+
PermissionRepromptState::kShow) {
147149
if (auto* manager =
148150
PermissionRequestManager::FromWebContents(web_contents)) {
149151
if (manager->IsCurrentRequestEmbeddedPermissionElementInitiated() &&
@@ -490,12 +492,4 @@ void GeolocationPermissionContextAndroid::SetLocationSettingsForTesting(
490492
location_settings_ = std::move(settings);
491493
}
492494

493-
bool GeolocationPermissionContextAndroid::ShouldRepromptUser(
494-
content::WebContents* web_contents) {
495-
return should_reprompt_user_for_testing_.value_or(
496-
ShouldRepromptUserForPermissions(web_contents,
497-
{content_settings_type()}) ==
498-
PermissionRepromptState::kShow);
499-
}
500-
501495
} // namespace permissions

components/permissions/contexts/geolocation_permission_context_android.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,6 @@ class GeolocationPermissionContextAndroid
7979
void SetLocationSettingsForTesting(
8080
std::unique_ptr<LocationSettings> settings);
8181

82-
void SetShouldRepromptUserForTesting(bool should_reprompt_user_for_testing) {
83-
should_reprompt_user_for_testing_ = should_reprompt_user_for_testing;
84-
}
85-
8682
private:
8783
// GeolocationPermissionContext:
8884
void RequestPermission(std::unique_ptr<PermissionRequestData> request_data,
@@ -151,8 +147,6 @@ class GeolocationPermissionContextAndroid
151147
PermissionDecision decision,
152148
std::optional<PromptOptions> prompt_options);
153149

154-
bool ShouldRepromptUser(content::WebContents* web_contents);
155-
156150
std::unique_ptr<LocationSettings> location_settings_;
157151

158152
PermissionRequestID location_settings_dialog_request_id_;
@@ -162,8 +156,6 @@ class GeolocationPermissionContextAndroid
162156
BrowserPermissionCallback>>
163157
pending_reprompt_requests_;
164158

165-
std::optional<bool> should_reprompt_user_for_testing_;
166-
167159
// Must be the last member, to ensure that it will be destroyed first, which
168160
// will invalidate weak pointers.
169161
base::WeakPtrFactory<GeolocationPermissionContextAndroid> weak_factory_{this};

components/permissions/permission_request_manager.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1595,7 +1595,6 @@ void PermissionRequestManager::LogWarningToConsole(const char* message) {
15951595
}
15961596

15971597
void PermissionRequestManager::DoAutoResponseForTesting() {
1598-
SetPromptOptions(auto_response_prompt_options_for_test_);
15991598
// The macOS prompt has its own mechanism of auto responding.
16001599
if (current_request_prompt_disposition_ ==
16011600
PermissionPromptDisposition::MAC_OS_PROMPT) {

components/permissions/permission_request_manager.h

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
#include "components/permissions/permission_uma_util.h"
2929
#include "components/permissions/prediction_service/permission_ui_selector.h"
3030
#include "components/permissions/request_type.h"
31-
#include "components/permissions/resolvers/permission_prompt_options.h"
3231
#include "components/tabs/public/tab_interface.h"
3332
#include "content/public/browser/global_routing_id.h"
3433
#include "content/public/browser/web_contents.h"
@@ -146,20 +145,13 @@ class PermissionRequestManager
146145
// Recreates a permission prompt.
147146
void RestorePrompt();
148147

149-
// Do NOT use these methods in production code. Use these methods in browser
148+
// Do NOT use this methods in production code. Use this methods in browser
150149
// tests that need to accept or deny permissions when requested in
151150
// JavaScript. Your test needs to set this appropriately, and then the bubble
152151
// will proceed as desired as soon as Show() is called.
153152
void set_auto_response_for_test(AutoResponseType response) {
154153
auto_response_for_test_ = response;
155154
}
156-
void set_auto_response_prompt_options_for_test(PromptOptions prompt_options) {
157-
CHECK_NE(auto_response_for_test_, AutoResponseType::NONE)
158-
<< "Call set_auto_response_for_test() before calling "
159-
"set_auto_response_prompt_options_for_test, since this does not "
160-
"have any effect otherwise.";
161-
auto_response_prompt_options_for_test_ = std::move(prompt_options);
162-
}
163155

164156
// WebContentsObserver:
165157
void DidStartNavigation(
@@ -544,7 +536,6 @@ class PermissionRequestManager
544536

545537
base::ObserverList<Observer> observer_list_;
546538
AutoResponseType auto_response_for_test_ = NONE;
547-
PromptOptions auto_response_prompt_options_for_test_ = std::monostate();
548539

549540
// Suppress notification permission prompts in this tab, regardless of the
550541
// origin requesting the permission.

0 commit comments

Comments
 (0)