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

chrome/browser/ui/gtk/website_settings_popup_gtk.cc

 // 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.
 
 #include "chrome/browser/ui/gtk/website_settings_popup_gtk.h"
 
 #include "base/basictypes.h"

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

include this for DISALLOW_ macro and compiler_specific for OVERRIDE in the
header file, and remove them from here.

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

Done.

 #include "base/compiler_specific.h"
 #include "base/i18n/rtl.h"
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/certificate_viewer.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/gtk/browser_toolbar_gtk.h"
 #include "chrome/browser/ui/gtk/browser_window_gtk.h"
 #include "chrome/browser/ui/gtk/collected_cookies_gtk.h"
 #include "chrome/browser/ui/gtk/gtk_chrome_link_button.h"
 #include "chrome/browser/ui/gtk/gtk_util.h"
 #include "chrome/browser/ui/gtk/gtk_theme_service.h"
 #include "chrome/browser/ui/gtk/location_bar_view_gtk.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
-#include "chrome/browser/website_settings.h"
+#include "chrome/browser/ui/website_settings/website_settings.h"
 #include "chrome/common/url_constants.h"
+#include "content/public/browser/cert_store.h"
 #include "grit/generated_resources.h"
 #include "grit/locale_settings.h"
 #include "grit/theme_resources.h"
 #include "ui/base/gtk/gtk_hig_constants.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "googleurl/src/gurl.h"

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

sort this.

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

Done.

 
 using content::OpenURLParams;
 
 namespace {
 
 // The background color of the tabs if a theme other than the native GTK theme
 // is selected.
 const GdkColor kBackgroundColor = GDK_COLOR_RGB(0xff, 0xff, 0xff);
 
 std::string PermissionTypeToString(ContentSettingsType type) {
@@ skipping 21 matching lines @@
     case CONTENT_SETTING_ASK:
       return l10n_util::GetStringUTF8(IDS_WEBSITE_SETTINGS_PERMISSION_ASK);
     default:
       NOTREACHED();
       return "";
   }
 }
 
 }  // namespace
 
+// static
+void WebsiteSettingsPopupGtk::Show(gfx::NativeWindow parent,

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

the order of the implementation of this function is also fine, but I tend to put
the at the end of the file.

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

I'd prefere to have the same order as in the header file. If you don't mind.

+                                   Profile* profile,
+                                   TabContentsWrapper* tab_contents_wrapper,
+                                   const GURL& url,
+                                   const content::SSLStatus& ssl) {
+  new WebsiteSettingsPopupGtk(parent,

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

since this is your code and I'm not the OWNER, this style is fine by me. But my
preference, and Peter's one (pkasting) is to write it like this:

new WebsiteSettingsPopupGtk(parent, profile, tab_contents_wrapper, url, ssl);

to save as much as possible vertical lines.

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

Cool. This fits in a single line. Great.

+                              profile,
+                              tab_contents_wrapper,
+                              url,
+                              ssl);
+}
+
 WebsiteSettingsPopupGtk::WebsiteSettingsPopupGtk(
     gfx::NativeWindow parent,
     Profile* profile,
-    TabContentsWrapper* tab_contents_wrapper)
+    TabContentsWrapper* tab_contents_wrapper,
+    const GURL& url,
+    const content::SSLStatus& ssl)
     : parent_(parent),
       contents_(NULL),
       theme_service_(GtkThemeService::GetFrom(profile)),
       profile_(profile),
       tab_contents_wrapper_(tab_contents_wrapper),
       browser_(NULL),
       cert_id_(0),
       header_box_(NULL),
       cookies_section_contents_(NULL),
       permissions_section_contents_(NULL),
@@ skipping 17 matching lines @@
                             arrow_location,
                             BubbleGtk::MATCH_SYSTEM_THEME |
                                 BubbleGtk::POPUP_WINDOW |
                                 BubbleGtk::GRAB_INPUT,
                             theme_service_,
                             this);  // |delegate|
   if (!bubble_) {
     NOTREACHED();
     return;
   }
+
+  presenter_.reset(new WebsiteSettings(

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

so you will have to do this in views and cocoa. :/

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

I instantiated the WebsiteSettings class in the platform specific show method in
the old version. This would have also required to do this for Views and cocoa
too. 
 

+      this,
+      profile,
+      tab_contents_wrapper->content_settings(),
+      url,
+      ssl,
+      content::CertStore::GetInstance()));
 }
 
 WebsiteSettingsPopupGtk::~WebsiteSettingsPopupGtk() {
 }
 
-void WebsiteSettingsPopupGtk::SetPresenter(WebsiteSettings* presenter) {
-  presenter_ = presenter;
-}
-
 void WebsiteSettingsPopupGtk::BubbleClosing(BubbleGtk* bubble,
                                             bool closed_by_escape) {
-  if (presenter_)
-    presenter_->OnUIClosing();
+  if (presenter_.get())
+    presenter_.reset(NULL);

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

you don't need to pass NULL here, scoped_ptr::reset has it as default parameter
(C* p = NULL). Yeah, I know this is forbidden by the style guide (I mean, the
use of default argument).

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

Done.

+  delete this;
 }
 
 void WebsiteSettingsPopupGtk::InitContents() {
   if (!contents_) {
     contents_ = gtk_vbox_new(FALSE, ui::kContentAreaSpacing);
     gtk_container_set_border_width(GTK_CONTAINER(contents_),
                                    ui::kContentAreaBorder);
   } else {
     gtk_util::RemoveAllChildren(contents_);
   }
@@ skipping 370 matching lines @@
   GtkTreeIter it;
   bool has_active = gtk_combo_box_get_active_iter(GTK_COMBO_BOX(widget), &it);
   DCHECK(has_active);
   GtkTreeModel* store =
       GTK_TREE_MODEL(gtk_combo_box_get_model(GTK_COMBO_BOX(widget)));
 
   int value = -1;
   int type = -1;
   gtk_tree_model_get(store, &it, 1, &value, 2, &type, -1);
 
-  if (presenter_)
+  if (presenter_.get())
     presenter_->OnSitePermissionChanged(ContentSettingsType(type),
                                        ContentSetting(value));

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

nit: while you are, fix the indentation here.

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

Done.  Same above.

 }
 
 void WebsiteSettingsPopupGtk::OnViewCertLinkClicked(GtkWidget* widget) {
   DCHECK_NE(cert_id_, 0);
   ShowCertificateViewerByID(GTK_WINDOW(parent_), cert_id_);
   bubble_->Close();
 }