Make the gtk download shelf autoclose on mouse out.

Make the gtk download shelf autoclose on mouse out.

BUG=27797
TEST=manual

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

[sky, 2011-01-31 17:14:04]

http://codereview.chromium.org/6249022/diff/2001/chrome/browser/ui/gtk/downlo...
File chrome/browser/ui/gtk/download_shelf_gtk.cc (right):

http://codereview.chromium.org/6249022/diff/2001/chrome/browser/ui/gtk/downlo...
chrome/browser/ui/gtk/download_shelf_gtk.cc:66: auto_close_factory_(this) {
Do we only use the ALLOW_THIS_IN... macro on windows?

http://codereview.chromium.org/6249022/diff/2001/chrome/browser/ui/gtk/downlo...
chrome/browser/ui/gtk/download_shelf_gtk.cc:165: }
Do you need SetCloseOnMouseOut(false) ?
Otherwise if close_on_mouse_out_ is true won't the MessageLoop still have this
DownloadShelfGtk as an observer?

http://codereview.chromium.org/6249022/diff/2001/gfx/insets.h
File gfx/insets.h (right):

http://codereview.chromium.org/6249022/diff/2001/gfx/insets.h#newcode32
gfx/insets.h:32: Insets(int top_bottom, int left_right)
Are you even using the two constructors you're adding?

http://codereview.chromium.org/6249022/diff/2001/gfx/insets.h#newcode37
gfx/insets.h:37: Insets(int inset)
explicit

[Evan Stade, 2011-01-31 18:59:48]

http://codereview.chromium.org/6249022/diff/2001/chrome/browser/ui/gtk/downlo...
File chrome/browser/ui/gtk/download_shelf_gtk.cc (right):

http://codereview.chromium.org/6249022/diff/2001/chrome/browser/ui/gtk/downlo...
chrome/browser/ui/gtk/download_shelf_gtk.cc:66: auto_close_factory_(this) {
On 2011/01/31 17:14:04, sky wrote:
> Do we only use the ALLOW_THIS_IN... macro on windows?

yep

http://codereview.chromium.org/6249022/diff/2001/chrome/browser/ui/gtk/downlo...
chrome/browser/ui/gtk/download_shelf_gtk.cc:165: }
On 2011/01/31 17:14:04, sky wrote:
> Do you need SetCloseOnMouseOut(false) ?
> Otherwise if close_on_mouse_out_ is true won't the MessageLoop still have this
> DownloadShelfGtk as an observer?

I suppose I do

http://codereview.chromium.org/6249022/diff/2001/gfx/insets.h
File gfx/insets.h (right):

http://codereview.chromium.org/6249022/diff/2001/gfx/insets.h#newcode32
gfx/insets.h:32: Insets(int top_bottom, int left_right)
On 2011/01/31 17:14:04, sky wrote:
> Are you even using the two constructors you're adding?

well I did at first, but then I changed my mind. I fashioned this change after
the way defining borders works in css. I can remove the change if you want, but
it seems like a common use case.

http://codereview.chromium.org/6249022/diff/2001/gfx/insets.h#newcode37
gfx/insets.h:37: Insets(int inset)
On 2011/01/31 17:14:04, sky wrote:
> explicit

Done.

[sky, 2011-01-31 20:36:25]

http://codereview.chromium.org/6249022/diff/2001/gfx/insets.h
File gfx/insets.h (right):

http://codereview.chromium.org/6249022/diff/2001/gfx/insets.h#newcode32
gfx/insets.h:32: Insets(int top_bottom, int left_right)
On 2011/01/31 18:59:49, Evan Stade wrote:
> On 2011/01/31 17:14:04, sky wrote:
> > Are you even using the two constructors you're adding?
> 
> well I did at first, but then I changed my mind. I fashioned this change after
> the way defining borders works in css. I can remove the change if you want,
but
> it seems like a common use case.

Given that no one is using them, I say nuke them. I don't so much mind the
single arg one, but the two arg one is mildly confusing.

http://codereview.chromium.org/6249022/diff/10001/chrome/browser/resources/op...
File chrome/browser/resources/options/content_settings.js (right):

http://codereview.chromium.org/6249022/diff/10001/chrome/browser/resources/op...
chrome/browser/resources/options/content_settings.js:39:
location.hash(event.target.getAttribute('contentType'));
You probably didn't intend to include this, did you?

http://codereview.chromium.org/6249022/diff/10001/chrome/browser/resources/op...
File chrome/browser/resources/options/content_settings_exceptions_area.js
(right):

http://codereview.chromium.org/6249022/diff/10001/chrome/browser/resources/op...
chrome/browser/resources/options/content_settings_exceptions_area.js:596:
handleHash: function(hash) {
Or this.

[Evan Stade, 2011-01-31 20:49:25]

done and done

[sky, 2011-01-31 21:17:51]

LGTM