Commit a8c389de authored by Johann's avatar Johann Committed by Commit Bot

heap mojo: remove exception for credential manager

Reset the HeapMojo objects in OnContextDestroyed instead of
waiting for GC.

This removes the need to call
FlushCredentialManagerConnectionForTesting() as it happens
automatically.

In the sms handler this changes the behavior on page reload.
Previously it was possible to receive a callback after the
page reload. Now that the connection is cleaned up as soon
as the page is reloaded it is not possible to go fishing
for that callback.

Bug: 1049056
Change-Id: Ib1b188dadff30e251459a6aa5156fbbd64bc0957
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2331250
Commit-Queue: Johann Koenig <johannkoenig@google.com>
Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarMike West <mkwst@chromium.org>
Reviewed-by: default avatarReilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802154}
parent 8c256cb2
...@@ -334,12 +334,6 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, Reload) { ...@@ -334,12 +334,6 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, Reload) {
MockSmsProvider* mock_provider_ptr = provider.get(); MockSmsProvider* mock_provider_ptr = provider.get();
BrowserMainLoop::GetInstance()->SetSmsProviderForTesting(std::move(provider)); BrowserMainLoop::GetInstance()->SetSmsProviderForTesting(std::move(provider));
std::string script = R"(
// kicks off the sms receiver, adding the service
// to the observer's list.
navigator.credentials.get({otp: {transport: ["sms"]}});
)";
base::RunLoop loop; base::RunLoop loop;
EXPECT_CALL(*mock_provider_ptr, Retrieve(_)).WillOnce(Invoke([&loop]() { EXPECT_CALL(*mock_provider_ptr, Retrieve(_)).WillOnce(Invoke([&loop]() {
...@@ -348,26 +342,22 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, Reload) { ...@@ -348,26 +342,22 @@ IN_PROC_BROWSER_TEST_F(SmsBrowserTest, Reload) {
loop.Quit(); loop.Quit();
})); }));
EXPECT_TRUE(ExecJs(shell(), script)); EXPECT_TRUE(ExecJs(shell(), R"(
navigator.credentials.get({otp: {transport: ["sms"]}});
)"));
loop.Run(); loop.Run();
ASSERT_TRUE(GetSmsFetcher()->HasSubscribers()); ASSERT_TRUE(GetSmsFetcher()->HasSubscribers());
ASSERT_TRUE(mock_provider_ptr->HasObservers());
// Wait for UKM to be recorded to avoid race condition between outcome // Reload the page. This destroys the ExecutionContext and resets any HeapMojo
// capture and evaluation. // connections.
base::RunLoop ukm_loop;
ukm_recorder()->SetOnAddEntryCallback(Entry::kEntryName,
ukm_loop.QuitClosure());
// Reload the page.
EXPECT_TRUE(NavigateToURL(shell(), url)); EXPECT_TRUE(NavigateToURL(shell(), url));
ukm_loop.Run();
ASSERT_FALSE(GetSmsFetcher()->HasSubscribers()); ASSERT_FALSE(GetSmsFetcher()->HasSubscribers());
ExpectOutcomeUKM(url, blink::SMSReceiverOutcome::kTimeout); ExpectNoOutcomeUKM();
} }
IN_PROC_BROWSER_TEST_F(SmsBrowserTest, Close) { IN_PROC_BROWSER_TEST_F(SmsBrowserTest, Close) {
......
...@@ -50,10 +50,6 @@ class MODULES_EXPORT CredentialManagerProxy ...@@ -50,10 +50,6 @@ class MODULES_EXPORT CredentialManagerProxy
payments::mojom::blink::PaymentCredential* PaymentCredential(); payments::mojom::blink::PaymentCredential* PaymentCredential();
void FlushCredentialManagerConnectionForTesting() {
credential_manager_.FlushForTesting();
}
void Trace(Visitor*) const override; void Trace(Visitor*) const override;
// Must be called only with argument representing a valid // Must be called only with argument representing a valid
...@@ -61,18 +57,10 @@ class MODULES_EXPORT CredentialManagerProxy ...@@ -61,18 +57,10 @@ class MODULES_EXPORT CredentialManagerProxy
static CredentialManagerProxy* From(ScriptState*); static CredentialManagerProxy* From(ScriptState*);
private: private:
HeapMojoRemote<mojom::blink::Authenticator, HeapMojoRemote<mojom::blink::Authenticator> authenticator_;
HeapMojoWrapperMode::kForceWithoutContextObserver> HeapMojoRemote<mojom::blink::CredentialManager> credential_manager_;
authenticator_; HeapMojoRemote<mojom::blink::SmsReceiver> sms_receiver_;
HeapMojoRemote<mojom::blink::CredentialManager, HeapMojoRemote<payments::mojom::blink::PaymentCredential> payment_credential_;
HeapMojoWrapperMode::kForceWithoutContextObserver>
credential_manager_;
HeapMojoRemote<mojom::blink::SmsReceiver,
HeapMojoWrapperMode::kForceWithoutContextObserver>
sms_receiver_;
HeapMojoRemote<payments::mojom::blink::PaymentCredential,
HeapMojoWrapperMode::kForceWithoutContextObserver>
payment_credential_;
}; };
} // namespace blink } // namespace blink
......
...@@ -171,7 +171,6 @@ TEST(CredentialsContainerTest, ...@@ -171,7 +171,6 @@ TEST(CredentialsContainerTest,
MockCredentialManager mock_credential_manager; MockCredentialManager mock_credential_manager;
CredentialManagerTestingContext context(&mock_credential_manager); CredentialManagerTestingContext context(&mock_credential_manager);
auto* proxy = CredentialManagerProxy::From(context.GetScriptState());
auto promise = MakeGarbageCollected<CredentialsContainer>()->get( auto promise = MakeGarbageCollected<CredentialsContainer>()->get(
context.GetScriptState(), CredentialRequestOptions::Create()); context.GetScriptState(), CredentialRequestOptions::Create());
mock_credential_manager.WaitForCallToGet(); mock_credential_manager.WaitForCallToGet();
...@@ -179,7 +178,6 @@ TEST(CredentialsContainerTest, ...@@ -179,7 +178,6 @@ TEST(CredentialsContainerTest,
context.Frame()->DomWindow()->FrameDestroyed(); context.Frame()->DomWindow()->FrameDestroyed();
mock_credential_manager.InvokeGetCallback(); mock_credential_manager.InvokeGetCallback();
proxy->FlushCredentialManagerConnectionForTesting();
EXPECT_EQ(v8::Promise::kPending, EXPECT_EQ(v8::Promise::kPending,
promise.V8Value().As<v8::Promise>()->State()); promise.V8Value().As<v8::Promise>()->State());
......
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