Uses child views in Autofill Popup so we can trigger

chrome/browser/ui/cocoa/autofill/autofill_popup_view_bridge.h

 // Copyright (c) 2012 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.
 
 #ifndef CHROME_BROWSER_UI_COCOA_AUTOFILL_AUTOFILL_POPUP_VIEW_BRIDGE_H_
 #define CHROME_BROWSER_UI_COCOA_AUTOFILL_AUTOFILL_POPUP_VIEW_BRIDGE_H_
 
 #include <stddef.h>
 
 #include <vector>
 
 #include "base/compiler_specific.h"
 #include "base/mac/scoped_nsobject.h"
 #include "base/macros.h"
 #include "chrome/browser/ui/autofill/autofill_popup_view.h"
 #include "chrome/browser/ui/cocoa/autofill/autofill_popup_view_cocoa.h"
+#include "ui/accessibility/ax_enums.h"
 
 @class AutofillPopupViewCocoa;
 @class NSWindow;
 
 namespace autofill {
 
 class AutofillPopupViewCocoaDelegate {
  public:
   // Returns the bounds of the item at |index| in the popup, relative to
   // the top left of the popup.
@@ skipping 15 matching lines @@
   // AutofillPopupViewCocoaDelegate implementation.
   gfx::Rect GetRowBounds(size_t index) override;
   int GetIconResourceID(const base::string16& resource_name) override;
 
  private:
   ~AutofillPopupViewBridge() override;
 
   // AutofillPopupView implementation.
   void Hide() override;
   void Show() override;
-  void InvalidateRow(size_t row) override;

[groby-ooo-7-16, 2017/03/20 20:42:55]

Why change away from InvalidateRow() - which doesn't require optional types - to
OnSelectedRowChanged, which does?

[Evan Stade, 2017/03/20 21:36:57]

I asked for this change. The optional change is to get rid of kNoSelection. The
API change is so that we only have to call it once, and because you're not just
invalidating, you're also sending an a11y event. The options are to modify this
API call or to add a new one.

[csashi, 2017/03/20 21:41:51]

FWIW, this was a result of a previous review. I agree that this particular
change is not needed for the bug I am fixing and that InvalidateRow did not need
an optional type, but the review recommendation seemed reasonable, so I
implemented it. As I see it, new interface is better because:

1. OnSelectedRowChanged better describes what happened.

2. We need some interface change, if not this one, then another way to indicate
what happened. Please see
https://codereview.chromium.org/2670003002/patch/440001/450008 and
https://codereview.chromium.org/2727233003/diff/220001/chrome/browser/ui/view...
for two other possibilities that I had implemented previously.

3. The view implementation in autofill_popup_view_views.cc is simpler (perhaps,
slightly faster?) because we schedule a single paint. I don't know if the
optimization applies to Cocoa. In order to reduce the scope of the CL I made
only the minimum required modifications to Cocoa.

[groby-ooo-7-16, 2017/03/20 22:53:10]

Thanks for the explanation - I wasn't aware of the views implication.

-  void UpdateBoundsAndRedrawPopup() override;
+  void OnSelectedRowChanged(int previous_row_selection,
+                            int current_row_selection) override;
+  void OnSuggestionsChanged() override;
 
   // Set the initial bounds of the popup, including its placement.
   void SetInitialBounds();
 
   // The native Cocoa view.
   base::scoped_nsobject<AutofillPopupViewCocoa> view_;
 
   AutofillPopupController* controller_;  // Weak.
 
   DISALLOW_COPY_AND_ASSIGN(AutofillPopupViewBridge);
 };
 
 }  // namespace autofill
 
 #endif  // CHROME_BROWSER_UI_COCOA_AUTOFILL_AUTOFILL_POPUP_VIEW_BRIDGE_H_