DialogBrowserTest implementation to invoke Content settings bubble dialogs.

chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.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_LOCATION_BAR_LOCATION_BAR_VIEW_MAC_H_
 #define CHROME_BROWSER_UI_COCOA_LOCATION_BAR_LOCATION_BAR_VIEW_MAC_H_
 
 #import <Cocoa/Cocoa.h>
 #include <stddef.h>
 
@@ skipping 65 matching lines @@
   OmniboxView* GetOmniboxView() override;
   LocationBarTesting* GetLocationBarForTesting() override;
 
   // Overridden from LocationBarTesting:
   int PageActionCount() override;
   int PageActionVisibleCount() override;
   ExtensionAction* GetPageAction(size_t index) override;
   ExtensionAction* GetVisiblePageAction(size_t index) override;
   void TestPageActionPressed(size_t index) override;
   bool GetBookmarkStarVisibility() override;
+  int ContentSettingImageModelCount() override;

[tapted, 2017/02/03 00:35:39]

Just a general comment - not sure there's much to do about it until the Cocoa
code is deleted - but this approach (inheriting from a testing interface in
release code) is quite unusual. The preferred approach is to implement a
`TestApi` which is a friend in release, but otherwise implemented (and compiled)
only in test files. (grep the codebase for test_api.h files to see examples - or
you can see a simple one I just made in
https://codereview.chromium.org/2666523002/ ).

There might not be a neat way to jump to this approach until we can assume we
are using a particular implementation (i.e. views only). I guess
LocationBarTesting has set a precedent already, but it feels quite bad.

Just something to think about -- maybe you can scrutinise the places in this CL
where test-only code is being added to the release build to see if there are
ways to avoid it.

[tapted, 2017/02/03 00:38:01]

Note I kinda went back on this in later comments - I think we should be using a
TestApi for the toolkit-views LocationBarView* . (The implementations of these
methods on LocationBarViewMac seem to be unused in the test, so they shouldn't
be added)

[kylix_rd, 2017/02/03 18:55:03]

I was just trying to do a minimum of damage while keeping the the existing model
intact. If this isn't the right approach, I would suggest a different, more
complete, pass is done. Since this is merely there to get the code to compile,
this CL should focus on its primary purpose. Revamping the testing APIs seem a
little out of scope here.

[tapted, 2017/02/03 23:07:02]

We tend not to worry about damage if there's an overall improvement to code
health. But maintaining an existing broken model increases the code debt and
makes it harder to fix in future.

The current model is a violation of
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...
. i.e.

"Functions used only for testing should be restricted to test-only scenarios
either by #ifdefing them appropriately (e.g. #if defined(UNIT_TEST)) or by
naming them with a ForTesting suffix. The latter will be checked at presubmit
time to ensure they're only called by test files."


If, while improving code health, there's sufficient damage that it makes code
review hard, then we try to split the CL, and land the cleanup first.

+  ContentSettingImageModel* GetContentSettingImageModel(size_t index) override;
 
   // Set/Get the editable state of the field.
   void SetEditable(bool editable);
   bool IsEditable();
 
   // Set the starred state of the bookmark star.
   void SetStarred(bool starred);
 
   // Set whether or not the translate icon is lit.
   void SetTranslateIconLit(bool on);
@@ skipping 236 matching lines @@
   // The security level of the location bar icon.
   security_state::SecurityLevel security_level_;
 
   // Used to schedule a task for the first run info bubble.
   base::WeakPtrFactory<LocationBarViewMac> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(LocationBarViewMac);
 };
 
 #endif  // CHROME_BROWSER_UI_COCOA_LOCATION_BAR_LOCATION_BAR_VIEW_MAC_H_