Commit 6cfdceed authored by Clemens Arbesser's avatar Clemens Arbesser Committed by Commit Bot

[Autofill Assistant] Add basic trigger script testing framework.

This CL adds a basic integration framework to allow writing integration
tests for trigger scripts. This is done analogous to the existing way
for regular scripts. However, the new way of injecting the request
sender directly (rather than a test service) is much more generic and
should in the mid-term replace the older way.

This CL also fixes several small issues that surfaced during testing:
- Fixed issue where an empty callout message would be shown.
- Fixed issue where the first time user flag was set too early.
- Fixed issue where the Facade would not correctly trigger the new
trigger scripts.

Bug: b/171776026
Change-Id: I7c5a77cd317996d58db57026e322fd982dfacdf1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2524530
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Reviewed-by: default avatarMarian Fechete <marianfe@google.com>
Cr-Commit-Position: refs/heads/master@{#826260}
parent 82a78873
......@@ -231,7 +231,10 @@ generate_jni("jni_headers") {
generate_jni("test_support_jni_headers") {
testonly = true
sources = [ "javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantTestService.java" ]
sources = [
"javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantTestService.java",
"javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantTestServiceRequestSender.java",
]
}
android_library("autofill_assistant_java_test_support") {
......@@ -241,6 +244,7 @@ android_library("autofill_assistant_java_test_support") {
"javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantService.java",
"javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantTestScript.java",
"javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantTestService.java",
"javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantTestServiceRequestSender.java",
]
deps = [
......@@ -288,6 +292,7 @@ android_library("test_java") {
"javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantProgressBarIntegrationTest.java",
"javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantPromptNavigationIntegrationTest.java",
"javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantTextUtilsTest.java",
"javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantTriggerScriptIntegrationTest.java",
"javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantTriggerScriptTest.java",
"javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiTest.java",
"javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiTestUtil.java",
......
......@@ -133,7 +133,8 @@ public class AutofillAssistantClient {
AutofillAssistantClientJni.get().startTriggerScript(mNativeClientAndroid, this, delegate,
initialUrl, experimentIds,
parameters.keySet().toArray(new String[parameters.size()]),
parameters.values().toArray(new String[parameters.size()]));
parameters.values().toArray(new String[parameters.size()]),
AutofillAssistantServiceInjector.getServiceRequestSenderToInject());
}
/**
......@@ -408,7 +409,7 @@ public class AutofillAssistantClient {
boolean onboardingShown, long nativeService);
void startTriggerScript(long nativeClientAndroid, AutofillAssistantClient caller,
AssistantTriggerScriptBridge delegate, String initialUrl, String experimentIds,
String[] parameterNames, String[] parameterValues);
String[] parameterNames, String[] parameterValues, long nativeServiceRequestSender);
void onAccessToken(long nativeClientAndroid, AutofillAssistantClient caller,
boolean success, String accessToken);
String getPrimaryAccountName(long nativeClientAndroid, AutofillAssistantClient caller);
......
......@@ -20,12 +20,28 @@ public class AutofillAssistantServiceInjector {
long createNativeService();
}
/**
* Interface for service request senders.
*/
public interface NativeServiceRequestSenderProvider {
/**
* Returns a pointer to a native service request sender, or 0 on failure.
*/
long createNativeServiceRequestSender();
}
/**
* Provider to create the native service to inject. Will be automatically called upon client
* startup.
*/
private static NativeServiceProvider sNativeServiceProvider;
/**
* Provider to create the native service request sender to inject. Will be automatically called
* upon trigger script startup.
*/
private static NativeServiceRequestSenderProvider sNativeServiceRequestSenderProvider;
/**
* Sets a service provider to create a native service to inject upon client startup.
*/
......@@ -33,6 +49,15 @@ public class AutofillAssistantServiceInjector {
sNativeServiceProvider = nativeServiceProvider;
}
/**
* Sets a service request sender provider to create a native service request sender to inject
* upon trigger script startup.
*/
public static void setServiceRequestSenderToInject(
NativeServiceRequestSenderProvider nativeServiceRequestSenderProvider) {
sNativeServiceRequestSenderProvider = nativeServiceRequestSenderProvider;
}
/**
* Returns the native pointer to the service to inject, or 0 if no service has been set (and the
* default should be used).
......@@ -47,4 +72,19 @@ public class AutofillAssistantServiceInjector {
return sNativeServiceProvider.createNativeService();
}
/**
* Returns the native pointer to the service request sender to inject, or 0 if no service
* request sender has been set (and the default should be used).
*
* <p>Please note: the caller must ensure to take ownership of the returned native pointer,
* else it will leak!</p>
*/
public static long getServiceRequestSenderToInject() {
if (sNativeServiceRequestSenderProvider == null) {
return 0;
}
return sNativeServiceRequestSenderProvider.createNativeServiceRequestSender();
}
}
......@@ -174,6 +174,9 @@ class AssistantHeaderViewBinder
private void showOrDismissBubble(AssistantHeaderModel model, ViewHolder view) {
String message = model.get(AssistantHeaderModel.BUBBLE_MESSAGE);
if (message.isEmpty() && view.mTextBubble == null) {
return;
}
if (message.isEmpty() && view.mTextBubble != null) {
view.mTextBubble.dismiss();
view.mTextBubble = null;
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.autofill_assistant;
import com.google.protobuf.GeneratedMessageLite;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods;
import java.util.ArrayList;
import java.util.List;
/**
* Test service request sender which serves preconfigured answers to server requests.
*/
@JNINamespace("autofill_assistant")
public class AutofillAssistantTestServiceRequestSender
implements AutofillAssistantServiceInjector.NativeServiceRequestSenderProvider {
static class Request {
Request(String url, byte[] request) {
mUrl = url;
mRequest = request;
}
String mUrl;
byte[] mRequest;
}
// TODO(arbesser): make this field accessible to callers.
private final List<Request> mReceivedRequests = new ArrayList();
private long mNativeServiceRequestSender;
int mNextHttpStatus;
GeneratedMessageLite mNextResponse;
AutofillAssistantTestServiceRequestSender() {}
/**
* Asks the client to inject this service request sender upon startup. Must be called prior to
* trigger script startup in order to take effect!
*/
void scheduleForInjection() {
AutofillAssistantServiceInjector.setServiceRequestSenderToInject(this);
}
/**
* Sets the response that will be returned for the next (and only the next) call to {@code
* SendRequest}.
*/
void setNextResponse(int httpStatus, GeneratedMessageLite response) {
mNextHttpStatus = httpStatus;
mNextResponse = response;
}
@Override
public long createNativeServiceRequestSender() {
// Ask native to create and return a wrapper around |this|. The wrapper will be injected
// upon startup, at which point native will take ownership of the wrapper.
mNativeServiceRequestSender =
AutofillAssistantTestServiceRequestSenderJni.get().createNative(this);
// Hold on to the created pointer in order to be able to keep communicating with the native
// instance.
return mNativeServiceRequestSender;
}
@CalledByNative
void sendRequest(String url, byte[] request) {
assert mNextResponse != null;
mReceivedRequests.add(new Request(url, request));
AutofillAssistantTestServiceRequestSenderJni.get().onResponse(mNativeServiceRequestSender,
AutofillAssistantTestServiceRequestSender.this, mNextHttpStatus,
mNextResponse.toByteArray());
mNextResponse = null;
mNextHttpStatus = -1;
}
@NativeMethods
interface Natives {
long createNative(AutofillAssistantTestServiceRequestSender caller);
void onResponse(long nativeJavaServiceRequestSender,
AutofillAssistantTestServiceRequestSender caller, int httpStatus, byte[] response);
}
}
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.autofill_assistant;
import static androidx.test.espresso.matcher.ViewMatchers.isCompletelyDisplayed;
import static androidx.test.espresso.matcher.ViewMatchers.withText;
import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.waitUntilViewMatchesCondition;
import android.support.test.InstrumentationRegistry;
import androidx.test.filters.MediumTest;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.chrome.browser.autofill_assistant.proto.Empty;
import org.chromium.chrome.browser.autofill_assistant.proto.GetTriggerScriptsResponseProto;
import org.chromium.chrome.browser.autofill_assistant.proto.TriggerScriptConditionProto;
import org.chromium.chrome.browser.autofill_assistant.proto.TriggerScriptProto;
import org.chromium.chrome.browser.autofill_assistant.proto.TriggerScriptUIProto;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.signin.UnifiedConsentServiceBridge;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.net.test.EmbeddedTestServer;
/** Integration tests for trigger scripts. */
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
@RunWith(ChromeJUnit4ClassRunner.class)
public class AutofillAssistantTriggerScriptIntegrationTest {
@Rule
public ChromeTabbedActivityTestRule mTestRule = new ChromeTabbedActivityTestRule();
private static final String HTML_DIRECTORY = "/components/test/data/autofill_assistant/html/";
private static final String TEST_PAGE = "autofill_assistant_target_website.html";
private EmbeddedTestServer mTestServer;
private String getURL(String page) {
return mTestServer.getURL(HTML_DIRECTORY + page);
}
private void setupTriggerScripts(GetTriggerScriptsResponseProto triggerScripts) {
AutofillAssistantTestServiceRequestSender testServiceRequestSender =
new AutofillAssistantTestServiceRequestSender();
testServiceRequestSender.setNextResponse(/* httpStatus = */ 200, triggerScripts);
testServiceRequestSender.scheduleForInjection();
;
}
private void startAutofillAssistantOnTab(String pageToLoad) {
TestThreadUtils.runOnUiThreadBlocking(
()
-> AutofillAssistantFacade.start(mTestRule.getActivity(),
AutofillAssistantArguments.newBuilder()
.fromBundle(null)
.withInitialUrl(getURL(pageToLoad))
.addParameter("REQUEST_TRIGGER_SCRIPT", true)
.build()));
}
@Before
public void setUp() {
mTestServer = EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext());
AutofillAssistantPreferencesUtil.setInitialPreferences(true);
mTestRule.startMainActivityWithURL(getURL(TEST_PAGE));
// Enable MSBB.
TestThreadUtils.runOnUiThreadBlocking(
()
-> UnifiedConsentServiceBridge.setUrlKeyedAnonymizedDataCollectionEnabled(
AutofillAssistantUiController.getProfile(), true));
}
@After
public void tearDown() throws Exception {
mTestServer.stopAndDestroyServer();
}
@Test
@MediumTest
@Features.EnableFeatures(ChromeFeatureList.AUTOFILL_ASSISTANT_PROACTIVE_HELP)
public void triggerScriptSmokeTest() {
GetTriggerScriptsResponseProto triggerScripts =
(GetTriggerScriptsResponseProto) GetTriggerScriptsResponseProto.newBuilder()
.addTriggerScripts(
TriggerScriptProto.newBuilder()
.setTriggerCondition(
TriggerScriptConditionProto.newBuilder()
.setIsFirstTimeUser(Empty.newBuilder()))
.setUserInterface(
TriggerScriptUIProto.newBuilder().setStatusMessage(
"Hello World!")))
.build();
setupTriggerScripts(triggerScripts);
startAutofillAssistantOnTab(TEST_PAGE);
waitUntilViewMatchesCondition(withText("Hello World!"), isCompletelyDisplayed());
}
}
......@@ -60,7 +60,7 @@ public class AutofillAssistantArguments {
return this;
}
public Builder addParameter(String key, String value) {
public Builder addParameter(String key, Object value) {
mArguments.mAutofillAssistantParameters.put(key, value);
return this;
}
......@@ -247,6 +247,12 @@ public class AutofillAssistantArguments {
return mInitialUrl;
}
/** Whether the caller requests the client to fetch trigger scripts from a remote endpoint. */
public boolean requestsTriggerScript() {
return getBooleanParameter(PARAMETER_REQUEST_TRIGGER_SCRIPT);
}
/** Deprecated. Whether the caller provides script paths for lite scripts to execute. */
public boolean containsTriggerScript() {
return !TextUtils.isEmpty(getStringParameter(PARAMETER_TRIGGER_FIRST_TIME_USER))
&& !TextUtils.isEmpty(getStringParameter(PARAMETER_TRIGGER_RETURNING_TIME_USER));
......
......@@ -107,7 +107,7 @@ public class AutofillAssistantFacade {
// Have an "attempted starts" baseline for the drop out histogram.
AutofillAssistantMetrics.recordDropOut(DropOutReason.AA_START);
waitForTabWithWebContents(activity, tab -> {
if (arguments.containsTriggerScript()) {
if (arguments.containsTriggerScript() || arguments.requestsTriggerScript()) {
// Create a field trial and assign experiment arm based on script parameter. This
// is needed to tag UKM data to allow for A/B experiment comparisons.
FieldTrialList.createFieldTrial(LITE_SCRIPT_EXPERIMENT_TRIAL,
......
......@@ -178,12 +178,14 @@ void ClientAndroid::StartTriggerScript(
const base::android::JavaParamRef<jstring>& jinitial_url,
const base::android::JavaParamRef<jstring>& jexperiment_ids,
const base::android::JavaParamRef<jobjectArray>& jparameter_names,
const base::android::JavaParamRef<jobjectArray>& jparameter_values) {
const base::android::JavaParamRef<jobjectArray>& jparameter_values,
jlong jservice_request_sender) {
trigger_script_bridge_.StartTriggerScript(
this, jdelegate,
GURL(base::android::ConvertJavaStringToUTF8(env, jinitial_url)),
CreateTriggerContext(env, jexperiment_ids, jparameter_names,
jparameter_values));
jparameter_values),
jservice_request_sender);
}
void ClientAndroid::OnJavaDestroyUI(
......
......@@ -71,7 +71,8 @@ class ClientAndroid : public Client,
const base::android::JavaParamRef<jstring>& jinitial_url,
const base::android::JavaParamRef<jstring>& jexperiment_ids,
const base::android::JavaParamRef<jobjectArray>& jparameter_names,
const base::android::JavaParamRef<jobjectArray>& jparameter_values);
const base::android::JavaParamRef<jobjectArray>& jparameter_values,
jlong jservice_request_sender);
base::android::ScopedJavaLocalRef<jstring> GetPrimaryAccountName(
JNIEnv* env,
......
......@@ -35,24 +35,37 @@ void TriggerScriptBridgeAndroid::StartTriggerScript(
Client* client,
const JavaParamRef<jobject>& jdelegate,
const GURL& initial_url,
std::unique_ptr<TriggerContext> trigger_context) {
std::unique_ptr<TriggerContext> trigger_context,
jlong jservice_request_sender) {
DCHECK(!java_object_);
java_object_ = ScopedJavaGlobalRef<jobject>(jdelegate);
Java_AssistantTriggerScriptBridge_setNativePtr(
AttachCurrentThread(), java_object_, reinterpret_cast<intptr_t>(this));
std::unique_ptr<ServiceRequestSender> service_request_sender = nullptr;
if (jservice_request_sender) {
service_request_sender.reset(static_cast<ServiceRequestSender*>(
reinterpret_cast<void*>(jservice_request_sender)));
ClientSettingsProto::IntegrationTestSettings test_settings;
test_settings.set_disable_header_animations(true);
test_settings.set_disable_carousel_change_animations(true);
client_settings_.integration_test_settings = test_settings;
} else {
service_request_sender = std::make_unique<ServiceRequestSenderImpl>(
client->GetWebContents()->GetBrowserContext(),
/* access_token_fetcher = */ nullptr,
std::make_unique<NativeURLLoaderFactory>(),
ApiKeyFetcher().GetAPIKey(chrome::GetChannel()),
/* auth_enabled = */ false,
/* disable_auth_if_no_access_token = */ true);
}
ServerUrlFetcher url_fetcher{ServerUrlFetcher::GetDefaultServerUrl()};
trigger_script_coordinator_ = std::make_unique<TriggerScriptCoordinator>(
client,
WebController::CreateForWebContents(client->GetWebContents(),
&client_settings_),
std::make_unique<ServiceRequestSenderImpl>(
client->GetWebContents()->GetBrowserContext(),
/* access_token_fetcher = */ nullptr,
std::make_unique<NativeURLLoaderFactory>(),
ApiKeyFetcher().GetAPIKey(chrome::GetChannel()),
/* auth_enabled = */ false,
/* disable_auth_if_no_access_token = */ true),
std::move(service_request_sender),
url_fetcher.GetTriggerScriptsEndpoint(),
std::make_unique<StaticTriggerConditions>(),
std::make_unique<DynamicTriggerConditions>(), ukm::UkmRecorder::Get());
......
......@@ -33,7 +33,8 @@ class TriggerScriptBridgeAndroid : public TriggerScriptCoordinator::Observer {
void StartTriggerScript(Client* client,
const base::android::JavaParamRef<jobject>& jdelegate,
const GURL& initial_url,
std::unique_ptr<TriggerContext> trigger_context);
std::unique_ptr<TriggerContext> trigger_context,
jlong jservice_request_sender);
// Stops and destroys the current trigger script, if any. Also disconnects the
// java-side delegate.
......
......@@ -440,6 +440,8 @@ if (is_android) {
sources = [
"service/java_service.cc",
"service/java_service.h",
"service/java_service_request_sender.cc",
"service/java_service_request_sender.h",
]
deps = [
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "components/autofill_assistant/browser/service/java_service_request_sender.h"
#include "base/android/jni_android.h"
#include "base/android/jni_string.h"
#include "chrome/android/features/autofill_assistant/test_support_jni_headers/AutofillAssistantTestServiceRequestSender_jni.h"
namespace autofill_assistant {
static jlong JNI_AutofillAssistantTestServiceRequestSender_CreateNative(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& java_service_request_sender) {
return reinterpret_cast<jlong>(
new JavaServiceRequestSender(java_service_request_sender));
}
JavaServiceRequestSender::JavaServiceRequestSender(
const base::android::JavaParamRef<jobject>& jservice_request_sender)
: jservice_request_sender_(jservice_request_sender) {}
JavaServiceRequestSender::~JavaServiceRequestSender() = default;
void JavaServiceRequestSender::SendRequest(const GURL& url,
const std::string& request_body,
ResponseCallback callback) {
DCHECK(!callback_)
<< __func__
<< " invoked while still waiting for response to previous request";
callback_ = std::move(callback);
JNIEnv* env = base::android::AttachCurrentThread();
Java_AutofillAssistantTestServiceRequestSender_sendRequest(
env, jservice_request_sender_,
base::android::ConvertUTF8ToJavaString(env, url.spec()),
base::android::ToJavaByteArray(env, request_body));
}
void JavaServiceRequestSender::OnResponse(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
jint http_status,
const base::android::JavaParamRef<jbyteArray>& jresponse) {
DCHECK(callback_);
std::string response;
if (jresponse) {
base::android::JavaByteArrayToString(env, jresponse, &response);
}
std::move(callback_).Run(http_status, response);
}
} // namespace autofill_assistant
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SERVICE_JAVA_SERVICE_REQUEST_SENDER_H_
#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SERVICE_JAVA_SERVICE_REQUEST_SENDER_H_
#include <memory>
#include <string>
#include <vector>
#include "base/android/jni_array.h"
#include "base/android/scoped_java_ref.h"
#include "components/autofill_assistant/browser/service/service_request_sender.h"
#include "url/gurl.h"
namespace autofill_assistant {
// TODO(arbesser): Move this to chrome/browser/android, it does not belong into
// components/autofill_assistant.
//
// Thin C++ wrapper around a service request sender implemented in Java.
// Intended for use in integration tests to inject as a mock backend.
class JavaServiceRequestSender : public ServiceRequestSender {
public:
JavaServiceRequestSender(
const base::android::JavaParamRef<jobject>& jservice_request_sender);
~JavaServiceRequestSender() override;
JavaServiceRequestSender(const JavaServiceRequestSender&) = delete;
JavaServiceRequestSender& operator=(const JavaServiceRequestSender&) = delete;
void SendRequest(const GURL& url,
const std::string& request_body,
ResponseCallback callback) override;
void OnResponse(JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
jint http_status,
const base::android::JavaParamRef<jbyteArray>& jresponse);
private:
ResponseCallback callback_;
base::android::ScopedJavaGlobalRef<jobject> jservice_request_sender_;
};
} // namespace autofill_assistant
#endif // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SERVICE_JAVA_SERVICE_REQUEST_SENDER_H_
......@@ -266,7 +266,6 @@ void TriggerScriptCoordinator::ShowTriggerScript(int index) {
ukm_recorder_, client_->GetWebContents(),
Metrics::LiteScriptShownToUser::LITE_SCRIPT_SHOWN_TO_USER);
visible_trigger_script_ = index;
static_trigger_conditions_->set_is_first_time_user(false);
auto proto = trigger_scripts_[index]->AsProto().user_interface();
for (Observer& observer : observers_) {
observer.OnTriggerScriptShown(proto);
......@@ -278,6 +277,7 @@ void TriggerScriptCoordinator::HideTriggerScript() {
return;
}
static_trigger_conditions_->set_is_first_time_user(false);
visible_trigger_script_ = -1;
for (Observer& observer : observers_) {
observer.OnTriggerScriptHidden();
......
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