Commit 8ff0ccd3 authored by Clemens Arbesser's avatar Clemens Arbesser Committed by Commit Bot

[Autofill Assistant] Add KeyboardHidden trigger condition.

This will allow trigger scripts to automatically hide while the keyboard
is being displayed. Example video of the interaction is in the linked
bug.

Bug: b/173038438
Change-Id: I874f40c359710d7dd63886f703c5448df2636a0e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2532464
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Reviewed-by: default avatarMathias Carlen <mcarlen@chromium.org>
Reviewed-by: default avatarMarian Fechete <marianfe@google.com>
Cr-Commit-Position: refs/heads/master@{#826803}
parent 3ea38e29
...@@ -30,6 +30,7 @@ class AssistantKeyboardCoordinator { ...@@ -30,6 +30,7 @@ class AssistantKeyboardCoordinator {
void onKeyboardVisibilityChanged(boolean visible); void onKeyboardVisibilityChanged(boolean visible);
} }
// TODO(b/173103628): refactor and inject the keyboard delegate directly.
AssistantKeyboardCoordinator(Activity activity, AssistantKeyboardCoordinator(Activity activity,
ActivityKeyboardVisibilityDelegate keyboardVisibilityDelegate, ActivityKeyboardVisibilityDelegate keyboardVisibilityDelegate,
CompositorViewHolder compositorViewHolder, AssistantModel model, Delegate delegate, CompositorViewHolder compositorViewHolder, AssistantModel model, Delegate delegate,
......
...@@ -29,6 +29,7 @@ import org.chromium.chrome.browser.compositor.CompositorViewHolder; ...@@ -29,6 +29,7 @@ import org.chromium.chrome.browser.compositor.CompositorViewHolder;
import org.chromium.chrome.browser.signin.UnifiedConsentServiceBridge; import org.chromium.chrome.browser.signin.UnifiedConsentServiceBridge;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController; import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.ActivityKeyboardVisibilityDelegate;
import java.util.Map; import java.util.Map;
...@@ -41,7 +42,8 @@ public class AutofillAssistantModuleEntryImpl implements AutofillAssistantModule ...@@ -41,7 +42,8 @@ public class AutofillAssistantModuleEntryImpl implements AutofillAssistantModule
@Override @Override
public void start(BottomSheetController bottomSheetController, public void start(BottomSheetController bottomSheetController,
BrowserControlsStateProvider browserControls, CompositorViewHolder compositorViewHolder, BrowserControlsStateProvider browserControls, CompositorViewHolder compositorViewHolder,
Context context, @NonNull WebContents webContents, boolean skipOnboarding, Context context, @NonNull WebContents webContents,
ActivityKeyboardVisibilityDelegate keyboardVisibilityDelegate, boolean skipOnboarding,
boolean isChromeCustomTab, @NonNull String initialUrl, Map<String, String> parameters, boolean isChromeCustomTab, @NonNull String initialUrl, Map<String, String> parameters,
String experimentIds, @Nullable String callerAccount, @Nullable String userName) { String experimentIds, @Nullable String callerAccount, @Nullable String userName) {
if (shouldStartTriggerScript(parameters)) { if (shouldStartTriggerScript(parameters)) {
...@@ -61,8 +63,9 @@ public class AutofillAssistantModuleEntryImpl implements AutofillAssistantModule ...@@ -61,8 +63,9 @@ public class AutofillAssistantModuleEntryImpl implements AutofillAssistantModule
if (TextUtils.equals(parameters.get(PARAMETER_REQUEST_TRIGGER_SCRIPT), "true")) { if (TextUtils.equals(parameters.get(PARAMETER_REQUEST_TRIGGER_SCRIPT), "true")) {
AssistantTriggerScriptBridge triggerScriptBridge = AssistantTriggerScriptBridge triggerScriptBridge =
new AssistantTriggerScriptBridge(); new AssistantTriggerScriptBridge();
triggerScriptBridge.start(bottomSheetController, context, webContents, initialUrl, triggerScriptBridge.start(bottomSheetController, context,
parameters, experimentIds, new AssistantTriggerScriptBridge.Delegate() { keyboardVisibilityDelegate, webContents, initialUrl, parameters,
experimentIds, new AssistantTriggerScriptBridge.Delegate() {
@Override @Override
public void onTriggerScriptFinished( public void onTriggerScriptFinished(
@LiteScriptFinishedState int finishedState) { @LiteScriptFinishedState int finishedState) {
......
...@@ -22,6 +22,8 @@ import org.chromium.chrome.browser.feedback.HelpAndFeedbackLauncherImpl; ...@@ -22,6 +22,8 @@ import org.chromium.chrome.browser.feedback.HelpAndFeedbackLauncherImpl;
import org.chromium.chrome.browser.tab.TabUtils; import org.chromium.chrome.browser.tab.TabUtils;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController; import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.KeyboardVisibilityDelegate;
import org.chromium.ui.base.ActivityKeyboardVisibilityDelegate;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
...@@ -36,6 +38,8 @@ public class AssistantTriggerScriptBridge { ...@@ -36,6 +38,8 @@ public class AssistantTriggerScriptBridge {
private long mNativeBridge; private long mNativeBridge;
private Delegate mDelegate; private Delegate mDelegate;
private Context mContext; private Context mContext;
private ActivityKeyboardVisibilityDelegate mKeyboardVisibilityDelegate;
private KeyboardVisibilityDelegate.KeyboardVisibilityListener mKeyboardVisibilityListener;
/** Interface for delegates of the {@code start} method. */ /** Interface for delegates of the {@code start} method. */
public interface Delegate { public interface Delegate {
...@@ -48,10 +52,12 @@ public class AssistantTriggerScriptBridge { ...@@ -48,10 +52,12 @@ public class AssistantTriggerScriptBridge {
* delegate}. * delegate}.
*/ */
public void start(BottomSheetController bottomSheetController, Context context, public void start(BottomSheetController bottomSheetController, Context context,
ActivityKeyboardVisibilityDelegate keyboardVisibilityDelegate,
@NonNull WebContents webContents, @NonNull String initialUrl, @NonNull WebContents webContents, @NonNull String initialUrl,
Map<String, String> scriptParameters, String experimentIds, Delegate delegate) { Map<String, String> scriptParameters, String experimentIds, Delegate delegate) {
mDelegate = delegate; mDelegate = delegate;
mContext = context; mContext = context;
mKeyboardVisibilityDelegate = keyboardVisibilityDelegate;
mTriggerScript = new AssistantTriggerScript(context, new AssistantTriggerScript.Delegate() { mTriggerScript = new AssistantTriggerScript(context, new AssistantTriggerScript.Delegate() {
@Override @Override
public void onTriggerScriptAction(int action) { public void onTriggerScriptAction(int action) {
...@@ -78,6 +84,12 @@ public class AssistantTriggerScriptBridge { ...@@ -78,6 +84,12 @@ public class AssistantTriggerScriptBridge {
} }
}, bottomSheetController); }, bottomSheetController);
if (mKeyboardVisibilityListener != null) {
mKeyboardVisibilityDelegate.removeKeyboardVisibilityListener(
mKeyboardVisibilityListener);
}
mKeyboardVisibilityListener = this::safeNativeOnKeyboardVisibilityChanged;
mKeyboardVisibilityDelegate.addKeyboardVisibilityListener(mKeyboardVisibilityListener);
// Request the client to start the trigger script. Native will then bind itself to this java // Request the client to start the trigger script. Native will then bind itself to this java
// instance via setNativePtr. // instance via setNativePtr.
AutofillAssistantClient.fromWebContents(webContents) AutofillAssistantClient.fromWebContents(webContents)
...@@ -131,6 +143,7 @@ public class AssistantTriggerScriptBridge { ...@@ -131,6 +143,7 @@ public class AssistantTriggerScriptBridge {
private void clearNativePtr() { private void clearNativePtr() {
mNativeBridge = 0; mNativeBridge = 0;
mTriggerScript.destroy(); mTriggerScript.destroy();
mKeyboardVisibilityDelegate.removeKeyboardVisibilityListener(mKeyboardVisibilityListener);
} }
private void safeNativeOnTriggerScriptAction(int action) { private void safeNativeOnTriggerScriptAction(int action) {
...@@ -155,6 +168,13 @@ public class AssistantTriggerScriptBridge { ...@@ -155,6 +168,13 @@ public class AssistantTriggerScriptBridge {
return false; return false;
} }
private void safeNativeOnKeyboardVisibilityChanged(boolean visible) {
if (mNativeBridge != 0) {
AssistantTriggerScriptBridgeJni.get().onKeyboardVisibilityChanged(
mNativeBridge, AssistantTriggerScriptBridge.this, visible);
}
}
@NativeMethods @NativeMethods
interface Natives { interface Natives {
void onTriggerScriptAction(long nativeTriggerScriptBridgeAndroid, void onTriggerScriptAction(long nativeTriggerScriptBridgeAndroid,
...@@ -163,5 +183,7 @@ public class AssistantTriggerScriptBridge { ...@@ -163,5 +183,7 @@ public class AssistantTriggerScriptBridge {
long nativeTriggerScriptBridgeAndroid, AssistantTriggerScriptBridge caller); long nativeTriggerScriptBridgeAndroid, AssistantTriggerScriptBridge caller);
boolean onBackButtonPressed( boolean onBackButtonPressed(
long nativeTriggerScriptBridgeAndroid, AssistantTriggerScriptBridge caller); long nativeTriggerScriptBridgeAndroid, AssistantTriggerScriptBridge caller);
void onKeyboardVisibilityChanged(long nativeTriggerScriptBridgeAndroid,
AssistantTriggerScriptBridge caller, boolean visible);
} }
} }
...@@ -16,6 +16,7 @@ import org.chromium.chrome.browser.compositor.CompositorViewHolder; ...@@ -16,6 +16,7 @@ import org.chromium.chrome.browser.compositor.CompositorViewHolder;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController; import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.ActivityKeyboardVisibilityDelegate;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
...@@ -65,8 +66,10 @@ class TestingAutofillAssistantModuleEntryProvider extends AutofillAssistantModul ...@@ -65,8 +66,10 @@ class TestingAutofillAssistantModuleEntryProvider extends AutofillAssistantModul
public void start(BottomSheetController bottomSheetController, public void start(BottomSheetController bottomSheetController,
BrowserControlsStateProvider browserControls, BrowserControlsStateProvider browserControls,
CompositorViewHolder compositorViewHolder, Context context, CompositorViewHolder compositorViewHolder, Context context,
@NonNull WebContents webContents, boolean skipOnboarding, boolean isChromeCustomTab, @NonNull WebContents webContents,
@NonNull String initialUrl, Map<String, String> parameters, String experimentIds, ActivityKeyboardVisibilityDelegate keyboardVisibilityDelegate,
boolean skipOnboarding, boolean isChromeCustomTab, @NonNull String initialUrl,
Map<String, String> parameters, String experimentIds,
@Nullable String callerAccount, @Nullable String userName) {} @Nullable String callerAccount, @Nullable String userName) {}
@Override @Override
......
...@@ -161,6 +161,7 @@ public class AutofillAssistantFacade { ...@@ -161,6 +161,7 @@ public class AutofillAssistantFacade {
BottomSheetControllerProvider.from(activity.getWindowAndroid()), BottomSheetControllerProvider.from(activity.getWindowAndroid()),
activity.getBrowserControlsManager(), activity.getBrowserControlsManager(),
activity.getCompositorViewHolder(), activity, tab.getWebContents(), activity.getCompositorViewHolder(), activity, tab.getWebContents(),
activity.getWindowAndroid().getKeyboardDelegate(),
!AutofillAssistantPreferencesUtil.getShowOnboarding(), !AutofillAssistantPreferencesUtil.getShowOnboarding(),
activity instanceof CustomTabActivity, arguments.getInitialUrl(), activity instanceof CustomTabActivity, arguments.getInitialUrl(),
arguments.getParameters(), arguments.getExperimentIds(), arguments.getParameters(), arguments.getExperimentIds(),
......
...@@ -15,6 +15,7 @@ import org.chromium.chrome.browser.compositor.CompositorViewHolder; ...@@ -15,6 +15,7 @@ import org.chromium.chrome.browser.compositor.CompositorViewHolder;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController; import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.components.module_installer.builder.ModuleInterface; import org.chromium.components.module_installer.builder.ModuleInterface;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.ActivityKeyboardVisibilityDelegate;
import java.util.Map; import java.util.Map;
...@@ -33,7 +34,8 @@ interface AutofillAssistantModuleEntry { ...@@ -33,7 +34,8 @@ interface AutofillAssistantModuleEntry {
*/ */
void start(BottomSheetController bottomSheetController, void start(BottomSheetController bottomSheetController,
BrowserControlsStateProvider browserControls, CompositorViewHolder compositorViewHolder, BrowserControlsStateProvider browserControls, CompositorViewHolder compositorViewHolder,
Context context, @NonNull WebContents webContents, boolean skipOnboarding, Context context, @NonNull WebContents webContents,
ActivityKeyboardVisibilityDelegate keyboardVisibilityDelegate, boolean skipOnboarding,
boolean isChromeCustomTab, @NonNull String initialUrl, Map<String, String> parameters, boolean isChromeCustomTab, @NonNull String initialUrl, Map<String, String> parameters,
String experimentIds, @Nullable String callerAccount, @Nullable String userName); String experimentIds, @Nullable String callerAccount, @Nullable String userName);
/** /**
......
...@@ -110,6 +110,16 @@ bool TriggerScriptBridgeAndroid::OnBackButtonPressed( ...@@ -110,6 +110,16 @@ bool TriggerScriptBridgeAndroid::OnBackButtonPressed(
return trigger_script_coordinator_->OnBackButtonPressed(); return trigger_script_coordinator_->OnBackButtonPressed();
} }
void TriggerScriptBridgeAndroid::OnKeyboardVisibilityChanged(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
jboolean jvisible) {
if (!trigger_script_coordinator_) {
return;
}
trigger_script_coordinator_->OnKeyboardVisibilityChanged(jvisible);
}
void TriggerScriptBridgeAndroid::OnTriggerScriptShown( void TriggerScriptBridgeAndroid::OnTriggerScriptShown(
const TriggerScriptUIProto& proto) { const TriggerScriptUIProto& proto) {
if (!java_object_) { if (!java_object_) {
......
...@@ -57,6 +57,12 @@ class TriggerScriptBridgeAndroid : public TriggerScriptCoordinator::Observer { ...@@ -57,6 +57,12 @@ class TriggerScriptBridgeAndroid : public TriggerScriptCoordinator::Observer {
bool OnBackButtonPressed(JNIEnv* env, bool OnBackButtonPressed(JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller); const base::android::JavaParamRef<jobject>& jcaller);
// Called by the UI when the keyboard was shown or hidden.
void OnKeyboardVisibilityChanged(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
jboolean jvisible);
private: private:
// From TriggerScriptCoordinator::Observer: // From TriggerScriptCoordinator::Observer:
void OnTriggerScriptShown(const TriggerScriptUIProto& proto) override; void OnTriggerScriptShown(const TriggerScriptUIProto& proto) override;
......
...@@ -527,6 +527,8 @@ message TriggerScriptConditionProto { ...@@ -527,6 +527,8 @@ message TriggerScriptConditionProto {
Empty is_first_time_user = 6; Empty is_first_time_user = 6;
// The condition matches if the client is in the specified experiment. // The condition matches if the client is in the specified experiment.
int32 experiment_id = 7; int32 experiment_id = 7;
// The condition matches if the keyboard is hidden.
Empty keyboard_hidden = 9;
} }
reserved 4; reserved 4;
......
...@@ -34,6 +34,7 @@ void ExtractSelectors(const TriggerScriptConditionProto& proto, ...@@ -34,6 +34,7 @@ void ExtractSelectors(const TriggerScriptConditionProto& proto,
case TriggerScriptConditionProto::kStoredLoginCredentials: case TriggerScriptConditionProto::kStoredLoginCredentials:
case TriggerScriptConditionProto::kIsFirstTimeUser: case TriggerScriptConditionProto::kIsFirstTimeUser:
case TriggerScriptConditionProto::kExperimentId: case TriggerScriptConditionProto::kExperimentId:
case TriggerScriptConditionProto::kKeyboardHidden:
case TriggerScriptConditionProto::TYPE_NOT_SET: case TriggerScriptConditionProto::TYPE_NOT_SET:
return; return;
case TriggerScriptConditionProto::kSelector: case TriggerScriptConditionProto::kSelector:
...@@ -65,6 +66,14 @@ base::Optional<bool> DynamicTriggerConditions::GetSelectorMatches( ...@@ -65,6 +66,14 @@ base::Optional<bool> DynamicTriggerConditions::GetSelectorMatches(
return it->second; return it->second;
} }
void DynamicTriggerConditions::SetKeyboardVisible(bool visible) {
keyboard_visible_ = visible;
}
bool DynamicTriggerConditions::GetKeyboardVisible() const {
return keyboard_visible_;
}
void DynamicTriggerConditions::Update(WebController* web_controller, void DynamicTriggerConditions::Update(WebController* web_controller,
base::OnceCallback<void(void)> callback) { base::OnceCallback<void(void)> callback) {
DCHECK(!callback_) << "Update called while already in progress"; DCHECK(!callback_) << "Update called while already in progress";
...@@ -72,8 +81,9 @@ void DynamicTriggerConditions::Update(WebController* web_controller, ...@@ -72,8 +81,9 @@ void DynamicTriggerConditions::Update(WebController* web_controller,
return; return;
} }
selector_matches_.clear(); temporary_selector_matches_.clear();
if (selectors_.empty()) { if (selectors_.empty()) {
selector_matches_ = temporary_selector_matches_;
std::move(callback).Run(); std::move(callback).Run();
return; return;
} }
...@@ -87,12 +97,18 @@ void DynamicTriggerConditions::Update(WebController* web_controller, ...@@ -87,12 +97,18 @@ void DynamicTriggerConditions::Update(WebController* web_controller,
} }
} }
bool DynamicTriggerConditions::HasResults() const {
return selector_matches_.size() == selectors_.size();
}
void DynamicTriggerConditions::OnFindElement( void DynamicTriggerConditions::OnFindElement(
const Selector& selector, const Selector& selector,
const ClientStatus& client_status, const ClientStatus& client_status,
std::unique_ptr<ElementFinder::Result> element) { std::unique_ptr<ElementFinder::Result> element) {
selector_matches_.emplace(std::make_pair(selector, client_status.ok())); temporary_selector_matches_.emplace(
if (selector_matches_.size() == selectors_.size()) { std::make_pair(selector, client_status.ok()));
if (temporary_selector_matches_.size() == selectors_.size()) {
selector_matches_ = temporary_selector_matches_;
std::move(callback_).Run(); std::move(callback_).Run();
} }
} }
......
...@@ -37,6 +37,12 @@ class DynamicTriggerConditions { ...@@ -37,6 +37,12 @@ class DynamicTriggerConditions {
virtual base::Optional<bool> GetSelectorMatches( virtual base::Optional<bool> GetSelectorMatches(
const Selector& selector) const; const Selector& selector) const;
// Sets whether the keyboard is currently visible.
virtual void SetKeyboardVisible(bool visible);
// Returns whether the keyboard is currently visible.
virtual bool GetKeyboardVisible() const;
// Matches all previously added selectors with the current DOM tree and caches // Matches all previously added selectors with the current DOM tree and caches
// the results to be available via |GetSelectorMatches|. Invokes |callback| // the results to be available via |GetSelectorMatches|. Invokes |callback|
// when done. // when done.
...@@ -46,6 +52,10 @@ class DynamicTriggerConditions { ...@@ -46,6 +52,10 @@ class DynamicTriggerConditions {
virtual void Update(WebController* web_controller, virtual void Update(WebController* web_controller,
base::OnceCallback<void(void)> callback); base::OnceCallback<void(void)> callback);
// If true, all values have been evaluated. They may be out-of-date by one
// cycle in case an update is currently scheduled.
virtual bool HasResults() const;
private: private:
friend class DynamicTriggerConditionsTest; friend class DynamicTriggerConditionsTest;
...@@ -57,8 +67,14 @@ class DynamicTriggerConditions { ...@@ -57,8 +67,14 @@ class DynamicTriggerConditions {
const ClientStatus& client_status, const ClientStatus& client_status,
std::unique_ptr<ElementFinder::Result> element); std::unique_ptr<ElementFinder::Result> element);
// Whether the keyboard is currently visible.
bool keyboard_visible_ = false;
// Lookup cache for selector matches. Must be updated by invoking |Update|. // Lookup cache for selector matches. Must be updated by invoking |Update|.
std::map<Selector, bool> selector_matches_; std::map<Selector, bool> selector_matches_;
// Temporary store for selector matches, used during |Update| as results
// trickle in. Once all results have been gathered, this becomes the new
// |selector_matches_|.
std::map<Selector, bool> temporary_selector_matches_;
// The list of selectors to look up on |Update|. // The list of selectors to look up on |Update|.
std::set<Selector> selectors_; std::set<Selector> selectors_;
// The callback to invoke after |Update| is finished. Only set during // The callback to invoke after |Update| is finished. Only set during
......
...@@ -124,4 +124,41 @@ TEST_F(DynamicTriggerConditionsTest, ClearSelectors) { ...@@ -124,4 +124,41 @@ TEST_F(DynamicTriggerConditionsTest, ClearSelectors) {
EXPECT_EQ(GetSelectorsForTest()->size(), 0u); EXPECT_EQ(GetSelectorsForTest()->size(), 0u);
} }
TEST_F(DynamicTriggerConditionsTest, HasResults) {
// Since no selectors were added to the evaluation, the result is valid.
EXPECT_TRUE(dynamic_trigger_conditions_.HasResults());
TriggerScriptProto proto;
*proto.mutable_trigger_condition()->mutable_selector() = ToSelectorProto("a");
dynamic_trigger_conditions_.AddSelectorsFromTriggerScript(proto);
EXPECT_FALSE(dynamic_trigger_conditions_.HasResults());
EXPECT_CALL(mock_web_controller_,
OnFindElement(Selector(ToSelectorProto("a")), _))
.WillOnce(RunOnceCallback<1>(OkClientStatus(), nullptr));
dynamic_trigger_conditions_.Update(&mock_web_controller_,
mock_callback_.Get());
EXPECT_TRUE(dynamic_trigger_conditions_.HasResults());
EXPECT_CALL(mock_web_controller_,
OnFindElement(Selector(ToSelectorProto("a")), _))
.WillOnce(
[&](const Selector& selector, ElementFinder::Callback& callback) {
// While Update is running, GetSelectorMatches should return the
// previous results.
EXPECT_EQ(dynamic_trigger_conditions_.GetSelectorMatches(
Selector(ToSelectorProto("a"))),
base::make_optional(true));
std::move(callback).Run(ClientStatus(ELEMENT_RESOLUTION_FAILED),
nullptr);
});
dynamic_trigger_conditions_.Update(&mock_web_controller_,
mock_callback_.Get());
// After the update, the new result is returned.
EXPECT_EQ(dynamic_trigger_conditions_.GetSelectorMatches(
Selector(ToSelectorProto("a"))),
base::make_optional(false));
}
} // namespace autofill_assistant } // namespace autofill_assistant
...@@ -29,6 +29,7 @@ class MockDynamicTriggerConditions : public DynamicTriggerConditions { ...@@ -29,6 +29,7 @@ class MockDynamicTriggerConditions : public DynamicTriggerConditions {
MOCK_METHOD1(AddSelectorsFromTriggerScript, MOCK_METHOD1(AddSelectorsFromTriggerScript,
void(const TriggerScriptProto& proto)); void(const TriggerScriptProto& proto));
MOCK_METHOD0(ClearSelectors, void(void)); MOCK_METHOD0(ClearSelectors, void(void));
MOCK_CONST_METHOD0(HasResults, bool(void));
}; };
} // namespace autofill_assistant } // namespace autofill_assistant
......
...@@ -24,6 +24,7 @@ class MockStaticTriggerConditions : public StaticTriggerConditions { ...@@ -24,6 +24,7 @@ class MockStaticTriggerConditions : public StaticTriggerConditions {
MOCK_CONST_METHOD0(is_first_time_user, bool()); MOCK_CONST_METHOD0(is_first_time_user, bool());
MOCK_CONST_METHOD0(has_stored_login_credentials, bool()); MOCK_CONST_METHOD0(has_stored_login_credentials, bool());
MOCK_CONST_METHOD1(is_in_experiment, bool(int experiment_id)); MOCK_CONST_METHOD1(is_in_experiment, bool(int experiment_id));
MOCK_CONST_METHOD0(has_results, bool());
}; };
} // namespace autofill_assistant } // namespace autofill_assistant
......
...@@ -48,9 +48,14 @@ bool StaticTriggerConditions::is_in_experiment(int experiment_id) const { ...@@ -48,9 +48,14 @@ bool StaticTriggerConditions::is_in_experiment(int experiment_id) const {
return trigger_context_->HasExperimentId(base::NumberToString(experiment_id)); return trigger_context_->HasExperimentId(base::NumberToString(experiment_id));
} }
bool StaticTriggerConditions::has_results() const {
return has_results_;
}
void StaticTriggerConditions::OnGetLogins( void StaticTriggerConditions::OnGetLogins(
std::vector<WebsiteLoginManager::Login> logins) { std::vector<WebsiteLoginManager::Login> logins) {
has_stored_login_credentials_ = !logins.empty(); has_stored_login_credentials_ = !logins.empty();
has_results_ = true;
DCHECK(callback_); DCHECK(callback_);
std::move(callback_).Run(); std::move(callback_).Run();
} }
......
...@@ -27,8 +27,6 @@ class StaticTriggerConditions { ...@@ -27,8 +27,6 @@ class StaticTriggerConditions {
// Initializes the field values according to |url| and the current state of // Initializes the field values according to |url| and the current state of
// |client|. Invokes |callback| when done. |client| and |trigger_context| must // |client|. Invokes |callback| when done. |client| and |trigger_context| must
// outlive this instance. // outlive this instance.
// NOTE: this class is not thread-safe. Do not invoke methods of this class
// while a call to |Init| has yet to finish.
virtual void Init(Client* client, virtual void Init(Client* client,
const GURL& url, const GURL& url,
TriggerContext* trigger_context, TriggerContext* trigger_context,
...@@ -38,12 +36,17 @@ class StaticTriggerConditions { ...@@ -38,12 +36,17 @@ class StaticTriggerConditions {
virtual bool has_stored_login_credentials() const; virtual bool has_stored_login_credentials() const;
virtual bool is_in_experiment(int experiment_id) const; virtual bool is_in_experiment(int experiment_id) const;
// If true, all values have been evaluated. They may be out-of-date by one
// cycle in case an update is currently scheduled.
virtual bool has_results() const;
private: private:
void OnGetLogins(std::vector<WebsiteLoginManager::Login> logins); void OnGetLogins(std::vector<WebsiteLoginManager::Login> logins);
// The callback to invoke once all information requested in |Init| has been // The callback to invoke once all information requested in |Init| has been
// collected. Only valid during calls of |Init|. // collected. Only valid during calls of |Init|.
base::OnceCallback<void(void)> callback_; base::OnceCallback<void(void)> callback_;
bool has_results_ = false;
bool is_first_time_user_ = true; bool is_first_time_user_ = true;
bool has_stored_login_credentials_ = false; bool has_stored_login_credentials_ = false;
TriggerContext* trigger_context_; TriggerContext* trigger_context_;
......
...@@ -61,5 +61,22 @@ TEST_F(StaticTriggerConditionsTest, SetIsFirstTimeUser) { ...@@ -61,5 +61,22 @@ TEST_F(StaticTriggerConditionsTest, SetIsFirstTimeUser) {
EXPECT_FALSE(static_trigger_conditions_.is_first_time_user()); EXPECT_FALSE(static_trigger_conditions_.is_first_time_user());
} }
TEST_F(StaticTriggerConditionsTest, HasResults) {
EXPECT_FALSE(static_trigger_conditions_.has_results());
TriggerContextImpl trigger_context(/* params = */ {}, /* exp = */ "1,2,4");
EXPECT_CALL(mock_client_, IsFirstTimeTriggerScriptUser)
.WillOnce(Return(true));
EXPECT_CALL(mock_client_, GetWebsiteLoginManager)
.WillOnce(Return(&mock_website_login_manager_));
EXPECT_CALL(mock_website_login_manager_, OnGetLoginsForUrl(GURL(kFakeUrl), _))
.WillOnce(RunOnceCallback<1>(std::vector<WebsiteLoginManager::Login>{
WebsiteLoginManager::Login(GURL(kFakeUrl), "fake_username")}));
EXPECT_CALL(mock_callback_, Run).Times(1);
static_trigger_conditions_.Init(&mock_client_, GURL(kFakeUrl),
&trigger_context, mock_callback_.Get());
EXPECT_TRUE(static_trigger_conditions_.has_results());
}
} // namespace } // namespace
} // namespace autofill_assistant } // namespace autofill_assistant
...@@ -52,6 +52,8 @@ bool EvaluateTriggerCondition( ...@@ -52,6 +52,8 @@ bool EvaluateTriggerCondition(
return static_trigger_conditions.is_first_time_user(); return static_trigger_conditions.is_first_time_user();
case TriggerScriptConditionProto::kExperimentId: case TriggerScriptConditionProto::kExperimentId:
return static_trigger_conditions.is_in_experiment(proto.experiment_id()); return static_trigger_conditions.is_in_experiment(proto.experiment_id());
case TriggerScriptConditionProto::kKeyboardHidden:
return !dynamic_trigger_conditions.GetKeyboardVisible();
case TriggerScriptConditionProto::TYPE_NOT_SET: case TriggerScriptConditionProto::TYPE_NOT_SET:
return true; return true;
} }
......
...@@ -201,6 +201,11 @@ bool TriggerScriptCoordinator::OnBackButtonPressed() { ...@@ -201,6 +201,11 @@ bool TriggerScriptCoordinator::OnBackButtonPressed() {
return true; return true;
} }
void TriggerScriptCoordinator::OnKeyboardVisibilityChanged(bool visible) {
dynamic_trigger_conditions_->SetKeyboardVisible(visible);
RunOutOfScheduleTriggerConditionCheck();
}
void TriggerScriptCoordinator::Stop(Metrics::LiteScriptFinishedState state) { void TriggerScriptCoordinator::Stop(Metrics::LiteScriptFinishedState state) {
HideTriggerScript(); HideTriggerScript();
StopCheckingTriggerConditions(); StopCheckingTriggerConditions();
...@@ -300,7 +305,8 @@ void TriggerScriptCoordinator::CheckDynamicTriggerConditions() { ...@@ -300,7 +305,8 @@ void TriggerScriptCoordinator::CheckDynamicTriggerConditions() {
web_controller_.get(), web_controller_.get(),
base::BindOnce( base::BindOnce(
&TriggerScriptCoordinator::OnDynamicTriggerConditionsEvaluated, &TriggerScriptCoordinator::OnDynamicTriggerConditionsEvaluated,
weak_ptr_factory_.GetWeakPtr())); weak_ptr_factory_.GetWeakPtr(),
/* is_out_of_schedule = */ false));
} }
void TriggerScriptCoordinator::StopCheckingTriggerConditions() { void TriggerScriptCoordinator::StopCheckingTriggerConditions() {
...@@ -338,10 +344,16 @@ void TriggerScriptCoordinator::HideTriggerScript() { ...@@ -338,10 +344,16 @@ void TriggerScriptCoordinator::HideTriggerScript() {
} }
} }
void TriggerScriptCoordinator::OnDynamicTriggerConditionsEvaluated() { void TriggerScriptCoordinator::OnDynamicTriggerConditionsEvaluated(
bool is_out_of_schedule) {
if (!web_contents_visible_ || !is_checking_trigger_conditions_) { if (!web_contents_visible_ || !is_checking_trigger_conditions_) {
return; return;
} }
if (!static_trigger_conditions_->has_results() ||
!dynamic_trigger_conditions_->HasResults()) {
DCHECK(is_out_of_schedule);
return;
}
std::vector<bool> evaluated_trigger_conditions; std::vector<bool> evaluated_trigger_conditions;
for (const auto& trigger_script : trigger_scripts_) { for (const auto& trigger_script : trigger_scripts_) {
...@@ -395,6 +407,10 @@ void TriggerScriptCoordinator::OnDynamicTriggerConditionsEvaluated() { ...@@ -395,6 +407,10 @@ void TriggerScriptCoordinator::OnDynamicTriggerConditionsEvaluated() {
} }
} }
if (is_out_of_schedule) {
// Out-of-schedule checks do not count towards the timeout.
return;
}
if (visible_trigger_script_ == -1 && if (visible_trigger_script_ == -1 &&
remaining_trigger_condition_evaluations_ > 0) { remaining_trigger_condition_evaluations_ > 0) {
remaining_trigger_condition_evaluations_--; remaining_trigger_condition_evaluations_--;
...@@ -411,6 +427,10 @@ void TriggerScriptCoordinator::OnDynamicTriggerConditionsEvaluated() { ...@@ -411,6 +427,10 @@ void TriggerScriptCoordinator::OnDynamicTriggerConditionsEvaluated() {
trigger_condition_check_interval_); trigger_condition_check_interval_);
} }
void TriggerScriptCoordinator::RunOutOfScheduleTriggerConditionCheck() {
OnDynamicTriggerConditionsEvaluated(/* is_out_of_schedule = */ true);
}
void TriggerScriptCoordinator::NotifyOnTriggerScriptFinished( void TriggerScriptCoordinator::NotifyOnTriggerScriptFinished(
Metrics::LiteScriptFinishedState state) { Metrics::LiteScriptFinishedState state) {
if (!finished_state_recorded_) { if (!finished_state_recorded_) {
......
...@@ -79,6 +79,9 @@ class TriggerScriptCoordinator : public content::WebContentsObserver { ...@@ -79,6 +79,9 @@ class TriggerScriptCoordinator : public content::WebContentsObserver {
// handled or not. // handled or not.
bool OnBackButtonPressed(); bool OnBackButtonPressed();
// Called when the keyboard was shown or hidden.
void OnKeyboardVisibilityChanged(bool visible);
void AddObserver(Observer* observer); void AddObserver(Observer* observer);
void RemoveObserver(const Observer* observer); void RemoveObserver(const Observer* observer);
...@@ -96,10 +99,15 @@ class TriggerScriptCoordinator : public content::WebContentsObserver { ...@@ -96,10 +99,15 @@ class TriggerScriptCoordinator : public content::WebContentsObserver {
void ShowTriggerScript(int index); void ShowTriggerScript(int index);
void HideTriggerScript(); void HideTriggerScript();
void CheckDynamicTriggerConditions(); void CheckDynamicTriggerConditions();
void OnDynamicTriggerConditionsEvaluated(); void OnDynamicTriggerConditionsEvaluated(bool is_out_of_schedule);
void OnGetTriggerScripts(int http_status, const std::string& response); void OnGetTriggerScripts(int http_status, const std::string& response);
void Stop(Metrics::LiteScriptFinishedState state); void Stop(Metrics::LiteScriptFinishedState state);
// Can be invoked to trigger an immediate check of the trigger condition,
// reusing the dynamic results of the last time. Does nothing if there are no
// previous results to reuse.
void RunOutOfScheduleTriggerConditionCheck();
void NotifyOnTriggerScriptFinished(Metrics::LiteScriptFinishedState state); void NotifyOnTriggerScriptFinished(Metrics::LiteScriptFinishedState state);
// Used to retrieve deps and also to request shutdown and, if applicable, // Used to retrieve deps and also to request shutdown and, if applicable,
......
...@@ -73,6 +73,11 @@ class TriggerScriptCoordinatorTest : public content::RenderViewHostTestHarness { ...@@ -73,6 +73,11 @@ class TriggerScriptCoordinatorTest : public content::RenderViewHostTestHarness {
std::make_unique<NiceMock<MockDynamicTriggerConditions>>(); std::make_unique<NiceMock<MockDynamicTriggerConditions>>();
mock_dynamic_trigger_conditions_ = mock_dynamic_trigger_conditions.get(); mock_dynamic_trigger_conditions_ = mock_dynamic_trigger_conditions.get();
ON_CALL(*mock_static_trigger_conditions, has_results)
.WillByDefault(Return(true));
ON_CALL(*mock_dynamic_trigger_conditions, HasResults)
.WillByDefault(Return(true));
coordinator_ = std::make_unique<TriggerScriptCoordinator>( coordinator_ = std::make_unique<TriggerScriptCoordinator>(
&mock_client_, std::move(mock_web_controller), &mock_client_, std::move(mock_web_controller),
std::move(mock_request_sender), GURL(kFakeServerUrl), std::move(mock_request_sender), GURL(kFakeServerUrl),
...@@ -680,4 +685,54 @@ TEST_F(TriggerScriptCoordinatorTest, NoTimeoutByDefault) { ...@@ -680,4 +685,54 @@ TEST_F(TriggerScriptCoordinatorTest, NoTimeoutByDefault) {
} }
} }
TEST_F(TriggerScriptCoordinatorTest, KeyboardEventTriggersOutOfScheduleCheck) {
GetTriggerScriptsResponseProto response;
*response.add_trigger_scripts()
->mutable_trigger_condition()
->mutable_selector() = ToSelectorProto("#selector");
response.set_timeout_ms(3000);
response.set_trigger_condition_check_interval_ms(1000);
std::string serialized_response;
response.SerializeToString(&serialized_response);
EXPECT_CALL(*mock_request_sender_, OnSendRequest(GURL(kFakeServerUrl), _, _))
.WillOnce(RunOnceCallback<2>(net::HTTP_OK, serialized_response));
EXPECT_CALL(*mock_static_trigger_conditions_, Init)
.WillOnce(RunOnceCallback<3>());
EXPECT_CALL(*mock_dynamic_trigger_conditions_,
OnUpdate(mock_web_controller_, _))
.WillOnce(RunOnceCallback<1>());
EXPECT_CALL(*mock_dynamic_trigger_conditions_, GetSelectorMatches)
.WillOnce(Return(false));
coordinator_->Start(GURL(kFakeDeepLink),
std::make_unique<TriggerContextImpl>());
// While the next call to Update is pending, a keyboard visibility event will
// immediately trigger an out-of-schedule update (which does not count towards
// the timeout).
for (int i = 0; i < 3; ++i) {
EXPECT_CALL(*mock_dynamic_trigger_conditions_, GetSelectorMatches)
.WillOnce(Return(false));
EXPECT_CALL(*mock_dynamic_trigger_conditions_, OnUpdate).Times(0);
coordinator_->OnKeyboardVisibilityChanged(true);
}
EXPECT_CALL(*mock_dynamic_trigger_conditions_, GetSelectorMatches)
.Times(3)
.WillRepeatedly(Return(false));
EXPECT_CALL(*mock_dynamic_trigger_conditions_,
OnUpdate(mock_web_controller_, _))
.Times(3)
.WillRepeatedly(RunOnceCallback<1>());
task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(1));
task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(1));
EXPECT_CALL(mock_observer_, OnTriggerScriptFinished(
Metrics::LiteScriptFinishedState::
LITE_SCRIPT_TRIGGER_CONDITION_TIMEOUT));
task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(1));
AssertRecordedFinishedState(
Metrics::LiteScriptFinishedState::LITE_SCRIPT_TRIGGER_CONDITION_TIMEOUT);
}
} // namespace autofill_assistant } // namespace autofill_assistant
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