Remove widgets related to restoring the default browser from uninstall view when default settings c…

Remove widgets related to restoring the default browser from uninstall view when default settings cannot be restored.


BUG=123320
TEST=N/A


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

[gab, 2012-06-07 19:13:28]

LGTM with modifications to added comment (see below).

Cheers,
Gab

https://chromiumcodereview.appspot.com/10453048/diff/4001/chrome/browser/ui/v...
File chrome/browser/ui/views/uninstall_view.cc (right):

https://chromiumcodereview.appspot.com/10453048/diff/4001/chrome/browser/ui/v...
chrome/browser/ui/views/uninstall_view.cc:73: // be set automatically as
default, neither can any other browser (for
automatically --> programmatically

https://chromiumcodereview.appspot.com/10453048/diff/4001/chrome/browser/ui/v...
chrome/browser/ui/views/uninstall_view.cc:74: // instance because the
distribution or the OS doesn't permit that.
"." => ")."

https://chromiumcodereview.appspot.com/10453048/diff/4001/chrome/browser/ui/v...
chrome/browser/ui/views/uninstall_view.cc:74: // instance because the
distribution or the OS doesn't permit that.
-distribution (i.e. our distribution is not the blocker, the OS is, the check
for dist->CanSetDefault() is only there because if we can't set ourselves as
default there is no way we actually *are* the default)

[motek., 2012-06-08 15:03:32]

http://codereview.chromium.org/10453048/diff/4001/chrome/browser/ui/views/uni...
File chrome/browser/ui/views/uninstall_view.cc (right):

http://codereview.chromium.org/10453048/diff/4001/chrome/browser/ui/views/uni...
chrome/browser/ui/views/uninstall_view.cc:73: // be set automatically as
default, neither can any other browser (for
On 2012/06/07 19:13:28, gab wrote:
> automatically --> programmatically

Done.

http://codereview.chromium.org/10453048/diff/4001/chrome/browser/ui/views/uni...
chrome/browser/ui/views/uninstall_view.cc:74: // instance because the
distribution or the OS doesn't permit that.
On 2012/06/07 19:13:28, gab wrote:
> "." => ")."

Done.

http://codereview.chromium.org/10453048/diff/4001/chrome/browser/ui/views/uni...
chrome/browser/ui/views/uninstall_view.cc:74: // instance because the
distribution or the OS doesn't permit that.
On 2012/06/07 19:13:28, gab wrote:
> -distribution (i.e. our distribution is not the blocker, the OS is, the check
> for dist->CanSetDefault() is only there because if we can't set ourselves as
> default there is no way we actually *are* the default)
Done + clarified a bit (because we indeed don't show it when distribution
settings say: "don't do it".

[gab, 2012-06-08 15:10:45]

LGTM

[grt (UTC plus 2), 2012-06-08 18:27:06]

lgtm

[Peter Kasting, 2012-06-08 18:37:48]

Rubber-stamp LGTM for OWNERS

http://codereview.chromium.org/10453048/diff/4002/chrome/browser/ui/views/uni...
File chrome/browser/ui/views/uninstall_view.cc (right):

http://codereview.chromium.org/10453048/diff/4002/chrome/browser/ui/views/uni...
chrome/browser/ui/views/uninstall_view.cc:79:
ShellIntegration::CanSetAsDefaultBrowser() !=
Nit: I'd put parens around this condition since it's split across multiple
lines.

http://codereview.chromium.org/10453048/diff/4002/chrome/browser/ui/views/uni...
chrome/browser/ui/views/uninstall_view.cc:137: if (change_default_browser_ &&
change_default_browser_ == sender) {
Nit: Why is this necessary?  You won't get called with |sender| == NULL so the
existing conditional already handles this.

[motek., 2012-06-08 18:49:17]

http://codereview.chromium.org/10453048/diff/4002/chrome/browser/ui/views/uni...
File chrome/browser/ui/views/uninstall_view.cc (right):

http://codereview.chromium.org/10453048/diff/4002/chrome/browser/ui/views/uni...
chrome/browser/ui/views/uninstall_view.cc:79:
ShellIntegration::CanSetAsDefaultBrowser() !=
On 2012/06/08 18:37:48, Peter Kasting wrote:
> Nit: I'd put parens around this condition since it's split across multiple
> lines.

Done.

http://codereview.chromium.org/10453048/diff/4002/chrome/browser/ui/views/uni...
chrome/browser/ui/views/uninstall_view.cc:137: if (change_default_browser_ &&
change_default_browser_ == sender) {
On 2012/06/08 18:37:48, Peter Kasting wrote:
> Nit: Why is this necessary?  You won't get called with |sender| == NULL so the
> existing conditional already handles this.
Wasn't sure of that. And since I am pathologically defensive... Removed.

[commit-bot: I haz the power, 2012-06-08 18:49:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/motek@chromium.org/10453048/10004

[commit-bot: I haz the power, 2012-06-08 19:00:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/motek@chromium.org/10453048/13002

[commit-bot: I haz the power, 2012-06-08 20:22:44]

Change committed as 141270