Skip to content

Commit

Permalink
Make Starter a WebContentUserData
Browse files Browse the repository at this point in the history
Before this change, the starter was owned by the Android platform
delegate. In order to support other platforms, the starter is
now a WebContentUserData and will be created on tab creation.
This CL adds the creation for android only, a follow-up will
add the same for other platforms.

Bug: b:201964911


Change-Id: I7fea6d3d47ea09502530f25daca891e0c09ab86c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3528555
Reviewed-by: Clemens Arbesser <arbesser@google.com>
Commit-Queue: Luca Hunkeler <hluca@google.com>
Cr-Commit-Position: refs/heads/main@{#987368}
  • Loading branch information
Luca Hunkeler authored and Chromium LUCI CQ committed Mar 31, 2022
1 parent 07705ab commit 3915e20
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ static jlong JNI_Starter_FromWebContents(
std::move(dependencies));
auto* tab_helper_android =
StarterDelegateAndroid::FromWebContents(web_contents);
Starter::CreateForWebContents(
web_contents, tab_helper_android, ukm::UkmRecorder::Get(),
RuntimeManagerImpl::GetForWebContents(web_contents)->GetWeakPtr(),
base::DefaultTickClock::GetInstance());
return reinterpret_cast<intptr_t>(tab_helper_android);
}

Expand All @@ -67,17 +71,19 @@ void StarterDelegateAndroid::Attach(JNIEnv* env,
Detach(env, jcaller);
java_object_ = base::android::ScopedJavaGlobalRef<jobject>(jcaller);

starter_ = std::make_unique<Starter>(
&GetWebContents(), this, ukm::UkmRecorder::Get(),
RuntimeManagerImpl::GetForWebContents(&GetWebContents())->GetWeakPtr(),
base::DefaultTickClock::GetInstance());
starter_ = Starter::FromWebContents(&GetWebContents());
DCHECK(starter_);
starter_->Init();
}

void StarterDelegateAndroid::Detach(JNIEnv* env,
const JavaParamRef<jobject>& jcaller) {
java_object_ = nullptr;
java_dependencies_ = nullptr;
starter_.reset();
if (starter_) {
starter_->Init();
}
starter_ = nullptr;
}

std::unique_ptr<TriggerScriptCoordinator::UiDelegate>
Expand Down Expand Up @@ -335,6 +341,10 @@ StarterDelegateAndroid::CreateFieldTrialUtil() {
return dependencies_->CreateFieldTrialUtil();
}

bool StarterDelegateAndroid::IsAttached() {
return !!java_object_;
}

WEB_CONTENTS_USER_DATA_KEY_IMPL(StarterDelegateAndroid);

} // namespace autofill_assistant
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class StarterDelegateAndroid
bool GetIsCustomTab() const override;
bool GetIsTabCreatedByGSA() const override;
std::unique_ptr<AssistantFieldTrialUtil> CreateFieldTrialUtil() override;
bool IsAttached() override;

// Called by Java to start an autofill-assistant flow for an incoming intent.
void Start(
Expand Down Expand Up @@ -124,7 +125,7 @@ class StarterDelegateAndroid
void CreateJavaDependenciesIfNecessary();

WEB_CONTENTS_USER_DATA_KEY_DECL();
std::unique_ptr<Starter> starter_;
raw_ptr<Starter> starter_;
// Contains AssistantStaticDependencies which do not change.
const std::unique_ptr<const Dependencies> dependencies_;
// Can change based on activity attachment.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,4 +119,8 @@ FakeStarterPlatformDelegate::CreateFieldTrialUtil() {
return std::make_unique<MockAssistantFieldTrialUtil>();
}

bool FakeStarterPlatformDelegate::IsAttached() {
return is_attached_;
}

} // namespace autofill_assistant
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class FakeStarterPlatformDelegate : public StarterPlatformDelegate {
bool GetIsCustomTab() const override;
bool GetIsTabCreatedByGSA() const override;
std::unique_ptr<AssistantFieldTrialUtil> CreateFieldTrialUtil() override;
bool IsAttached() override;

// Intentionally public to give tests direct access.
std::unique_ptr<TriggerScriptCoordinator::UiDelegate>
Expand All @@ -74,6 +75,7 @@ class FakeStarterPlatformDelegate : public StarterPlatformDelegate {
bool is_custom_tab_ = true;
bool is_tab_created_by_gsa_ = true;
std::unique_ptr<AssistantFieldTrialUtil> field_trial_util_;
bool is_attached_ = true;

base::OnceCallback<void(
GURL url,
Expand Down
21 changes: 15 additions & 6 deletions components/autofill_assistant/browser/starter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ Starter::Starter(content::WebContents* web_contents,
base::WeakPtr<RuntimeManager> runtime_manager,
const base::TickClock* tick_clock)
: content::WebContentsObserver(web_contents),
content::WebContentsUserData<Starter>(*web_contents),
current_ukm_source_id_(
ukm::GetSourceIdForWebContentsDocument(web_contents)),
cached_failed_trigger_script_fetches_(
Expand All @@ -191,9 +192,7 @@ Starter::Starter(content::WebContents* web_contents,
ukm_recorder_(ukm_recorder),
runtime_manager_(runtime_manager),
starter_heuristic_(GetOrCreateStarterHeuristic()),
tick_clock_(tick_clock) {
CheckSettings();
}
tick_clock_(tick_clock) {}

Starter::~Starter() = default;

Expand Down Expand Up @@ -361,13 +360,18 @@ void Starter::RegisterSyntheticFieldTrials(
}

void Starter::OnTabInteractabilityChanged(bool is_interactable) {
CheckSettings();
Init();
if (trigger_script_coordinator_) {
trigger_script_coordinator_->OnTabInteractabilityChanged(is_interactable);
}
}

void Starter::CheckSettings() {
void Starter::Init() {
if (!platform_delegate_->IsAttached()) {
CancelPendingStartup(Metrics::TriggerScriptFinishedState::CANCELED);
return;
}

bool prev_is_custom_tab = is_custom_tab_;
is_custom_tab_ = platform_delegate_->GetIsCustomTab();
bool switched_from_cct_to_tab = prev_is_custom_tab && !is_custom_tab_;
Expand Down Expand Up @@ -437,7 +441,7 @@ void Starter::RecordDependenciesInvalidated() const {

void Starter::OnDependenciesInvalidated() {
RecordDependenciesInvalidated();
CheckSettings();
Init();
}

void Starter::Start(std::unique_ptr<TriggerContext> trigger_context) {
Expand All @@ -449,6 +453,9 @@ void Starter::Start(std::unique_ptr<TriggerContext> trigger_context) {

CancelPendingStartup(Metrics::TriggerScriptFinishedState::CANCELED);
pending_trigger_context_ = std::move(trigger_context);
if (!platform_delegate_->IsAttached()) {
OnStartDone(/* start_regular_script = */ false);
}

if (base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
switches::kAutofillAssistantForceOnboarding) == "true") {
Expand Down Expand Up @@ -809,4 +816,6 @@ void Starter::DeleteTriggerScriptCoordinator() {
trigger_script_coordinator_.reset();
}

WEB_CONTENTS_USER_DATA_KEY_IMPL(Starter);

} // namespace autofill_assistant
9 changes: 7 additions & 2 deletions components/autofill_assistant/browser/starter.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,16 @@
#include "components/autofill_assistant/browser/startup_util.h"
#include "components/autofill_assistant/browser/trigger_scripts/trigger_script_coordinator.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/browser/web_contents_user_data.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

namespace autofill_assistant {

// Starts autofill-assistant flows. Uses a platform delegate to show UI and
// access platform-dependent features.
class Starter : public content::WebContentsObserver {
class Starter : public content::WebContentsObserver,
public content::WebContentsUserData<Starter> {
public:
explicit Starter(content::WebContents* web_contents,
StarterPlatformDelegate* platform_delegate,
Expand Down Expand Up @@ -64,14 +66,15 @@ class Starter : public content::WebContentsObserver {

// Re-check settings. This may cancel ongoing startup requests if the required
// settings are no longer enabled.
void CheckSettings();
void Init();

// Records the invalidation of platform-specific depencendies. For example:
// When the activity is changed on Android.
void OnDependenciesInvalidated();

private:
friend class StarterTest;
friend class content::WebContentsUserData<Starter>;

// Starts a flow for |url| if possible. Will fail (do nothing) if the feature
// is disabled or if there is already a pending startup.
Expand Down Expand Up @@ -145,6 +148,8 @@ class Starter : public content::WebContentsObserver {
void RegisterSyntheticFieldTrials(
const TriggerContext& trigger_context) const;

WEB_CONTENTS_USER_DATA_KEY_DECL();

// The UKM source id to use for UKM metrics.
ukm::SourceId current_ukm_source_id_ = ukm::kInvalidSourceId;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ class StarterPlatformDelegate {
virtual bool GetIsTabCreatedByGSA() const = 0;
// Creates the util for registering synthetic field trials.
virtual std::unique_ptr<AssistantFieldTrialUtil> CreateFieldTrialUtil() = 0;
// Whether the starter platform delegate is currently attached.
// The starter platform delegate should only be interacted with while attached
// as it might not be able to perform its functions while detached.
virtual bool IsAttached() = 0;
};

} // namespace autofill_assistant
Expand Down

0 comments on commit 3915e20

Please sign in to comment.