Skip to content

Commit

Permalink
Revert "[FedCM] Implement a command line option to use a fake UI"
Browse files Browse the repository at this point in the history
This reverts commit 4880920.

Reason for revert:
Findit (https://goo.gl/kROfz5) identified this CL at revision 4880920 as
the culprit for failures in the continuous build including:

Sample Failed Build: https://ci.chromium.org/b/8818226933029618241
Sample Failed Step: compile

If it is a false positive, please report it at https://bugs.chromium.org/p/chromium/issues/entry?status=Available&comment=Datastore+key+for+the+culprit+entity%3A+chromium.googlesource.com%2Fchromium%2Fsrc%2Frefs%2Fheads%2Fmain%2F48809204fea3a0b4498a9a995c30dadca6f8d0a3&labels=Test-Findit-Wrong&components=Tools%3ETest%3EFindIt&summary=Wrongly+blame+48809204fea3a0b4498a9a995c30dadca6f8d0a3

Original change's description:
> [FedCM] Implement a command line option to use a fake UI
> 
> This is useful for WPT tests and Webdriver, along the lines of the
> existing --use-fake-ui-for-media-stream.
> 
> This also merges the two existing fake dialogs into a single impl
> in content/public/browser.
> 
> Bug: 1302097
> Change-Id: I7bc173c432a17755c2c3ea7ecc9c4c64fd3166d5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3546261
> Reviewed-by: Yi Gu <yigu@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#987225}


Change-Id: Ida0e5f6ce4c6e10ef102dea971f2b7383e0595b8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1302097
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3562265
Reviewed-by: Jiewei Qian <qjw@chromium.org>
Commit-Queue: Jiewei Qian <qjw@chromium.org>
Owners-Override: Jiewei Qian <qjw@chromium.org>
Cr-Commit-Position: refs/heads/main@{#987345}
  • Loading branch information
Findit authored and Chromium LUCI CQ committed Mar 31, 2022
1 parent ca4dd20 commit 45b5705
Show file tree
Hide file tree
Showing 15 changed files with 124 additions and 76 deletions.
2 changes: 0 additions & 2 deletions content/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2091,8 +2091,6 @@ source_set("browser") {
"webauth/client_data_json.h",
"webauth/webauth_request_security_checker.cc",
"webauth/webauth_request_security_checker.h",
"webid/fake_identity_request_dialog_controller.cc",
"webid/fake_identity_request_dialog_controller.h",
"webid/fedcm_metrics.cc",
"webid/fedcm_metrics.h",
"webid/federated_auth_request_impl.cc",
Expand Down
39 changes: 0 additions & 39 deletions content/browser/webid/fake_identity_request_dialog_controller.h

This file was deleted.

14 changes: 0 additions & 14 deletions content/browser/webid/federated_auth_request_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,12 @@
#include "content/browser/webid/federated_auth_request_impl.h"

#include "base/callback.h"
#include "base/command_line.h"
#include "base/ranges/algorithm.h"
#include "base/strings/string_piece.h"
#include "base/threading/sequenced_task_runner_handle.h"
#include "base/time/time.h"
#include "content/browser/bad_message.h"
#include "content/browser/renderer_host/render_frame_host_impl.h"
#include "content/browser/webid/fake_identity_request_dialog_controller.h"
#include "content/browser/webid/fedcm_metrics.h"
#include "content/browser/webid/flags.h"
#include "content/browser/webid/webid_utils.h"
Expand All @@ -25,7 +23,6 @@
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_client.h"
#include "content/public/common/content_switches.h"
#include "third_party/blink/public/mojom/devtools/console_message.mojom.h"
#include "third_party/blink/public/mojom/devtools/inspector_issue.mojom.h"
#include "ui/accessibility/ax_mode.h"
Expand Down Expand Up @@ -1111,17 +1108,6 @@ FederatedAuthRequestImpl::CreateDialogController() {
if (mock_dialog_controller_)
return std::move(mock_dialog_controller_);

if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kUseFakeUIForFedCM)) {
std::string selected_account =
base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
switches::kUseFakeUIForFedCM);
return std::make_unique<FakeIdentityRequestDialogController>(
selected_account.empty()
? absl::nullopt
: absl::optional<std::string>(selected_account));
}

return GetContentClient()->browser()->CreateIdentityRequestDialogController();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,32 +1,33 @@
// Copyright 2022 The Chromium Authors. All rights reserved.
// Copyright 2021 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 "content/browser/webid/fake_identity_request_dialog_controller.h"
#include "content/browser/webid/test/fake_identity_request_dialog_controller.h"

#include "base/bind.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace content {

FakeIdentityRequestDialogController::FakeIdentityRequestDialogController(
absl::optional<std::string> selected_account)
: selected_account_(selected_account) {}
absl::optional<std::string> dialog_selected_account)
: dialog_selected_account_(dialog_selected_account) {}

FakeIdentityRequestDialogController::~FakeIdentityRequestDialogController() =
default;

void FakeIdentityRequestDialogController::ShowAccountsDialog(
content::WebContents* rp_web_contents,
WebContents* rp_web_contents,
const GURL& idp_signin_url,
base::span<const IdentityRequestAccount> accounts,
const IdentityProviderMetadata& idp_metadata,
const ClientIdData& client_id_data,
IdentityRequestAccount::SignInMode sign_in_mode,
AccountSelectionCallback on_selected) {
DCHECK_GT(accounts.size(), 0ul);
// Use the provided account, if any. Otherwise use the first one.
if (selected_account_)
std::move(on_selected).Run(*selected_account_, /* is_sign_in= */ true);
else
std::move(on_selected).Run(accounts[0].id, /* is_sign_in= */ true);
if (dialog_selected_account_) {
std::move(on_selected)
.Run(*dialog_selected_account_, true /* is_sign_in */);
}
}

} // namespace content
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2021 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 CONTENT_BROWSER_WEBID_TEST_FAKE_IDENTITY_REQUEST_DIALOG_CONTROLLER_H_
#define CONTENT_BROWSER_WEBID_TEST_FAKE_IDENTITY_REQUEST_DIALOG_CONTROLLER_H_

#include <string>

#include "content/public/browser/identity_request_dialog_controller.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "url/gurl.h"

namespace content {

class WebContents;

// This fakes the request dialogs to always provide user consent.
// Tests that need to vary the responses or set test expectations should use
// MockIdentityRequestDialogController.
// This also fakes an IdP sign-in page until tests can be set up to
// verify the FederatedAuthResponse mechanics.
class FakeIdentityRequestDialogController
: public IdentityRequestDialogController {
public:
explicit FakeIdentityRequestDialogController(
absl::optional<std::string> dialog_selected_account);
~FakeIdentityRequestDialogController() override;

FakeIdentityRequestDialogController(
const FakeIdentityRequestDialogController&) = delete;
FakeIdentityRequestDialogController& operator=(
const FakeIdentityRequestDialogController&) = delete;

void ShowAccountsDialog(WebContents* rp_web_contents,
const GURL& idp_signin_url,
base::span<const IdentityRequestAccount> accounts,
const IdentityProviderMetadata& idp_metadata,
const ClientIdData& client_id_data,
IdentityRequestAccount::SignInMode sign_in_mode,
AccountSelectionCallback on_selected) override;

private:
absl::optional<std::string> dialog_selected_account_;
};

} // namespace content

#endif // CONTENT_BROWSER_WEBID_TEST_FAKE_IDENTITY_REQUEST_DIALOG_CONTROLLER_H_
2 changes: 1 addition & 1 deletion content/browser/webid/webid_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include "base/strings/string_number_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "components/network_session_configurator/common/network_switches.h"
#include "content/browser/webid/fake_identity_request_dialog_controller.h"
#include "content/browser/webid/test/fake_identity_request_dialog_controller.h"
#include "content/browser/webid/test/webid_test_content_browser_client.h"
#include "content/public/browser/content_browser_client.h"
#include "content/public/browser/identity_request_dialog_controller.h"
Expand Down
5 changes: 0 additions & 5 deletions content/public/common/content_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -799,11 +799,6 @@ const char kTrustableWebBundleFileUrl[] = "trustable-web-bundles-file-url";
const char kUseFakeCodecForPeerConnection[] =
"use-fake-codec-for-peer-connection";

// Bypass the FedCM account selection dialog. If a value is provided for
// this switch, that account ID is selected, otherwise the first account
// is chosen.
const char kUseFakeUIForFedCM[] = "use-fake-ui-for-fedcm";

// Bypass the media stream infobar by selecting the default device for media
// streams (e.g. WebRTC). Works with --use-fake-device-for-media-stream.
const char kUseFakeUIForMediaStream[] = "use-fake-ui-for-media-stream";
Expand Down
1 change: 0 additions & 1 deletion content/public/common/content_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,6 @@ CONTENT_EXPORT extern const char kTouchEventFeatureDetectionEnabled[];
CONTENT_EXPORT extern const char kTouchEventFeatureDetectionDisabled[];
CONTENT_EXPORT extern const char kTrustableWebBundleFileUrl[];
CONTENT_EXPORT extern const char kUseFakeCodecForPeerConnection[];
CONTENT_EXPORT extern const char kUseFakeUIForFedCM[];
CONTENT_EXPORT extern const char kUseFakeUIForMediaStream[];
CONTENT_EXPORT extern const char kVideoImageTextureTarget[];
CONTENT_EXPORT extern const char kUseMobileUserAgent[];
Expand Down
2 changes: 2 additions & 0 deletions content/shell/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ static_library("content_shell_lib") {
"browser/shell_download_manager_delegate.h",
"browser/shell_federated_permission_context.cc",
"browser/shell_federated_permission_context.h",
"browser/shell_identity_dialog_controller.cc",
"browser/shell_identity_dialog_controller.h",
"browser/shell_javascript_dialog.h",
"browser/shell_javascript_dialog_manager.cc",
"browser/shell_javascript_dialog_manager.h",
Expand Down
6 changes: 6 additions & 0 deletions content/shell/browser/shell_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#include "content/shell/browser/shell_browser_context.h"
#include "content/shell/browser/shell_browser_main_parts.h"
#include "content/shell/browser/shell_devtools_manager_delegate.h"
#include "content/shell/browser/shell_identity_dialog_controller.h"
#include "content/shell/browser/shell_paths.h"
#include "content/shell/browser/shell_quota_permission_context.h"
#include "content/shell/browser/shell_web_contents_view_delegate_creator.h"
Expand Down Expand Up @@ -620,6 +621,11 @@ bool ShellContentBrowserClient::HasErrorPage(int http_status_code) {
return http_status_code >= 400 && http_status_code < 600;
}

std::unique_ptr<IdentityRequestDialogController>
ShellContentBrowserClient::CreateIdentityRequestDialogController() {
return std::make_unique<ShellIdentityDialogController>();
}

void ShellContentBrowserClient::CreateFeatureListAndFieldTrials() {
local_state_ = CreateLocalState();
SetUpFieldTrials();
Expand Down
2 changes: 2 additions & 0 deletions content/shell/browser/shell_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ class ShellContentBrowserClient : public ContentBrowserClient {
void GetHyphenationDictionary(
base::OnceCallback<void(const base::FilePath&)>) override;
bool HasErrorPage(int http_status_code) override;
std::unique_ptr<IdentityRequestDialogController>
CreateIdentityRequestDialogController() override;
void OnNetworkServiceCreated(
network::mojom::NetworkService* network_service) override;

Expand Down
23 changes: 23 additions & 0 deletions content/shell/browser/shell_identity_dialog_controller.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2022 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 "content/shell/browser/shell_identity_dialog_controller.h"

namespace content {

void ShellIdentityDialogController::ShowAccountsDialog(
content::WebContents* rp_web_contents,
const GURL& idp_signin_url,
base::span<const IdentityRequestAccount> accounts,
const IdentityProviderMetadata& idp_metadata,
const ClientIdData& client_id_data,
IdentityRequestAccount::SignInMode sign_in_mode,
AccountSelectionCallback on_selected) {
// Similar in spirit to allowlisted permissions in ShellPermissionManager,
// we automatically select the first account here so that tests can pass.
DCHECK_GT(accounts.size(), 0ul);
std::move(on_selected).Run(accounts[0].id, /* is_sign_in= */ false);
}

} // namespace content
25 changes: 25 additions & 0 deletions content/shell/browser/shell_identity_dialog_controller.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2022 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 CONTENT_SHELL_BROWSER_SHELL_IDENTITY_DIALOG_CONTROLLER_H_
#define CONTENT_SHELL_BROWSER_SHELL_IDENTITY_DIALOG_CONTROLLER_H_

#include "content/public/browser/identity_request_dialog_controller.h"

namespace content {

class ShellIdentityDialogController : public IdentityRequestDialogController {
public:
void ShowAccountsDialog(content::WebContents* rp_web_contents,
const GURL& idp_signin_url,
base::span<const IdentityRequestAccount> accounts,
const IdentityProviderMetadata& idp_metadata,
const ClientIdData& client_id_data,
IdentityRequestAccount::SignInMode sign_in_mode,
AccountSelectionCallback on_selected) override;
};

} // namespace content

#endif // CONTENT_SHELL_BROWSER_SHELL_IDENTITY_DIALOG_CONTROLLER_H_
4 changes: 4 additions & 0 deletions content/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1357,6 +1357,8 @@ test("content_browsertests") {
"../browser/web_package/web_bundle_file_browsertest.cc",
"../browser/web_package/web_bundle_network_browsertest.cc",
"../browser/web_package/web_bundle_trustable_file_browsertest.cc",
"../browser/webid/test/fake_identity_request_dialog_controller.cc",
"../browser/webid/test/fake_identity_request_dialog_controller.h",
"../browser/webid/test/webid_test_content_browser_client.cc",
"../browser/webid/test/webid_test_content_browser_client.h",
"../browser/webid/webid_browsertest.cc",
Expand Down Expand Up @@ -2331,6 +2333,8 @@ test("content_unittests") {
"../browser/web_package/web_bundle_utils_unittest.cc",
"../browser/webid/federated_auth_request_impl_unittest.cc",
"../browser/webid/idp_network_request_manager_unittest.cc",
"../browser/webid/test/fake_identity_request_dialog_controller.cc",
"../browser/webid/test/fake_identity_request_dialog_controller.h",
"../browser/webid/test/mock_active_session_permission_delegate.cc",
"../browser/webid/test/mock_active_session_permission_delegate.h",
"../browser/webid/test/mock_api_permission_delegate.cc",
Expand Down
3 changes: 0 additions & 3 deletions content/web_test/browser/web_test_browser_main_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,6 @@ void WebTestBrowserMainRunner::Initialize() {
command_line.AppendSwitch(switches::kUseFakeUIForMediaStream);
command_line.AppendSwitch(switches::kUseFakeDeviceForMediaStream);

// Always run with fake FedCM UI.
command_line.AppendSwitch(switches::kUseFakeUIForFedCM);

// Enable the deprecated WebAuthn Mojo Testing API.
command_line.AppendSwitch(switches::kEnableWebAuthDeprecatedMojoTestingApi);

Expand Down

0 comments on commit 45b5705

Please sign in to comment.