DialogBrowserTest implementation to invoke Content settings bubble dialogs.

chrome/browser/ui/content_settings/content_setting_image_model.h

 // 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.
 
 #ifndef CHROME_BROWSER_UI_CONTENT_SETTINGS_CONTENT_SETTING_IMAGE_MODEL_H_
 #define CHROME_BROWSER_UI_CONTENT_SETTINGS_CONTENT_SETTING_IMAGE_MODEL_H_
 
 #include "base/macros.h"
 #include "base/strings/string16.h"
 #include "build/build_config.h"
@@ skipping 46 matching lines @@
       content::WebContents* web_contents);
 #endif
 
   gfx::Image GetIcon(SkColor nearby_text_color) const;
 
   // Returns the resource ID of a string to show when the icon appears, or 0 if
   // we don't wish to show anything.
   int explanatory_string_id() const { return explanatory_string_id_; }
   const base::string16& get_tooltip() const { return tooltip_; }
 
+  // For testing only. If this image model has a content type, it is returned
+  // here. Otherwise, CONTENT_SETTINGS_TYPE_DEFAULT is returned.
+  virtual ContentSettingsType GetContentType();

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

This is adding a lot of plumbing. (and we generally shouldn't rely on a comment
saying `ForTestingOnly`, instead we do GetContentTypeForTesting() -- there are
presubmits that check for that being used outside of test code -- but it doesn't
work for virtual methods.  there is also a performance cost converting an
accessor into a virtual method).

I see that it's tricky with all the subclasses though. But this is
cross-platform code, so there's no reason to avoid a TestApi. And I think at
least we should try to remove the virtual method. I suggest

namespace test {
class ContentSettingImageModelTestApi;
}

class ContentSettingImageModel {
..
protected:
  set_last_content_type_for_test(ContentSettingsType content_type) {
    last_content_type_for_test_ = content_type;
  }
..

private:
  friend class test::ContentSettingImageModelTestApi;
..
  ContentSettingsType last_content_type_for_test_ = /*something invalid */
};


in content_setting_bubble_dialog_browsertest.cc (for now - we can move it later
if other tests want it)

namespace test {
class ContentSettingImageModelTestApi {
 public:
  explicit ContentSettingImageModelTestApi(ContentSettingImageModel* model) :
model_(model) {}

  Foo::ContentType last_content_type() { return
model_->last_content_type_for_test_; }

 private:
  ContentSettingImageModel* model_;
};
}  // namespace test


(then remove the ContentSettingMediaImageModel bits and have other subclasses
set last_content_type_for_test_ in their constructor)

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

This seems a little overly complicated for merely getting one value. I would
argue that where this code is exercised (in the UI), the performance hit for the
added virtual method is negligible. I understand the sentiment here, but I don't
think the overhead is enough to warrant all this complexity. In plenty of other
cases, yes, but these classes are fairly simple.

+
  protected:
   ContentSettingImageModel();
 
   void set_icon_by_vector_id(gfx::VectorIconId id, gfx::VectorIconId badge_id) {
     icon_id_ = id;
     icon_badge_id_ = badge_id;
   }
 
   void set_visible(bool visible) { is_visible_ = visible; }
   void set_explanatory_string_id(int text_id) {
@@ skipping 18 matching lines @@
   explicit ContentSettingSimpleImageModel(ContentSettingsType content_type);
 
   // ContentSettingImageModel implementation.
   ContentSettingBubbleModel* CreateBubbleModel(
       ContentSettingBubbleModel::Delegate* delegate,
       content::WebContents* web_contents,
       Profile* profile) override;
   bool ShouldRunAnimation(content::WebContents* web_contents) override;
   void SetAnimationHasRun(content::WebContents* web_contents) override;
 
+  ContentSettingsType GetContentType() override;
+
   // Factory method. Used only for testing.
   static std::unique_ptr<ContentSettingImageModel>
   CreateForContentTypeForTesting(ContentSettingsType content_type);
 
- protected:
-  ContentSettingsType content_type() { return content_type_; }
-
  private:
   ContentSettingsType content_type_;
 
   DISALLOW_COPY_AND_ASSIGN(ContentSettingSimpleImageModel);
 };
 
 // Image model for subresource filter icons in the location bar.
 class ContentSettingSubresourceFilterImageModel
     : public ContentSettingImageModel {
  public:
   ContentSettingSubresourceFilterImageModel();
 
   void UpdateFromWebContents(content::WebContents* web_contents) override;
 
   ContentSettingBubbleModel* CreateBubbleModel(
       ContentSettingBubbleModel::Delegate* delegate,
       content::WebContents* web_contents,
       Profile* profile) override;
 
   bool ShouldRunAnimation(content::WebContents* web_contents) override;
   void SetAnimationHasRun(content::WebContents* web_contents) override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ContentSettingSubresourceFilterImageModel);
 };
 
 #endif  // CHROME_BROWSER_UI_CONTENT_SETTINGS_CONTENT_SETTING_IMAGE_MODEL_H_