Commit eee0a0b8 authored by Sujie Zhu's avatar Sujie Zhu Committed by Commit Bot

[Autofill Auth] Fix for the delayed task

Previously the post delayed task will cause crashes on Linux and Mac. Every time we post a delayed task, then we refresh/close the tap, it will crash when the delayed task executed. It might because the WaitableEvent can only be used within single address space for Non-Windows systems[1]. And when we refresh/close the tap, the credit_card_access_manager is reset when DidNavigateMainFrame [2].

In order to fix it, we replace the callback with CreditCardAccessManager::SignalCanFetchUnmaskDetails function together with weak_ptr of credit_card_access_manager. With the weak pointer, the BindOnce callback won't be executed when credit_card_access_manager is reset.

[1] https://cs.chromium.org/chromium/src/base/synchronization/waitable_event.h?l=39-41&rcl=0b35f9e032c5ac21ef25df8d5cf113529ce2f4c3
[2] https://cs.chromium.org/chromium/src/components/autofill/core/browser/autofill_manager.cc?l=1450-1451&rcl=fd806bd17d805398213f349649276356c5616c6e

Bug: 949269
Change-Id: I8e7acdb411e4421b55a685ff470b82fd954149d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1922913
Commit-Queue: Sujie Zhu <sujiezhu@google.com>
Reviewed-by: default avatarJared Saul <jsaul@google.com>
Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716880}
parent 1f7ebde5
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "base/synchronization/waitable_event.h" #include "base/synchronization/waitable_event.h"
#include "base/task/post_task.h" #include "base/task/post_task.h"
#include "base/task/task_traits.h" #include "base/task/task_traits.h"
#include "base/threading/sequenced_task_runner_handle.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "components/autofill/core/browser/autofill_client.h" #include "components/autofill/core/browser/autofill_client.h"
...@@ -38,7 +39,7 @@ namespace autofill { ...@@ -38,7 +39,7 @@ namespace autofill {
namespace { namespace {
// Timeout to wait for unmask details from Google Payments in milliseconds. // Timeout to wait for unmask details from Google Payments in milliseconds.
constexpr int64_t kUnmaskDetailsResponseTimeoutMs = 1000; constexpr int64_t kUnmaskDetailsResponseTimeoutMs = 3 * 1000; // 3 sec
// Time to wait between multiple calls to GetUnmaskDetails(). // Time to wait between multiple calls to GetUnmaskDetails().
constexpr int64_t kDelayForGetUnmaskDetails = 3 * 60 * 1000; // 3 min constexpr int64_t kDelayForGetUnmaskDetails = 3 * 60 * 1000; // 3 min
...@@ -48,11 +49,6 @@ bool WaitForEvent(base::WaitableEvent* event) { ...@@ -48,11 +49,6 @@ bool WaitForEvent(base::WaitableEvent* event) {
return event->TimedWait( return event->TimedWait(
base::TimeDelta::FromMilliseconds(kUnmaskDetailsResponseTimeoutMs)); base::TimeDelta::FromMilliseconds(kUnmaskDetailsResponseTimeoutMs));
} }
// Used with PostTaskWithDelay() to signal event after a timeout.
void SignalEvent(base::WaitableEvent* event) {
event->Signal();
}
} // namespace } // namespace
CreditCardAccessManager::CreditCardAccessManager( CreditCardAccessManager::CreditCardAccessManager(
...@@ -254,8 +250,12 @@ void CreditCardAccessManager::OnDidGetUnmaskDetails( ...@@ -254,8 +250,12 @@ void CreditCardAccessManager::OnDidGetUnmaskDetails(
#endif #endif
ready_to_start_authentication_.Signal(); ready_to_start_authentication_.Signal();
base::PostDelayedTask( // Use the weak_ptr here so that the delayed task won't be executed if the
FROM_HERE, base::BindOnce(&SignalEvent, &can_fetch_unmask_details_), // |credit_card_access_manager| is reset.
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&CreditCardAccessManager::SignalCanFetchUnmaskDetails,
weak_ptr_factory_.GetWeakPtr()),
base::TimeDelta::FromMilliseconds(delay_ms)); base::TimeDelta::FromMilliseconds(delay_ms));
} }
...@@ -532,4 +532,8 @@ void CreditCardAccessManager::OnDidCancelCardVerification() { ...@@ -532,4 +532,8 @@ void CreditCardAccessManager::OnDidCancelCardVerification() {
} }
#endif #endif
void CreditCardAccessManager::SignalCanFetchUnmaskDetails() {
can_fetch_unmask_details_.Signal();
}
} // namespace autofill } // namespace autofill
...@@ -171,6 +171,10 @@ class CreditCardAccessManager : public CreditCardCVCAuthenticator::Requester, ...@@ -171,6 +171,10 @@ class CreditCardAccessManager : public CreditCardCVCAuthenticator::Requester,
void OnDidCancelCardVerification(); void OnDidCancelCardVerification();
#endif #endif
// Used with PostTaskWithDelay() to signal |can_fetch_unmask_details_| event
// after a timeout.
void SignalCanFetchUnmaskDetails();
// Is set to true only when waiting for the callback to // Is set to true only when waiting for the callback to
// OnCVCAuthenticationComplete() to be executed. // OnCVCAuthenticationComplete() to be executed.
bool is_authentication_in_progress_ = false; bool is_authentication_in_progress_ = false;
......
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