Performance improvement to make the first chips appear faster.

Previously, the controller would request the scripts only after GetLastCommittedURL() was available, which takes quite a bit of time (> 1 second on the emulator). Now, the URL is passed directly to the constructor of the controller, which allows it to request the scripts immediately. This results in a moderate performance improvement at the cost of an additional string parameter.

Bug: 806868
Change-Id: Ib11024ce8eb4228b1703abfd58a2e52fbad8e1e8
Reviewed-on: https://chromium-review.googlesource.com/c/1297969Reviewed-by: Stephane Zermatten <szermatt@chromium.org>
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#603472}

chrome/android/java/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiController.java

@@ -81,7 +81,8 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
         mWebContents = activityTab.getWebContents();
         mUiControllerAndroid =
                 nativeInit(mWebContents, parameters.keySet().toArray(new String[parameters.size()]),
-                        parameters.values().toArray(new String[parameters.size()]));
+                        parameters.values().toArray(new String[parameters.size()]),
+                        activity.getInitialIntent().getDataString());
 
         // Shut down Autofill Assistant when the tab is detached from the activity.
         activityTab.addObserver(new EmptyTabObserver() {
@@ -337,8 +338,8 @@ public class AutofillAssistantUiController implements AutofillAssistantUiDelegat
     }
 
     // native methods.
-    private native long nativeInit(
-            WebContents webContents, String[] parameterNames, String[] parameterValues);
+    private native long nativeInit(WebContents webContents, String[] parameterNames,
+            String[] parameterValues, String initialUrl);
     private native void nativeDestroy(long nativeUiControllerAndroid);
     private native void nativeOnScriptSelected(long nativeUiControllerAndroid, String scriptPath);
     private native void nativeOnAddressSelected(long nativeUiControllerAndroid, String guid);

chrome/browser/android/autofill_assistant/ui_controller_android.cc

@@ -60,16 +60,20 @@ UiControllerAndroid::UiControllerAndroid(
     jobject jcaller,
     const JavaParamRef<jobject>& webContents,
     const base::android::JavaParamRef<jobjectArray>& parameterNames,
-    const base::android::JavaParamRef<jobjectArray>& parameterValues)
+    const base::android::JavaParamRef<jobjectArray>& parameterValues,
+    const base::android::JavaParamRef<jstring>& initialUrlString)
     : ui_delegate_(nullptr) {
   java_autofill_assistant_ui_controller_.Reset(env, jcaller);
 
   content::WebContents* web_contents =
       content::WebContents::FromJavaWebContents(webContents);
   DCHECK(web_contents);
+  GURL initialUrl =
+      GURL(base::android::ConvertJavaStringToUTF8(env, initialUrlString));
   Controller::CreateAndStartForWebContents(
       web_contents, base::WrapUnique(this),
-      BuildParametersFromJava(env, parameterNames, parameterValues));
+      BuildParametersFromJava(env, parameterNames, parameterValues),
+      initialUrl);
   DCHECK(ui_delegate_);
 }
 
@@ -326,9 +330,11 @@ static jlong JNI_AutofillAssistantUiController_Init(
     const base::android::JavaParamRef<jobject>& jcaller,
     const base::android::JavaParamRef<jobject>& webContents,
     const base::android::JavaParamRef<jobjectArray>& parameterNames,
-    const base::android::JavaParamRef<jobjectArray>& parameterValues) {
+    const base::android::JavaParamRef<jobjectArray>& parameterValues,
+    const base::android::JavaParamRef<jstring>& initialUrlString) {
   auto* ui_controller_android = new autofill_assistant::UiControllerAndroid(
-      env, jcaller, webContents, parameterNames, parameterValues);
+      env, jcaller, webContents, parameterNames, parameterValues,
+      initialUrlString);
   return reinterpret_cast<intptr_t>(ui_controller_android);
 }
 

chrome/browser/android/autofill_assistant/ui_controller_android.h

@@ -22,7 +22,8 @@ class UiControllerAndroid : public UiController, public Client {
       jobject jcaller,
       const base::android::JavaParamRef<jobject>& webContents,
       const base::android::JavaParamRef<jobjectArray>& parameterNames,
-      const base::android::JavaParamRef<jobjectArray>& parameterValues);
+      const base::android::JavaParamRef<jobjectArray>& parameterValues,
+      const base::android::JavaParamRef<jstring>& initialUrlString);
   ~UiControllerAndroid() override;
 
   // Overrides UiController:

components/autofill_assistant/browser/controller.cc

@@ -42,7 +42,8 @@ static const char* const kCallerScriptParameterName = "CALLER";
 void Controller::CreateAndStartForWebContents(
     content::WebContents* web_contents,
     std::unique_ptr<Client> client,
-    std::unique_ptr<std::map<std::string, std::string>> parameters) {
+    std::unique_ptr<std::map<std::string, std::string>> parameters,
+    const GURL& initialUrl) {
   // Get the key early since |client| will be invalidated when moved below.
   GURL server_url(client->GetServerUrl());
   DCHECK(server_url.is_valid());
@@ -51,7 +52,7 @@ void Controller::CreateAndStartForWebContents(
       client->GetIdentityManagerForPrimaryAccount());
   new Controller(web_contents, std::move(client),
                  WebController::CreateForWebContents(web_contents),
-                 std::move(service), std::move(parameters));
+                 std::move(service), std::move(parameters), initialUrl);
 }
 
 Service* Controller::GetService() {
@@ -87,7 +88,8 @@ Controller::Controller(
     std::unique_ptr<Client> client,
     std::unique_ptr<WebController> web_controller,
     std::unique_ptr<Service> service,
-    std::unique_ptr<std::map<std::string, std::string>> parameters)
+    std::unique_ptr<std::map<std::string, std::string>> parameters,
+    const GURL& initialUrl)
     : content::WebContentsObserver(web_contents),
       client_(std::move(client)),
       web_controller_(std::move(web_controller)),
@@ -110,9 +112,8 @@ Controller::Controller(
   }
 
   GetUiController()->SetUiDelegate(this);
-  if (!web_contents->IsLoading()) {
-    GetOrCheckScripts(web_contents->GetLastCommittedURL());
-  }
+  if (initialUrl.is_valid())
+    GetOrCheckScripts(initialUrl);
 
   if (allow_autostart_) {
     auto iter = parameters_->find(kCallerScriptParameterName);

components/autofill_assistant/browser/controller.h

@@ -42,7 +42,8 @@ class Controller : public ScriptExecutorDelegate,
   static void CreateAndStartForWebContents(
       content::WebContents* web_contents,
       std::unique_ptr<Client> client,
-      std::unique_ptr<std::map<std::string, std::string>> parameters);
+      std::unique_ptr<std::map<std::string, std::string>> parameters,
+      const GURL& initialUrl);
 
   // Overrides ScriptExecutorDelegate:
   Service* GetService() override;
@@ -60,7 +61,8 @@ class Controller : public ScriptExecutorDelegate,
              std::unique_ptr<Client> client,
              std::unique_ptr<WebController> web_controller,
              std::unique_ptr<Service> service,
-             std::unique_ptr<std::map<std::string, std::string>> parameters);
+             std::unique_ptr<std::map<std::string, std::string>> parameters,
+             const GURL& initialUrl);
   ~Controller() override;
 
   void GetOrCheckScripts(const GURL& url);

components/autofill_assistant/browser/controller_unittest.cc

@@ -61,6 +61,22 @@ class ControllerTest : public content::RenderViewHostTestHarness {
   ControllerTest() {}
   ~ControllerTest() override {}
 
+  static Controller* CreateController(
+      content::WebContents* web_contents,
+      std::unique_ptr<Client> client,
+      std::unique_ptr<WebController> web_controller,
+      std::unique_ptr<Service> service,
+      std::unique_ptr<std::map<std::string, std::string>> parameters,
+      const GURL& initialUrl) {
+    return new Controller(web_contents, std::move(client),
+                          std::move(web_controller), std::move(service),
+                          std::move(parameters), initialUrl);
+  }
+
+  static void DestroyController(Controller* controller) {
+    controller->OnDestroy();
+  }
+
   void SetUp() override {
     content::RenderViewHostTestHarness::SetUp();
 
@@ -72,10 +88,12 @@ class ControllerTest : public content::RenderViewHostTestHarness {
     mock_service_ = service.get();
     auto parameters = std::make_unique<std::map<std::string, std::string>>();
     parameters->insert(std::make_pair("a", "b"));
+    GURL initialUrl("");
 
     controller_ = new Controller(
         web_contents(), std::make_unique<FakeClient>(std::move(ui_controller)),
-        std::move(web_controller), std::move(service), std::move(parameters));
+        std::move(web_controller), std::move(service), std::move(parameters),
+        initialUrl);
 
     // Fetching scripts succeeds for all URLs, but return nothing.
     ON_CALL(*mock_service_, OnGetScriptsForUrl(_, _, _))
@@ -92,7 +110,7 @@ class ControllerTest : public content::RenderViewHostTestHarness {
   }
 
   void TearDown() override {
-    controller_->OnDestroy();  // deletes the controller and mocks
+    DestroyController(controller_);  // deletes the controller and mocks
     content::RenderViewHostTestHarness::TearDown();
   }
 
@@ -348,4 +366,20 @@ TEST_F(ControllerTest, LoadProgressChanged) {
   SimulateProgressChanged(0.4);
 }
 
+TEST_F(ControllerTest, InitialUrlLoads) {
+  GURL initialUrl("http://a.example.com/path");
+  auto service = std::make_unique<NiceMock<MockService>>();
+
+  EXPECT_CALL(*service.get(), OnGetScriptsForUrl(Eq(initialUrl), _, _))
+      .WillOnce(RunOnceCallback<2>(true, ""));
+
+  Controller* controller = ControllerTest::CreateController(
+      web_contents(),
+      std::make_unique<FakeClient>(
+          std::make_unique<NiceMock<MockUiController>>()),
+      std::make_unique<NiceMock<MockWebController>>(), std::move(service),
+      std::make_unique<std::map<std::string, std::string>>(), initialUrl);
+  ControllerTest::DestroyController(controller);
+}
+
 }  // namespace autofill_assistant