Use alternate icon for Chrome shortcuts if specified in master preferences.

Use alternate icon for Chrome shortcuts if specified in master preferences.

This change depends on change 159539 that is currently under review and a new chrome2.ico that is yet to be added to the repository.

BUG=12701
TEST=Specify an alternate icon in --installerdata file and make sure that icon is used for shortcuts and it not changed later by updates.

Committed: http://src.chromium.org/viewvc/22314

[cpu_(ooo_6.6-7.5), 2009-07-31 01:20:54]

can you update the unit test as well?

http://codereview.chromium.org/159618/diff/1/2
File chrome/app/chrome_exe.rc (right):

http://codereview.chromium.org/159618/diff/1/2#newcode66
Line 66: IDR_MAINFRAME_2           ICON                   
"theme\google_chrome\chrome2.ico"
can you remove 1 space so they align vertically?

http://codereview.chromium.org/159618/diff/1/3
File chrome/browser/first_run_win.cc (right):

http://codereview.chromium.org/159618/diff/1/3#newcode194
Line 194: *ping_delay = 90;
can you update
http://wiki.corp.google.com/twiki/bin/view/Main/ChromeRLZImplementation

with this information?

http://codereview.chromium.org/159618/diff/1/7
File chrome/installer/util/master_preferences.cc (right):

http://codereview.chromium.org/159618/diff/1/7#newcode20
Line 20: const wchar_t kChromeIconIndex[] = L"chrome_icon_index";
"desktop_icon_index" ? "chrome" seems vague here.

[kuchhal, 2009-07-31 16:54:10]

Done with all the changes.

On Thu, Jul 30, 2009 at 6:20 PM, <cpu@chromium.org> wrote:

> can you update the unit test as well?
>
>
> http://codereview.chromium.org/159618/diff/1/2
> File chrome/app/chrome_exe.rc (right):
>
> http://codereview.chromium.org/159618/diff/1/2#newcode66
> Line 66: IDR_MAINFRAME_2           ICON
> "theme\google_chrome\chrome2.ico"
> can you remove 1 space so they align vertically?
>
> http://codereview.chromium.org/159618/diff/1/3
> File chrome/browser/first_run_win.cc (right):
>
> http://codereview.chromium.org/159618/diff/1/3#newcode194
> Line 194: *ping_delay = 90;
> can you update
> http://wiki.corp.google.com/twiki/bin/view/Main/ChromeRLZImplementation
>
> with this information?
>
> http://codereview.chromium.org/159618/diff/1/7
> File chrome/installer/util/master_preferences.cc (right):
>
> http://codereview.chromium.org/159618/diff/1/7#newcode20
> Line 20: const wchar_t kChromeIconIndex[] = L"chrome_icon_index";
> "desktop_icon_index" ? "chrome" seems vague here.
>
>
> http://codereview.chromium.org/159618
>

[kuchhal, 2009-08-03 18:31:47]

Does this look OK now for commit?

[cpu_(ooo_6.6-7.5), 2009-08-03 20:42:25]

LGTM

The only thing I didn't see was the updated unit test with the new variable. It
can be done afterwards if you like.


On 2009/08/03 18:31:47, kuchhal wrote:
> Does this look OK now for commit?