[refactor] Fix autofill features for payments when the form is inside an OOPIF

chrome/renderer/autofill/page_click_tracker_browsertest.cc

 // Copyright (c) 2011 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 <memory>
 
 #include "chrome/test/base/chrome_render_view_test.h"
+#include "components/autofill/content/renderer/autofill_agent.h"
 #include "components/autofill/content/renderer/page_click_listener.h"
 #include "components/autofill/content/renderer/page_click_tracker.h"
 #include "content/public/renderer/render_view.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/WebKit/public/platform/WebFloatPoint.h"
 #include "third_party/WebKit/public/platform/WebSize.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebInputElement.h"
 #include "third_party/WebKit/public/web/WebSettings.h"
 #include "third_party/WebKit/public/web/WebView.h"
@@ skipping 25 matching lines @@
   bool was_focused_;
 };
 
 class PageClickTrackerTest : public ChromeRenderViewTest {
  protected:
   void SetUp() override {
     ChromeRenderViewTest::SetUp();
 
     // RenderView creates PageClickTracker but it doesn't keep it around.
     // Rather than make it do so for the test, we create a new object.
-    page_click_tracker_.reset(new PageClickTracker(view_->GetMainRenderFrame(),
-                                                   &test_listener_));
+    page_click_tracker_ =

[vabr (Chromium), 2017/04/14 06:44:24]

While your code is correct, it takes some mental effort to understand the
ownership transfer (page_clicl_tracker_ starts owning the object but becomes
just weak). Please consider making the code clearer like this:

auto page_click_tracker = base::MakeUnique<PageClickTracker>(...);
page_click_tracker_weak_ = page_click_tracker.get();
autofill_agent_->set_page_click_tracker_for_testing(std::move(page_click_tracker));

[EhsanK, 2017/04/18 17:55:19]

Thanks! Sorry for the vagueness in the code. But I noticed that we do not even
need a local reference to PageClickTracker in the test class since it is never
used. I removed the reference to it in this patch.

+        new PageClickTracker(view_->GetMainRenderFrame(), &test_listener_);
+
+    autofill_agent_->set_page_click_tracker_for_testing(page_click_tracker_);
 
     // Must be set before loading HTML.
     view_->GetWebView()->SetDefaultPageScaleLimits(1, 4);
 
     LoadHTML("<form>"
              "  <input type='text' id='text_1'></input><br>"
              "  <input type='text' id='text_2'></input><br>"
              "  <textarea  id='textarea_1'></textarea><br>"
              "  <textarea  id='textarea_2'></textarea><br>"
              "  <input type='button' id='button'></input><br>"
@@ skipping 10 matching lines @@
 
     // Enable show-ime event when element is focused by indicating that a user
     // gesture has been processed since load.
     EXPECT_TRUE(SimulateElementClick("button"));
   }
 
   void TearDown() override {
     text_.Reset();
     textarea_.Reset();
     test_listener_.ClearResults();
-    page_click_tracker_.reset();
     ChromeRenderViewTest::TearDown();
   }
 
-  std::unique_ptr<PageClickTracker> page_click_tracker_;
+  PageClickTracker* page_click_tracker_;

[vabr (Chromium), 2017/04/14 06:44:24]

nit: Please rename to page_click_tracker_weak_ to clarify that this is not an
owning pointer.

[EhsanK, 2017/04/18 17:55:19]

Thanks! Removed as explained in comment above.

   TestPageClickListener test_listener_;
   blink::WebElement text_;
   blink::WebElement textarea_;
 };
 
 // Tests that PageClickTracker does notify correctly when an input
 // node is clicked.
 TEST_F(PageClickTrackerTest, PageClickTrackerInputClicked) {
   EXPECT_NE(text_, text_.GetDocument().FocusedElement());
   // Click the text field once.
@@ skipping 140 matching lines @@
   // Tap outside of element bounds, but tap width is overlapping the field.
   gfx::Rect element_bounds = GetElementBounds("text_1");
   SimulateRectTap(element_bounds -
                   gfx::Vector2d(element_bounds.width() / 2 + 1, 0));
   EXPECT_TRUE(test_listener_.form_control_element_clicked_called_);
   EXPECT_FALSE(test_listener_.was_focused_);
   EXPECT_TRUE(text_ == test_listener_.form_control_element_clicked_);
 }
 
 }  // namespace autofill