Forward Bundle version and path from GetScriptsForUrl to GetActions.

Read the bundle path and version from the latest GetScriptsForUrl call,
and store it in the service's state. Forward them in the Initial
request's ScriptStoreConfig.

Change-Id: I80aef768ea893146af42f6671a03379356bdcf67
Bug: b/174227808
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2582118
Commit-Queue: Ernest Galbrun <galbrun@google.com>
Reviewed-by: Clemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#837998}

components/autofill_assistant/browser/controller.cc

@@ -925,7 +925,9 @@ void Controller::OnGetScripts(const GURL& url,
       observer.OnClientSettingsChanged(settings_);
     }
   }
-
+  if (response_proto.has_script_store_config()) {
+    GetService()->SetScriptStoreConfig(response_proto.script_store_config());
+  }
   std::vector<std::unique_ptr<Script>> scripts;
   for (const auto& script_proto : response_proto.scripts()) {
     ProtocolUtils::AddScript(script_proto, &scripts);

components/autofill_assistant/browser/controller_unittest.cc

@@ -1167,6 +1167,26 @@ TEST_F(ControllerTest, WaitForNavigationActionStartWithinTimeout) {
   EXPECT_EQ(ACTION_APPLIED, processed_actions_capture[1].status());
 }
 
+TEST_F(ControllerTest, SetScriptStoreConfig) {
+  // A single script, and its corresponding bundle info.
+  SupportsScriptResponseProto script_response;
+  AddRunnableScript(&script_response, "script");
+  script_response.mutable_script_store_config()->set_bundle_path("bundle/path");
+  script_response.mutable_script_store_config()->set_bundle_version(12);
+  SetupScripts(script_response);
+
+  ScriptStoreConfig script_store_config;
+  std::vector<ProcessedActionProto> processed_actions_capture;
+  EXPECT_CALL(*mock_service_, SetScriptStoreConfig(_))
+      .WillOnce(SaveArg<0>(&script_store_config));
+
+  Start("http://a.example.com/path");
+  controller_->GetUserActions();
+
+  EXPECT_THAT(script_store_config.bundle_path(), Eq("bundle/path"));
+  EXPECT_THAT(script_store_config.bundle_version(), Eq(12));
+}
+
 TEST_F(ControllerTest, InitialDataUrlDoesNotChange) {
   const std::string deeplink_url("http://initialurl.com/path");
   Start(deeplink_url);

components/autofill_assistant/browser/protocol_utils.cc

@@ -123,10 +123,15 @@ std::string ProtocolUtils::CreateInitialScriptActionsRequest(
     const std::string& global_payload,
     const std::string& script_payload,
     const ClientContextProto& client_context,
-    const std::map<std::string, std::string>& script_parameters) {
+    const std::map<std::string, std::string>& script_parameters,
+    const base::Optional<ScriptStoreConfig>& script_store_config) {
   ScriptActionRequestProto request_proto;
   InitialScriptActionsRequestProto* initial_request_proto =
       request_proto.mutable_initial_request();
+  if (script_store_config.has_value()) {
+    *initial_request_proto->mutable_script_store_config() =
+        *script_store_config;
+  }
   InitialScriptActionsRequestProto::QueryProto* query =
       initial_request_proto->mutable_query();
   query->add_script_path(script_path);

components/autofill_assistant/browser/protocol_utils.h

@@ -45,7 +45,8 @@ class ProtocolUtils {
       const std::string& global_payload,
       const std::string& script_payload,
       const ClientContextProto& client_context,
-      const std::map<std::string, std::string>& script_parameters);
+      const std::map<std::string, std::string>& script_parameters,
+      const base::Optional<ScriptStoreConfig>& script_store_config);
 
   // Create request to get next sequence of actions for a script.
   static std::string CreateNextScriptActionsRequest(

components/autofill_assistant/browser/protocol_utils_unittest.cc

@@ -126,10 +126,14 @@ TEST_F(ProtocolUtilsTest, CreateInitialScriptActionsRequest) {
   std::map<std::string, std::string> parameters = {{"key_a", "value_a"},
                                                    {"key_b", "value_b"}};
   ScriptActionRequestProto request;
+  ScriptStoreConfig config;
+  config.set_bundle_path("bundle/path");
+  config.set_bundle_version(12);
   EXPECT_TRUE(
       request.ParseFromString(ProtocolUtils::CreateInitialScriptActionsRequest(
           "script_path", GURL("http://example.com/"), "global_payload",
-          "script_payload", client_context_proto_, parameters)));
+          "script_payload", client_context_proto_, parameters,
+          base::Optional<ScriptStoreConfig>(config))));
 
   const InitialScriptActionsRequestProto& initial = request.initial_request();
   EXPECT_THAT(initial.query().script_path(), ElementsAre("script_path"));
@@ -139,6 +143,8 @@ TEST_F(ProtocolUtilsTest, CreateInitialScriptActionsRequest) {
   AssertClientContext(request.client_context());
   EXPECT_EQ("global_payload", request.global_payload());
   EXPECT_EQ("script_payload", request.script_payload());
+  EXPECT_EQ("bundle/path", initial.script_store_config().bundle_path());
+  EXPECT_EQ(12, initial.script_store_config().bundle_version());
 }
 
 TEST_F(ProtocolUtilsTest, CreateNextScriptActionsRequest) {

components/autofill_assistant/browser/service.proto

@@ -123,6 +123,11 @@ message SupportsScriptResponseProto {
   optional ScriptTimeoutError script_timeout_error = 2;
 
   optional ClientSettingsProto client_settings = 3;
+
+  // The ScriptStoreConfig to use with the initial GetActions request.
+  optional ScriptStoreConfig script_store_config = 6;
+
+  reserved 4, 5;
 }
 
 // Overlay image to be drawn on top of full overlays.
@@ -375,6 +380,13 @@ message ScriptActionRequestProto {
   }
 }
 
+// Message specifying how to retrieve the scripts. If bundle_path is set, it
+// will read that bundle with the associated version.
+message ScriptStoreConfig {
+  optional string bundle_path = 2;
+  optional int64 bundle_version = 3;
+}
+
 // Initial request to get a script's actions.
 message InitialScriptActionsRequestProto {
   message QueryProto {
@@ -388,6 +400,9 @@ message InitialScriptActionsRequestProto {
   optional QueryProto query = 3;
 
   repeated ScriptParameterProto script_parameters = 2;
+
+  // Specify a bundle and version to run.
+  optional ScriptStoreConfig script_store_config = 6;
 }
 
 message RoundtripTimingStats {

components/autofill_assistant/browser/service/lite_service.cc

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 #include "components/autofill_assistant/browser/service/lite_service.h"
-#include "components/autofill_assistant/browser/service/lite_service_util.h"
 
 #include <algorithm>
 #include <string>
@@ -13,6 +12,7 @@
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "components/autofill_assistant/browser/protocol_utils.h"
+#include "components/autofill_assistant/browser/service/lite_service_util.h"
 #include "net/http/http_status_code.h"
 
 namespace autofill_assistant {
@@ -78,7 +78,7 @@ void LiteService::GetActions(const std::string& script_path,
       ProtocolUtils::CreateInitialScriptActionsRequest(
           trigger_script_path_, GURL(), /* global_payload = */ std::string(),
           /* script_payload = */ std::string(), EmptyClientContext().AsProto(),
-          /* script_parameters = */ {}),
+          /* script_parameters = */ {}, /* bundle_info */ base::nullopt),
       base::BindOnce(&LiteService::OnGetActions, weak_ptr_factory_.GetWeakPtr(),
                      std::move(callback)));
 }

components/autofill_assistant/browser/service/mock_service.h

@@ -70,6 +70,9 @@ class MockService : public ServiceImpl {
                     ResponseCallback& callback));
 
   MOCK_CONST_METHOD0(IsLiteService, bool());
+
+  MOCK_METHOD1(SetScriptStoreConfig,
+               void(const ScriptStoreConfig& script_store_config));
 };
 
 }  // namespace autofill_assistant

components/autofill_assistant/browser/service/service.h

@@ -51,6 +51,9 @@ class Service {
       const RoundtripTimingStats& timing_stats,
       ResponseCallback callback) = 0;
 
+  virtual void SetScriptStoreConfig(
+      const ScriptStoreConfig& script_store_config) {}
+
  protected:
   Service() = default;
 };

components/autofill_assistant/browser/service/service_impl.cc

@@ -61,6 +61,11 @@ ServiceImpl::ServiceImpl(std::unique_ptr<ServiceRequestSender> request_sender,
 
 ServiceImpl::~ServiceImpl() {}
 
+void ServiceImpl::SetScriptStoreConfig(
+    const ScriptStoreConfig& script_store_config) {
+  script_store_config_ = script_store_config;
+}
+
 void ServiceImpl::GetScriptsForUrl(const GURL& url,
                                    const TriggerContext& trigger_context,
                                    ResponseCallback callback) {
@@ -89,7 +94,8 @@ void ServiceImpl::GetActions(const std::string& script_path,
       script_action_server_url_,
       ProtocolUtils::CreateInitialScriptActionsRequest(
           script_path, url, global_payload, script_payload,
-          client_context_->AsProto(), trigger_context.GetParameters()),
+          client_context_->AsProto(), trigger_context.GetParameters(),
+          script_store_config_),
       std::move(callback));
 }
 

components/autofill_assistant/browser/service/service_impl.h

@@ -75,6 +75,9 @@ class ServiceImpl : public Service {
       const RoundtripTimingStats& timing_stats,
       ResponseCallback callback) override;
 
+  void SetScriptStoreConfig(
+      const ScriptStoreConfig& script_store_config) override;
+
  private:
   // The request sender responsible for communicating with a remote endpoint.
   std::unique_ptr<ServiceRequestSender> request_sender_;
@@ -86,6 +89,10 @@ class ServiceImpl : public Service {
   // The client context to send to the backend.
   std::unique_ptr<ClientContext> client_context_;
 
+  // The script store config used for GetActions request. This is set by the
+  // controller, obtained from the GetScriptsForUrl's response.
+  base::Optional<ScriptStoreConfig> script_store_config_;
+
   base::WeakPtrFactory<ServiceImpl> weak_ptr_factory_{this};
 };
 

components/autofill_assistant/browser/service/service_impl_unittest.cc

@@ -4,12 +4,11 @@
 
 #include "components/autofill_assistant/browser/service/service_impl.h"
 
+#include "base/test/gmock_callback_support.h"
+#include "base/test/mock_callback.h"
 #include "components/autofill_assistant/browser/mock_client_context.h"
 #include "components/autofill_assistant/browser/service/mock_service_request_sender.h"
 #include "components/autofill_assistant/browser/service/service.h"
-
-#include "base/test/gmock_callback_support.h"
-#include "base/test/mock_callback.h"
 #include "net/http/http_status_code.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
@@ -17,8 +16,10 @@ namespace autofill_assistant {
 
 using ::base::test::RunOnceCallback;
 using ::testing::_;
+using ::testing::DoAll;
 using ::testing::NiceMock;
 using ::testing::Return;
+using ::testing::SaveArg;
 
 namespace {
 
@@ -80,6 +81,41 @@ TEST_F(ServiceImplTest, GetActions) {
       std::string("fake_script_payload"), mock_response_callback_.Get());
 }
 
+TEST_F(ServiceImplTest, GetActionsForwardsScriptStoreConfig) {
+  EXPECT_CALL(*mock_client_context_, Update).Times(1);
+
+  ScriptActionRequestProto expected_get_actions;
+  ScriptStoreConfig* config = expected_get_actions.mutable_initial_request()
+                                  ->mutable_script_store_config();
+  config->set_bundle_path("bundle/path");
+  config->set_bundle_version(12);
+
+  std::string get_actions_request;
+  EXPECT_CALL(*mock_request_sender_,
+              OnSendRequest(GURL(kActionServerUrl), _, _))
+      .WillOnce(SaveArg<1>(&get_actions_request));
+
+  ScriptStoreConfig set_config;
+  set_config.set_bundle_path("bundle/path");
+  set_config.set_bundle_version(12);
+  service_->SetScriptStoreConfig(set_config);
+
+  TriggerContextImpl trigger_context;
+  service_->GetActions(
+      std::string("fake_script_path"), GURL("https://www.example.com"),
+      trigger_context, std::string("fake_global_payload"),
+      std::string("fake_script_payload"), mock_response_callback_.Get());
+
+  ScriptActionRequestProto get_actions_request_proto;
+  EXPECT_TRUE(get_actions_request_proto.ParseFromString(get_actions_request));
+  EXPECT_EQ("bundle/path", get_actions_request_proto.initial_request()
+                               .script_store_config()
+                               .bundle_path());
+  EXPECT_EQ(12, get_actions_request_proto.initial_request()
+                    .script_store_config()
+                    .bundle_version());
+}
+
 TEST_F(ServiceImplTest, GetNextActions) {
   EXPECT_CALL(*mock_client_context_, Update).Times(1);
   EXPECT_CALL(*mock_request_sender_,