Remove any platform specific code from the chrome/browser/website_settings.* .

chrome/browser/ui/gtk/website_settings_popup_gtk.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_GTK_WEBSITE_SETTINGS_POPUP_GTK_H_
 #define CHROME_BROWSER_UI_GTK_WEBSITE_SETTINGS_POPUP_GTK_H_
 #pragma once
 
-#include "chrome/browser/ui/website_settings_ui.h"
+#include "chrome/browser/ui/website_settings/website_settings_ui.h"

[tfarina, 2012/06/01 21:20:26]

nit: group with the other includes below. This is a header file, I'm pretty sure
we don't use this order in header files.

[markusheintz_, 2012/06/02 21:22:33]

Done.

 
 #include <gtk/gtk.h>
 
+#include "base/memory/scoped_ptr.h"
 #include "chrome/browser/ui/gtk/bubble/bubble_gtk.h"
 
 class Browser;
 class BubbleGtk;

[tfarina, 2012/06/01 21:20:26]

nit: forgot to remove?

[markusheintz_, 2012/06/02 21:22:33]

uups sorry. Removed.

+
+namespace content {

[tfarina, 2012/06/01 21:20:26]

please, move this after the forward declarations below.

[markusheintz_, 2012/06/02 21:22:33]

Done.

+struct SSLStatus;
+}
+
 class GtkThemeService;

[tfarina, 2012/06/01 21:20:26]

group these forward declarations with the Browser one above.

[markusheintz_, 2012/06/02 21:22:33]

Done.

+class GURL;
 class Profile;
 class TabContentsWrapper;
 class WebsiteSettings;
 
-// GTK implementation of the website settings UI. The website settings UI is
-// displayed in a popup that is positioned relative the an anchor element.
+// GTK implementation of the website settings UI.
 class WebsiteSettingsPopupGtk : public WebsiteSettingsUI,
                                 public BubbleDelegateGtk {
  public:
-  WebsiteSettingsPopupGtk(gfx::NativeWindow parent,
-                          Profile* profile,
-                          TabContentsWrapper* wrapper);
-  virtual ~WebsiteSettingsPopupGtk();
+  // Creates a |WebsiteSettingsPopupGtk| and displays the UI. The |url|
+  // contains the omnibox URL of the currently active tab, |parent| contains
+  // the currently active window, |profile| contains the currently active
+  // profile and |ssl| contains the |SSLStatus| of the connection to the
+  // website in the currently active tab that is wrapped by the
+  // |tab_contents_wrapper|.
+  static void Show(gfx::NativeWindow parent,
+                   Profile* profile,
+                   TabContentsWrapper* tab_contents_wrapper,
+                   const GURL& url,
+                   const content::SSLStatus& ssl);
 
   // WebsiteSettingsUI implementations.
-  virtual void SetPresenter(WebsiteSettings* presenter) OVERRIDE;
   virtual void SetCookieInfo(const CookieInfoList& cookie_info_list) OVERRIDE;

[tfarina, 2012/06/01 21:20:26]

I'm away from my Linux desktop, but I think I can prove that you can move these
to private section, with a simple test program. ;)
An exercise that I'll do in the weekend.

If I'm not wrong, this might work, but I'll verify on the weekend:

class FooInterface {
 public:
  virtual void Show() = 0;

 protected:
  virtual ~FooInterface();
};

class FooImpl : public FooInterface {
 private:
  virtual void Show() OVERRIDE;
};

FooInterface* interface = new FooImpl();
interface->Show();

[markusheintz_, 2012/06/02 21:22:33]

hm ... why should I make these methods private here? After all they are part of
the public interface. Being a C++ Padawan I don't understand the motivation for
this change (yet).
 

[tfarina, 2012/06/03 16:09:04]

The reason I suggested this is:

- WebsiteSettingsPopupGtk is a delegate for BubbleGtk. So BubbleGtk will call
BubbleClosing through its BubbleDelegateGtk interface, which I showed an example
above.
- Same thing for WebsiteSettingsUI.
- While these overridden methods might be part of WebsiteSettingsPopupGtk public
interface, I see they aren't intended to be called by the consumers of
WebsiteSettingsPopupGtk.

You aren't going to do this, for example:
WebsiteSettingsPopupGtk* popup = new WebsiteSettingsPopupGtk (...);
popup->SetPermissingInfo(...);

But that is just my view, I'm fine with whatever you think it's more
appropriate.

[markusheintz_, 2012/06/04 16:33:54]

You are right. Thanks a lot for the long description !!!

I made these methods private.

   virtual void SetPermissionInfo(
       const PermissionInfoList& permission_info_list) OVERRIDE;
   virtual void SetIdentityInfo(const IdentityInfo& identity_info) OVERRIDE;
   virtual void SetFirstVisit(const string16& first_visit) OVERRIDE;
 
   // BubbleDelegateGtk implementation.
   virtual void BubbleClosing(BubbleGtk* bubble, bool closed_by_escape) OVERRIDE;
 
  private:
+  WebsiteSettingsPopupGtk(gfx::NativeWindow parent,
+                          Profile* profile,
+                          TabContentsWrapper* wrapper,
+                          const GURL& url,
+                          const content::SSLStatus& ssl);
+
+  virtual ~WebsiteSettingsPopupGtk();
+
   // Layouts the different sections retrieved from the model.
   void InitContents();
 
   // Removes all children of |container|.
   void ClearContainer(GtkWidget* container);
 
   // Creates a label that contains the given |text| and has the given |width|.
   GtkWidget* CreateTextLabel(const std::string& text, int width);
 
   // Creates a popup section and returns a virtual box that contains the
@@ skipping 47 matching lines @@
   GtkWidget* permissions_section_contents_;
 
   // Container for the identity tab content.
   GtkWidget* identity_tab_contents_;
 
   // Container for the information about the first visit date of the website.
   GtkWidget* first_visit_contents_;
 
   // The UI translates user actions to specific events and forwards them to the
   // |presenter_|. The |presenter_| handles these events and updates the UI.
-  WebsiteSettings* presenter_;
+  scoped_ptr<WebsiteSettings> presenter_;
 
   DISALLOW_COPY_AND_ASSIGN(WebsiteSettingsPopupGtk);
 };
 
 #endif  // CHROME_BROWSER_UI_GTK_WEBSITE_SETTINGS_POPUP_GTK_H_