[go: up one dir, main page]

[Autofill Assistant] Report the intent data url.

Before this patch we reported the committed URL to our backend in the
initial request. The committed URL can potentially contain PII and can
therefore not be kept.

This patch changes that and reports the data url from the intent as
passed to chrome by a caller. There is approval for that piece of
information to be logged.

This is the M-76 merge for http://crrev/c/1632252.

(cherry picked from commit e6f8f11b56e7a7f55ce9eeb99042f1dfe409d79d)

Bug: 973025
Bug: b/133732376
Change-Id: Idb98a727d37f2b9e7488f2f84b8fd273c2f4240f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1632252
Commit-Queue: Mathias Carlen <mcarlen@chromium.org>
Reviewed-by: Stephane Zermatten <szermatt@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#665489}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1657830
Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#278}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/components/autofill_assistant/browser/controller.cc b/components/autofill_assistant/browser/controller.cc
index c506856..04146487 100644
--- a/components/autofill_assistant/browser/controller.cc
+++ b/components/autofill_assistant/browser/controller.cc
@@ -70,7 +70,11 @@
   if (!last_committed.is_empty())
     return last_committed;
 
-  return initial_url_;
+  return deeplink_url_;
+}
+
+const GURL& Controller::GetDeeplinkURL() {
+  return deeplink_url_;
 }
 
 Service* Controller::GetService() {
@@ -690,7 +694,7 @@
     return;
   }
   GetWebController()->SetCookie(
-      initial_url_.host(),
+      deeplink_url_.host(),
       base::BindOnce(&Controller::OnSetCookie,
                      // WebController is owned by Controller.
                      base::Unretained(this)));
@@ -706,7 +710,7 @@
   if (allow_autostart_) {
     SetStatusMessage(
         l10n_util::GetStringFUTF8(IDS_AUTOFILL_ASSISTANT_LOADING,
-                                  base::UTF8ToUTF16(initial_url_.host())));
+                                  base::UTF8ToUTF16(deeplink_url_.host())));
     SetProgress(kAutostartInitialProgress);
   }
   GetOrCheckScripts();
@@ -741,7 +745,7 @@
          state_ != AutofillAssistantState::STOPPED;
 }
 
-void Controller::Start(const GURL& initial_url,
+void Controller::Start(const GURL& deeplink_url,
                        std::unique_ptr<TriggerContext> trigger_context) {
   if (state_ != AutofillAssistantState::INACTIVE) {
     NOTREACHED();
@@ -749,7 +753,7 @@
   }
   trigger_context_ = std::move(trigger_context);
   InitFromParameters();
-  initial_url_ = initial_url;
+  deeplink_url_ = deeplink_url;
   EnterState(AutofillAssistantState::STARTING);
   client_->ShowUI();
   if (IsCookieExperimentEnabled()) {
diff --git a/components/autofill_assistant/browser/controller.h b/components/autofill_assistant/browser/controller.h
index ac418a1..7053fada 100644
--- a/components/autofill_assistant/browser/controller.h
+++ b/components/autofill_assistant/browser/controller.h
@@ -59,7 +59,7 @@
   bool NeedsUI() const;
 
   // Called when autofill assistant can start executing scripts.
-  void Start(const GURL& initial_url,
+  void Start(const GURL& deeplink_url,
              std::unique_ptr<TriggerContext> trigger_context);
 
   // Lets the controller know it's about to be deleted. This is normally called
@@ -69,6 +69,7 @@
   // Overrides ScriptExecutorDelegate:
   const ClientSettings& GetSettings() override;
   const GURL& GetCurrentURL() override;
+  const GURL& GetDeeplinkURL() override;
   Service* GetService() override;
   UiController* GetUiController() override;
   WebController* GetWebController() override;
@@ -236,7 +237,9 @@
   AutofillAssistantState state_ = AutofillAssistantState::INACTIVE;
 
   // The URL passed to Start(). Used only as long as there's no committed URL.
-  GURL initial_url_;
+  // Note that this is the deeplink passed by a caller and reported to the
+  // backend in an initial get action request.
+  GURL deeplink_url_;
 
   // Domain of the last URL the controller requested scripts from.
   std::string script_domain_;
diff --git a/components/autofill_assistant/browser/controller_unittest.cc b/components/autofill_assistant/browser/controller_unittest.cc
index c0e36ac..22e7bc0 100644
--- a/components/autofill_assistant/browser/controller_unittest.cc
+++ b/components/autofill_assistant/browser/controller_unittest.cc
@@ -934,4 +934,16 @@
   EXPECT_EQ(ACTION_APPLIED, processed_actions_capture[1].status());
 }
 
+TEST_F(ControllerTest, InitialDataUrlDoesNotChange) {
+  const std::string deeplink_url("http://initialurl.com/path");
+  Start(deeplink_url);
+  EXPECT_THAT(controller_->GetDeeplinkURL(), deeplink_url);
+  EXPECT_THAT(controller_->GetCurrentURL(), deeplink_url);
+
+  const std::string navigate_url("http://navigateurl.com/path");
+  SimulateNavigateToUrl(GURL(navigate_url));
+  EXPECT_THAT(controller_->GetDeeplinkURL().spec(), deeplink_url);
+  EXPECT_THAT(controller_->GetCurrentURL().spec(), navigate_url);
+}
+
 }  // namespace autofill_assistant
diff --git a/components/autofill_assistant/browser/fake_script_executor_delegate.cc b/components/autofill_assistant/browser/fake_script_executor_delegate.cc
index cf6744c..1559ec5 100644
--- a/components/autofill_assistant/browser/fake_script_executor_delegate.cc
+++ b/components/autofill_assistant/browser/fake_script_executor_delegate.cc
@@ -19,6 +19,10 @@
   return current_url_;
 }
 
+const GURL& FakeScriptExecutorDelegate::GetDeeplinkURL() {
+  return current_url_;
+}
+
 Service* FakeScriptExecutorDelegate::GetService() {
   return service_;
 }
diff --git a/components/autofill_assistant/browser/fake_script_executor_delegate.h b/components/autofill_assistant/browser/fake_script_executor_delegate.h
index c02e5883c..671d27d 100644
--- a/components/autofill_assistant/browser/fake_script_executor_delegate.h
+++ b/components/autofill_assistant/browser/fake_script_executor_delegate.h
@@ -27,6 +27,7 @@
 
   const ClientSettings& GetSettings() override;
   const GURL& GetCurrentURL() override;
+  const GURL& GetDeeplinkURL() override;
   Service* GetService() override;
   UiController* GetUiController() override;
   WebController* GetWebController() override;
diff --git a/components/autofill_assistant/browser/script_executor.cc b/components/autofill_assistant/browser/script_executor.cc
index bc248d5..06cee7c 100644
--- a/components/autofill_assistant/browser/script_executor.cc
+++ b/components/autofill_assistant/browser/script_executor.cc
@@ -104,7 +104,7 @@
 
   DVLOG(2) << "GetActions for " << delegate_->GetCurrentURL().host();
   delegate_->GetService()->GetActions(
-      script_path_, delegate_->GetCurrentURL(), delegate_->GetTriggerContext(),
+      script_path_, delegate_->GetDeeplinkURL(), delegate_->GetTriggerContext(),
       last_global_payload_, last_script_payload_,
       base::BindOnce(&ScriptExecutor::OnGetActions,
                      weak_ptr_factory_.GetWeakPtr()));
diff --git a/components/autofill_assistant/browser/script_executor_delegate.h b/components/autofill_assistant/browser/script_executor_delegate.h
index 3a46eea..7da41dd 100644
--- a/components/autofill_assistant/browser/script_executor_delegate.h
+++ b/components/autofill_assistant/browser/script_executor_delegate.h
@@ -45,6 +45,7 @@
 
   virtual const ClientSettings& GetSettings() = 0;
   virtual const GURL& GetCurrentURL() = 0;
+  virtual const GURL& GetDeeplinkURL() = 0;
   virtual Service* GetService() = 0;
   virtual UiController* GetUiController() = 0;
   virtual WebController* GetWebController() = 0;