Commit 02303ee2 authored by Haiyang Pan's avatar Haiyang Pan Committed by Commit Bot

Revert "fido: make AuthenticatorTestBase set the MOCK_TIME TaskEnvironment trait"

This reverts commit f55cd37e.

Reason for revert: The tests testAuthenticatorImplGetAssertionBridge_resultCanceled and testAuthenticatorImplGetAssertionBridgeWithUvmRequestedWithUvmResponded_success in org.chromium.chrome.browser.webauth.Fido2CredentialRequestTest consistently fail on android-pie-x86-rel since https://ci.chromium.org/p/chromium/builders/ci/android-pie-x86-rel/1498.
This CL is the only one that touches authenticator_impl around that time. Revert it to see if it can fix the failure

Original change's description:
> fido: make AuthenticatorTestBase set the MOCK_TIME TaskEnvironment trait
> 
> This allows us to get rid of base::Timer injection into
> AuthenticatorCommon, which tests used to wait for requests to time out.
> 
> Also get remove a number of superfluous RunUntilIdle() calls, and of
> calls to OverrideLastCommittedOrigin() where SimulateNavigation() would
> suffice.
> 
> Change-Id: I8cb4980d51efe43f59d828cde5f34ce67d38d928
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2299630
> Auto-Submit: Martin Kreichgauer <martinkr@google.com>
> Reviewed-by: Jared Saul <jsaul@google.com>
> Reviewed-by: Adam Langley <agl@chromium.org>
> Commit-Queue: Martin Kreichgauer <martinkr@google.com>
> Cr-Commit-Position: refs/heads/master@{#788885}

TBR=agl@chromium.org,jsaul@google.com,martinkr@google.com

Change-Id: I47c8796582f40f1946811b5eca4ae3cdfe5aac0f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2303030Reviewed-by: default avatarHaiyang Pan <hypan@google.com>
Commit-Queue: Haiyang Pan <hypan@google.com>
Cr-Commit-Position: refs/heads/master@{#789079}
parent 8ebf771a
......@@ -7,6 +7,7 @@
#include <string>
#include <utility>
#include "base/timer/timer.h"
#include "content/browser/webauth/authenticator_common.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/render_frame_host.h"
......@@ -17,12 +18,20 @@ namespace content {
InternalAuthenticatorImpl::InternalAuthenticatorImpl(
RenderFrameHost* render_frame_host)
: InternalAuthenticatorImpl(render_frame_host,
std::make_unique<AuthenticatorCommon>(
render_frame_host,
std::make_unique<base::OneShotTimer>())) {}
InternalAuthenticatorImpl::InternalAuthenticatorImpl(
RenderFrameHost* render_frame_host,
std::unique_ptr<AuthenticatorCommon> authenticator_common)
: WebContentsObserver(WebContents::FromRenderFrameHost(render_frame_host)),
render_frame_host_(render_frame_host),
effective_origin_(render_frame_host->GetLastCommittedOrigin()),
authenticator_common_(
std::make_unique<AuthenticatorCommon>(render_frame_host)) {
authenticator_common_(std::move(authenticator_common)) {
DCHECK(render_frame_host_);
DCHECK(authenticator_common_);
// Disabling WebAuthn modal dialogs to avoid conflict with Autofill's own
// modal dialogs. Since WebAuthn is designed for websites, rather than browser
// components, the UI can be confusing for users in the case for Autofill.
......
......@@ -57,6 +57,14 @@ class InternalAuthenticatorImpl : public autofill::InternalAuthenticator,
private:
friend class InternalAuthenticatorImplTest;
// By being able to set AuthenticatorCommon, this constructor permits setting
// the connector and timer for testing. Using this constructor will also empty
// out the protocol set, since no device discovery will take place during
// tests.
InternalAuthenticatorImpl(
RenderFrameHost* render_frame_host,
std::unique_ptr<AuthenticatorCommon> authenticator_common);
AuthenticatorCommon* get_authenticator_common_for_testing() {
return authenticator_common_.get();
}
......
......@@ -534,11 +534,15 @@ base::flat_set<device::FidoTransportProtocol> GetAvailableTransports(
} // namespace
AuthenticatorCommon::AuthenticatorCommon(RenderFrameHost* render_frame_host)
AuthenticatorCommon::AuthenticatorCommon(
RenderFrameHost* render_frame_host,
std::unique_ptr<base::OneShotTimer> timer)
: render_frame_host_(render_frame_host),
security_checker_(static_cast<RenderFrameHostImpl*>(render_frame_host)
->GetWebAuthRequestSecurityChecker()) {
->GetWebAuthRequestSecurityChecker()),
timer_(std::move(timer)) {
DCHECK(render_frame_host_);
DCHECK(timer_);
// Disable the back-forward cache for any document that makes WebAuthn
// requests. Pages using privacy-sensitive APIs are generally exempt from
// back-forward cache for now as a precaution.
......
......@@ -16,7 +16,6 @@
#include "base/containers/span.h"
#include "base/macros.h"
#include "base/optional.h"
#include "base/timer/timer.h"
#include "content/common/content_export.h"
#include "content/public/browser/authenticator_request_client_delegate.h"
#include "content/public/browser/web_contents_observer.h"
......@@ -68,7 +67,9 @@ CONTENT_EXPORT extern const char kGetType[];
// Common code for any WebAuthn Authenticator interfaces.
class CONTENT_EXPORT AuthenticatorCommon {
public:
explicit AuthenticatorCommon(RenderFrameHost* render_frame_host);
// Permits setting timer for testing.
AuthenticatorCommon(RenderFrameHost* render_frame_host,
std::unique_ptr<base::OneShotTimer>);
virtual ~AuthenticatorCommon();
// This is not-quite an implementation of blink::mojom::Authenticator. The
......@@ -182,8 +183,7 @@ class CONTENT_EXPORT AuthenticatorCommon {
url::Origin caller_origin_;
std::string relying_party_id_;
scoped_refptr<WebAuthRequestSecurityChecker> security_checker_;
std::unique_ptr<base::OneShotTimer> timer_ =
std::make_unique<base::OneShotTimer>();
std::unique_ptr<base::OneShotTimer> timer_;
base::Optional<device::AuthenticatorSelectionCriteria>
authenticator_selection_criteria_;
base::Optional<std::string> app_id_;
......
......@@ -17,9 +17,10 @@
namespace content {
AuthenticatorImpl::AuthenticatorImpl(RenderFrameHost* render_frame_host)
: AuthenticatorImpl(
render_frame_host,
std::make_unique<AuthenticatorCommon>(render_frame_host)) {}
: AuthenticatorImpl(render_frame_host,
std::make_unique<AuthenticatorCommon>(
render_frame_host,
std::make_unique<base::OneShotTimer>())) {}
AuthenticatorImpl::AuthenticatorImpl(
RenderFrameHost* render_frame_host,
......
......@@ -46,8 +46,9 @@ class CONTENT_EXPORT AuthenticatorImpl : public blink::mojom::Authenticator,
public:
explicit AuthenticatorImpl(RenderFrameHost* render_frame_host);
// Constructs an AuthenticatorImpl with an injected AuthenticatorCommon for
// testing.
// By being able to set AuthenticatorCommon, this constructor permits setting
// the timer for testing. Using this constructor will also empty out the
// protocol set, since no device discovery will take place during tests.
AuthenticatorImpl(RenderFrameHost* render_frame_host,
std::unique_ptr<AuthenticatorCommon> authenticator_common);
~AuthenticatorImpl() override;
......
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