Implementation of the page info dialog on Linux Gtk.

Implementation of the page info dialog on Linux Gtk.

BUG=http://crbug.com/11598
TEST=Visit an HTTPS page, click the lock icon, the page info should show.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=22070

[mattm, 2009-07-28 21:00:37]

Also, see the win and mac try results.

http://codereview.chromium.org/159521/diff/24/27
File chrome/browser/gtk/location_bar_view_gtk.h (right):

http://codereview.chromium.org/159521/diff/24/27#newcode92
Line 92: LocationBarViewGtk* location_bar);
Move this up or down (so it doesn't split up HandleExpose and HandleExposeThunk)

http://codereview.chromium.org/159521/diff/24/28
File chrome/browser/gtk/page_info_window_gtk.cc (right):

http://codereview.chromium.org/159521/diff/24/28#newcode102
Line 102: GtkWidget* vbox = gtk_vbox_new(FALSE, gtk_util::kContentAreaBorder);
Should be kControlSpacing, I think.

http://codereview.chromium.org/159521/diff/24/28#newcode108
Line 108: gtk_label_set_attributes(GTK_LABEL(label), attributes);
should call pango_attr_list_unref(attributes) after this

http://codereview.chromium.org/159521/diff/24/28#newcode121
Line 121: GtkWidget* vbox2 = gtk_vbox_new(FALSE, gtk_util::kControlSpacing);
if possible, might be nice to come up with more descriptive names for some of
these boxes.  Just calling the outer one vbox seems okay, but with the inner
ones it could be helpful...

http://codereview.chromium.org/159521/diff/24/28#newcode153
Line 153: gtk_widget_show_now(dialog_);
Is there any reason to use this over gtk_widget_show?

[jcampan, 2009-07-28 22:08:28]

I think the Mac and Windows failures are related to my change in the page info
model (std::wstring -> string16). I am looking into it.

http://codereview.chromium.org/159521/diff/24/27
File chrome/browser/gtk/location_bar_view_gtk.h (right):

http://codereview.chromium.org/159521/diff/24/27#newcode92
Line 92: LocationBarViewGtk* location_bar);
On 2009/07/28 21:00:37, mattm wrote:
> Move this up or down (so it doesn't split up HandleExpose and
HandleExposeThunk)
Done.
(I was trying to keep the static methods grouped, but I don't think it matters)

http://codereview.chromium.org/159521/diff/24/28
File chrome/browser/gtk/page_info_window_gtk.cc (right):

http://codereview.chromium.org/159521/diff/24/28#newcode102
Line 102: GtkWidget* vbox = gtk_vbox_new(FALSE, gtk_util::kContentAreaBorder);
On 2009/07/28 21:00:37, mattm wrote:
> Should be kControlSpacing, I think.

Done.

http://codereview.chromium.org/159521/diff/24/28#newcode108
Line 108: gtk_label_set_attributes(GTK_LABEL(label), attributes);
On 2009/07/28 21:00:37, mattm wrote:
> should call pango_attr_list_unref(attributes) after this

Done.

http://codereview.chromium.org/159521/diff/24/28#newcode121
Line 121: GtkWidget* vbox2 = gtk_vbox_new(FALSE, gtk_util::kControlSpacing);
On 2009/07/28 21:00:37, mattm wrote:
> if possible, might be nice to come up with more descriptive names for some of
> these boxes.  Just calling the outer one vbox seems okay, but with the inner
> ones it could be helpful... 

Done.

http://codereview.chromium.org/159521/diff/24/28#newcode153
Line 153: gtk_widget_show_now(dialog_);
On 2009/07/28 21:00:37, mattm wrote:
> Is there any reason to use this over gtk_widget_show?

Not really, it is just me being unfamiliar with Gtk.
Changed to use gtk_widget_show.

[mattm, 2009-07-29 03:09:19]

http://codereview.chromium.org/159521/diff/42/50
File chrome/browser/page_info_window.h (left):

http://codereview.chromium.org/159521/diff/42/50#oldcode21
Line 21: class PageInfoWindow {
So chrome/browser/page_info_window.cc isn't used by anything?  Delete it?

[jcampan, 2009-07-29 17:06:32]

http://codereview.chromium.org/159521/diff/42/50
File chrome/browser/page_info_window.h (left):

http://codereview.chromium.org/159521/diff/42/50#oldcode21
Line 21: class PageInfoWindow {
On 2009/07/29 03:09:19, mattm wrote:
> So chrome/browser/page_info_window.cc isn't used by anything?  Delete it?

Good catch, removed.

[mattm, 2009-07-29 19:15:04]

On 2009/07/29 17:06:32, jcampan wrote:
> http://codereview.chromium.org/159521/diff/42/50
> File chrome/browser/page_info_window.h (left):
> 
> http://codereview.chromium.org/159521/diff/42/50#oldcode21
> Line 21: class PageInfoWindow {
> On 2009/07/29 03:09:19, mattm wrote:
> > So chrome/browser/page_info_window.cc isn't used by anything?  Delete it?
> 
> Good catch, removed.

lgtm