Commit 89bf26b0 authored by Stephane Zermatten's avatar Stephane Zermatten Committed by Commit Bot

[Autofill Assistant] Add parameter precondition.

This change reads the original intent, looks for parameters that are
meant for Autofill Assistant and allow scripts to check these in
preconditions.

Bug: 806868
Change-Id: I8bd6b51cf281d1f21ce2ccf6128b7345290cab0a
Reviewed-on: https://chromium-review.googlesource.com/1225696
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: default avatarGanggui Tang <gogerald@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592780}
parent 159ea0fa
......@@ -4,6 +4,8 @@
package org.chromium.chrome.browser.autofill_assistant;
import android.os.Bundle;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace;
import org.chromium.chrome.browser.customtabs.CustomTabActivity;
......@@ -15,7 +17,9 @@ import org.chromium.chrome.browser.tabmodel.TabModel.TabSelectionType;
import org.chromium.content_public.browser.WebContents;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
/**
* Bridge to native side autofill_assistant::UiControllerAndroid. It allows native side to control
......@@ -23,6 +27,10 @@ import java.util.List;
*/
@JNINamespace("autofill_assistant")
public class AutofillAssistantUiController implements BottomBarController.Client {
/** Prefix for Intent extras relevant to this feature. */
private static final String INTENT_EXTRA_PREFIX =
"org.chromium.chrome.browser.autofill_assistant.";
private final long mUiControllerAndroid;
private final BottomBarController mBottomBarController;
......@@ -34,7 +42,15 @@ public class AutofillAssistantUiController implements BottomBarController.Client
public AutofillAssistantUiController(CustomTabActivity activity) {
// TODO(crbug.com/806868): Implement corresponding UI.
Tab activityTab = activity.getActivityTab();
mUiControllerAndroid = nativeInit(activityTab.getWebContents());
Map<String, String> parameters = extractParameters(activity.getInitialIntent().getExtras());
// TODO(crbug.com/806868): Treat parameter
// org.chromium.chrome.browser.autofill_assistant.ENABLED specially, and disable autofill
// assistant if it is false or unset.
mUiControllerAndroid = nativeInit(activityTab.getWebContents(),
parameters.keySet().toArray(new String[parameters.size()]),
parameters.values().toArray(new String[parameters.size()]));
// Stop Autofill Assistant when the tab is detached from the activity.
activityTab.addObserver(new EmptyTabObserver() {
......@@ -69,6 +85,17 @@ public class AutofillAssistantUiController implements BottomBarController.Client
nativeOnScriptSelected(mUiControllerAndroid, scriptPath);
}
/** Returns a map containing the extras starting with {@link #INTENT_EXTRA_PREFIX}. */
private static Map<String, String> extractParameters(Bundle extras) {
Map<String, String> result = new HashMap<>();
for (String key : extras.keySet()) {
if (key.startsWith(INTENT_EXTRA_PREFIX)) {
result.put(key.substring(INTENT_EXTRA_PREFIX.length()), extras.get(key).toString());
}
}
return result;
}
@CalledByNative
private void onShowStatusMessage(String message) {
mBottomBarController.showStatusMessage(message);
......@@ -95,7 +122,8 @@ public class AutofillAssistantUiController implements BottomBarController.Client
}
// native methods.
private native long nativeInit(WebContents webContents);
private native long nativeInit(
WebContents webContents, String[] parameterNames, String[] parameterValues);
private native void nativeDestroy(long nativeUiControllerAndroid);
private native void nativeOnScriptSelected(long nativeUiControllerAndroid, String scriptPath);
}
......@@ -4,6 +4,10 @@
#include "chrome/browser/android/autofill_assistant/ui_controller_android.h"
#include <map>
#include <memory>
#include <utility>
#include "base/android/jni_android.h"
#include "base/android/jni_array.h"
#include "base/android/jni_string.h"
......@@ -23,18 +27,40 @@ namespace switches {
const char* const kAutofillAssistantServerKey = "autofill-assistant-key";
} // namespace switches
namespace {
// Builds a map from two Java arrays of strings with the same length.
std::unique_ptr<std::map<std::string, std::string>>
BuildParametersFromJava(JNIEnv* env, jobjectArray names, jobjectArray values) {
std::vector<std::string> names_vector;
base::android::AppendJavaStringArrayToStringVector(env, names, &names_vector);
std::vector<std::string> values_vector;
base::android::AppendJavaStringArrayToStringVector(env, values,
&values_vector);
DCHECK_EQ(names_vector.size(), values_vector.size());
auto parameters = std::make_unique<std::map<std::string, std::string>>();
for (size_t i = 0; i < names_vector.size(); ++i) {
parameters->insert(std::make_pair(names_vector[i], values_vector[i]));
}
return parameters;
}
} // namespace
UiControllerAndroid::UiControllerAndroid(
JNIEnv* env,
jobject jcaller,
const JavaParamRef<jobject>& webContents)
const JavaParamRef<jobject>& webContents,
const base::android::JavaParamRef<jobjectArray>& parameterNames,
const base::android::JavaParamRef<jobjectArray>& parameterValues)
: ui_delegate_(nullptr) {
java_autofill_assistant_ui_controller_.Reset(env, jcaller);
content::WebContents* web_contents =
content::WebContents::FromJavaWebContents(webContents);
DCHECK(web_contents);
Controller::CreateAndStartForWebContents(web_contents,
base::WrapUnique(this));
Controller::CreateAndStartForWebContents(
web_contents, base::WrapUnique(this),
BuildParametersFromJava(env, parameterNames, parameterValues));
DCHECK(ui_delegate_);
}
......@@ -124,9 +150,11 @@ void UiControllerAndroid::Destroy(JNIEnv* env,
static jlong JNI_AutofillAssistantUiController_Init(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
const base::android::JavaParamRef<jobject>& webContents) {
auto* ui_controller_android =
new autofill_assistant::UiControllerAndroid(env, jcaller, webContents);
const base::android::JavaParamRef<jobject>& webContents,
const base::android::JavaParamRef<jobjectArray>& parameterNames,
const base::android::JavaParamRef<jobjectArray>& parameterValues) {
auto* ui_controller_android = new autofill_assistant::UiControllerAndroid(
env, jcaller, webContents, parameterNames, parameterValues);
return reinterpret_cast<intptr_t>(ui_controller_android);
}
......
......@@ -6,6 +6,7 @@
#define CHROME_BROWSER_ANDROID_AUTOFILL_ASSISTANT_UI_CONTROLLER_ANDROID_H_
#include <string>
#include <vector>
#include "base/android/scoped_java_ref.h"
#include "base/macros.h"
......@@ -16,9 +17,12 @@ namespace autofill_assistant {
// Class implements UiController, Client and starts the Controller.
class UiControllerAndroid : public UiController, public Client {
public:
UiControllerAndroid(JNIEnv* env,
UiControllerAndroid(
JNIEnv* env,
jobject jcaller,
const base::android::JavaParamRef<jobject>& webContents);
const base::android::JavaParamRef<jobject>& webContents,
const base::android::JavaParamRef<jobjectArray>& parameterNames,
const base::android::JavaParamRef<jobjectArray>& parameterValues);
~UiControllerAndroid() override;
// Overrides UiController:
......
......@@ -15,13 +15,15 @@ namespace autofill_assistant {
// static
void Controller::CreateAndStartForWebContents(
content::WebContents* web_contents,
std::unique_ptr<Client> client) {
std::unique_ptr<Client> client,
std::unique_ptr<std::map<std::string, std::string>> parameters) {
// Get the key early since |client| will be invalidated when moved below.
const std::string api_key = client->GetApiKey();
new Controller(
web_contents, std::move(client),
WebController::CreateForWebContents(web_contents),
std::make_unique<Service>(api_key, web_contents->GetBrowserContext()));
std::make_unique<Service>(api_key, web_contents->GetBrowserContext()),
std::move(parameters));
}
Service* Controller::GetService() {
......@@ -40,15 +42,18 @@ ClientMemory* Controller::GetClientMemory() {
return memory_.get();
}
Controller::Controller(content::WebContents* web_contents,
Controller::Controller(
content::WebContents* web_contents,
std::unique_ptr<Client> client,
std::unique_ptr<WebController> web_controller,
std::unique_ptr<Service> service)
std::unique_ptr<Service> service,
std::unique_ptr<std::map<std::string, std::string>> parameters)
: content::WebContentsObserver(web_contents),
client_(std::move(client)),
web_controller_(std::move(web_controller)),
service_(std::move(service)),
script_tracker_(std::make_unique<ScriptTracker>(this, this)),
script_tracker_(
std::make_unique<ScriptTracker>(this, this, std::move(parameters))),
memory_(std::make_unique<ClientMemory>()),
allow_autostart_(true) {
GetUiController()->SetUiDelegate(this);
......
......@@ -5,6 +5,7 @@
#ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_CONTROLLER_H_
#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_CONTROLLER_H_
#include <map>
#include <memory>
#include <string>
#include <vector>
......@@ -36,8 +37,10 @@ class Controller : public ScriptExecutorDelegate,
public ScriptTracker::Listener,
private content::WebContentsObserver {
public:
static void CreateAndStartForWebContents(content::WebContents* web_contents,
std::unique_ptr<Client> client);
static void CreateAndStartForWebContents(
content::WebContents* web_contents,
std::unique_ptr<Client> client,
std::unique_ptr<std::map<std::string, std::string>> parameters);
// Overrides ScriptExecutorDelegate:
Service* GetService() override;
......@@ -51,7 +54,8 @@ class Controller : public ScriptExecutorDelegate,
Controller(content::WebContents* web_contents,
std::unique_ptr<Client> client,
std::unique_ptr<WebController> web_controller,
std::unique_ptr<Service> service);
std::unique_ptr<Service> service,
std::unique_ptr<std::map<std::string, std::string>> parameters);
~Controller() override;
void GetOrCheckScripts(const GURL& url);
......
......@@ -62,7 +62,8 @@ class ControllerTest : public content::RenderViewHostTestHarness {
mock_service_ = service.get();
controller_ = new Controller(
web_contents(), std::make_unique<FakeClient>(std::move(ui_controller)),
std::move(web_controller), std::move(service));
std::move(web_controller), std::move(service),
std::make_unique<std::map<std::string, std::string>>());
// Fetching scripts succeeds for all URLs, but return nothing.
ON_CALL(*mock_service_, OnGetScriptsForUrl(_, _))
......
......@@ -35,25 +35,31 @@ std::unique_ptr<ScriptPrecondition> ScriptPrecondition::FromProto(
for (const auto& pattern : proto.path_pattern()) {
auto re = std::make_unique<re2::RE2>(pattern);
if (re->error_code() != re2::RE2::NoError) {
LOG(ERROR) << "Invalid regexp in script precondition '" << pattern
<< "'. Will never match.";
LOG(ERROR) << "Invalid regexp in script precondition '" << pattern;
return nullptr;
}
path_pattern.emplace_back(std::move(re));
}
std::vector<ScriptParameterMatchProto> parameter_match;
for (const auto& match : proto.script_parameter_match()) {
parameter_match.emplace_back(match);
}
// TODO(crbug.com/806868): Detect unknown or unsupported conditions and reject
// them.
return std::make_unique<ScriptPrecondition>(elements_exist, domain_match,
std::move(path_pattern));
return std::make_unique<ScriptPrecondition>(
elements_exist, domain_match, std::move(path_pattern), parameter_match);
}
ScriptPrecondition::~ScriptPrecondition() {}
void ScriptPrecondition::Check(WebController* web_controller,
void ScriptPrecondition::Check(
WebController* web_controller,
const std::map<std::string, std::string>& parameters,
base::OnceCallback<void(bool)> callback) {
const GURL& url = web_controller->GetUrl();
if (!MatchDomain(url) || !MatchPath(url)) {
if (!MatchDomain(url) || !MatchPath(url) || !MatchParameters(parameters)) {
std::move(callback).Run(false);
return;
}
......@@ -76,10 +82,12 @@ void ScriptPrecondition::Check(WebController* web_controller,
ScriptPrecondition::ScriptPrecondition(
const std::vector<std::vector<std::string>>& elements_exist,
const std::set<std::string>& domain_match,
std::vector<std::unique_ptr<re2::RE2>> path_pattern)
std::vector<std::unique_ptr<re2::RE2>> path_pattern,
const std::vector<ScriptParameterMatchProto>& parameter_match)
: elements_exist_(elements_exist),
domain_match_(domain_match),
path_pattern_(std::move(path_pattern)),
parameter_match_(parameter_match),
weak_ptr_factory_(this) {}
void ScriptPrecondition::OnCheckElementExists(bool result) {
......@@ -117,4 +125,24 @@ bool ScriptPrecondition::MatchPath(const GURL& url) const {
return false;
}
bool ScriptPrecondition::MatchParameters(
const std::map<std::string, std::string>& parameters) const {
for (const auto& match : parameter_match_) {
auto iter = parameters.find(match.name());
if (match.exists()) {
// parameter must exist and optionally have a specific value
if (iter == parameters.end())
return false;
if (!match.value_equals().empty() && iter->second != match.value_equals())
return false;
} else {
// parameter must not exist
if (iter != parameters.end())
return false;
}
}
return true;
}
} // namespace autofill_assistant.
......@@ -5,6 +5,7 @@
#ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SCRIPT_PRECONDITION_H_
#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_SCRIPT_PRECONDITION_H_
#include <map>
#include <memory>
#include <set>
#include <string>
......@@ -13,6 +14,7 @@
#include "base/callback.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "components/autofill_assistant/browser/service.pb.h"
#include "components/autofill_assistant/browser/web_controller.h"
namespace re2 {
......@@ -20,7 +22,6 @@ class RE2;
} // namespace re2
namespace autofill_assistant {
class ScriptPreconditionProto;
// Class represents a set of preconditions for a script to be executed.
class ScriptPrecondition {
......@@ -33,18 +34,22 @@ class ScriptPrecondition {
ScriptPrecondition(
const std::vector<std::vector<std::string>>& elements_exist,
const std::set<std::string>& domain_match,
std::vector<std::unique_ptr<re2::RE2>> path_pattern);
std::vector<std::unique_ptr<re2::RE2>> path_pattern,
const std::vector<ScriptParameterMatchProto>& parameter_match);
~ScriptPrecondition();
// Check whether the conditions satisfied and return the result through
// |callback|.
void Check(WebController* web_controller,
const std::map<std::string, std::string>& parameters,
base::OnceCallback<void(bool)> callback);
private:
void OnCheckElementExists(bool result);
bool MatchDomain(const GURL& url) const;
bool MatchPath(const GURL& url) const;
bool MatchParameters(
const std::map<std::string, std::string>& parameters) const;
std::vector<std::vector<std::string>> elements_exist_;
base::OnceCallback<void(bool)> check_preconditions_callback_;
......@@ -56,6 +61,9 @@ class ScriptPrecondition {
// Pattern of the path parts of the URL.
std::vector<std::unique_ptr<re2::RE2>> path_pattern_;
// Condition on parameters, identified by name, as found in the intent.
std::vector<ScriptParameterMatchProto> parameter_match_;
base::WeakPtrFactory<ScriptPrecondition> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(ScriptPrecondition);
......
......@@ -76,12 +76,13 @@ class ScriptPreconditionTest : public testing::Test {
return false;
DirectCallback callback;
precondition->Check(&mock_web_controller_, callback.Get());
precondition->Check(&mock_web_controller_, parameters_, callback.Get());
return callback.GetResultOrDie();
}
GURL url_;
MockWebController mock_web_controller_;
std::map<std::string, std::string> parameters_;
};
TEST_F(ScriptPreconditionTest, NoConditions) {
......@@ -136,6 +137,48 @@ TEST_F(ScriptPreconditionTest, BadPathPattern) {
EXPECT_EQ(nullptr, ScriptPrecondition::FromProto(proto));
}
TEST_F(ScriptPreconditionTest, ParameterMustExist) {
ScriptPreconditionProto proto;
ScriptParameterMatchProto* match = proto.add_script_parameter_match();
match->set_name("param");
match->set_exists(true);
EXPECT_FALSE(Check(proto));
parameters_["param"] = "exists";
EXPECT_TRUE(Check(proto));
}
TEST_F(ScriptPreconditionTest, ParameterMustNotExist) {
ScriptPreconditionProto proto;
ScriptParameterMatchProto* match = proto.add_script_parameter_match();
match->set_name("param");
match->set_exists(false);
EXPECT_TRUE(Check(proto));
parameters_["param"] = "exists";
EXPECT_FALSE(Check(proto));
}
TEST_F(ScriptPreconditionTest, ParameterMustHaveValue) {
ScriptPreconditionProto proto;
ScriptParameterMatchProto* match = proto.add_script_parameter_match();
match->set_name("param");
match->set_value_equals("value");
EXPECT_FALSE(Check(proto));
parameters_["param"] = "another value";
EXPECT_FALSE(Check(proto));
parameters_["param"] = "value";
EXPECT_TRUE(Check(proto));
}
TEST_F(ScriptPreconditionTest, MultipleConditions) {
ScriptPreconditionProto proto;
proto.add_domain("match.example.com");
......
......@@ -13,14 +13,18 @@
namespace autofill_assistant {
ScriptTracker::ScriptTracker(ScriptExecutorDelegate* delegate,
ScriptTracker::Listener* listener)
ScriptTracker::ScriptTracker(
ScriptExecutorDelegate* delegate,
ScriptTracker::Listener* listener,
std::unique_ptr<std::map<std::string, std::string>> parameters)
: delegate_(delegate),
listener_(listener),
parameters_(std::move(parameters)),
pending_precondition_check_count_(0),
weak_ptr_factory_(this) {
DCHECK(delegate_);
DCHECK(listener_);
DCHECK(parameters_);
}
ScriptTracker::~ScriptTracker() = default;
......@@ -59,7 +63,7 @@ void ScriptTracker::CheckScripts() {
} else {
for (Script* script : scripts_to_check) {
script->precondition->Check(
delegate_->GetWebController(),
delegate_->GetWebController(), *parameters_.get(),
base::BindOnce(&ScriptTracker::OnPreconditionCheck,
weak_ptr_factory_.GetWeakPtr(), script));
}
......
......@@ -40,7 +40,8 @@ class ScriptTracker {
// |delegate| and |listener| should outlive this object and should not be
// nullptr.
ScriptTracker(ScriptExecutorDelegate* delegate,
ScriptTracker::Listener* listener);
ScriptTracker::Listener* listener,
std::unique_ptr<std::map<std::string, std::string>> parameters);
~ScriptTracker();
......@@ -76,6 +77,7 @@ class ScriptTracker {
ScriptExecutorDelegate* const delegate_;
ScriptTracker::Listener* const listener_;
std::unique_ptr<std::map<std::string, std::string>> parameters_;
// Paths and names of scripts known to be runnable.
//
......
......@@ -45,7 +45,11 @@ class ScriptTrackerTest : public testing::Test,
}
protected:
ScriptTrackerTest() : runnable_scripts_changed_(0), tracker_(this, this) {}
ScriptTrackerTest()
: runnable_scripts_changed_(0),
tracker_(this,
this,
std::make_unique<std::map<std::string, std::string>>()) {}
// Overrides ScriptTrackerDelegate
Service* GetService() override { return &mock_service_; }
......
......@@ -57,6 +57,20 @@ message ScriptPreconditionProto {
repeated string path_pattern = 5;
// Domain (exact match) excluding the last '/' character.
repeated string domain = 6;
// Combined with AND: all matches must be true for precondition to hold.
repeated ScriptParameterMatchProto script_parameter_match = 7;
}
message ScriptParameterMatchProto {
// Parameter name, as found in the Intent, without prefix.
optional string name = 4;
// Checks whether the script parameter is present.
optional bool exists = 2 [default = true];
// Checks whether the script parameter has exact value. Empty or missing value
// is treated as wildcard - any value will pass.
optional string value_equals = 3;
}
enum PolicyType {
......
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