views: Packaged app window icon should be system/generic icon, if icon is not set by default.

views: Packaged app window icon should be system/generic icon, if icon is not set by default.

BUG=148390
TEST=manually


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

[Junmin Zhu, 2012-09-21 08:12:33]

Please review the patch, thanks.

[sky, 2012-09-21 16:15:51]

http://codereview.chromium.org/10967036/diff/1/chrome/browser/ui/views/extens...
File chrome/browser/ui/views/extensions/shell_window_views.cc (right):

http://codereview.chromium.org/10967036/diff/1/chrome/browser/ui/views/extens...
chrome/browser/ui/views/extensions/shell_window_views.cc:334:
views::ViewsDelegate* original_delegate =
This feels like a hack and is likely to be problematic in the future. What is
the ChromeViewsDelegate doing that we don't want done here? Can we push some
state into the widget/window so ViewsDelegate doesn't attempt to do whatever it
is doing now that is causing problems?

[Junmin Zhu, 2012-09-23 10:19:52]

On 2012/09/21 16:15:51, sky wrote:
>
http://codereview.chromium.org/10967036/diff/1/chrome/browser/ui/views/extens...
> File chrome/browser/ui/views/extensions/shell_window_views.cc (right):
> 
>
http://codereview.chromium.org/10967036/diff/1/chrome/browser/ui/views/extens...
> chrome/browser/ui/views/extensions/shell_window_views.cc:334:
> views::ViewsDelegate* original_delegate =
> This feels like a hack and is likely to be problematic in the future. What is
> the ChromeViewsDelegate doing that we don't want done here? Can we push some
> state into the widget/window so ViewsDelegate doesn't attempt to do whatever
it
> is doing now that is causing problems?

Yes, we can add bool variable into Init::params for widget, and this would be
also avoid ViewsDelegate to set the default icon with chromium icon.

Update the patch, please kindly review it again. Thanks.

[sky, 2012-09-24 14:12:43]

LGTM

http://codereview.chromium.org/10967036/diff/5001/ui/views/widget/widget.h
File ui/views/widget/widget.h (right):

http://codereview.chromium.org/10967036/diff/5001/ui/views/widget/widget.h#ne...
ui/views/widget/widget.h:167: // Only used by ShellWindow. Specifies that the
default icon of packaged app
Document only used by windows.

[commit-bot: I haz the power, 2012-09-24 16:04:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/junmin.zhu@intel.com/10967036/9002

[commit-bot: I haz the power, 2012-09-24 19:12:55]

Change committed as 158343