Let mini_installer pass any arbitrary command line flags to setup.exe. This w...

Let mini_installer pass any arbitrary command line flags to setup.exe. This will let us run setup.exe in verbose logging mode without mucking around in the registry to keep setup.exe around after mini_installer finishes.

Also replace mini_installer black box icon with installer icon of setup.exe.


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

[cpu_(ooo_6.6-7.5), 2008-09-19 19:59:08]

http://codereview.chromium.org/2992/diff/1/3
File chrome/installer/mini_installer/mini_installer.cc (right):

http://codereview.chromium.org/2992/diff/1/3#newcode264
Line 264: // total size as three + one additional to hold command line options
periods at the end of comments.

http://codereview.chromium.org/2992/diff/1/3#newcode288
Line 288: 
Is omaha going to pass extra parameters to setup? why do we need this? is it for
debugging?

http://codereview.chromium.org/2992/diff/1/3#newcode297
Line 297: 
args[0] might not be the exe name. Depends how the process is created.

http://codereview.chromium.org/2992/diff/1/3#newcode301
Line 301: }
are we missing a LocalFree() ?

http://codereview.chromium.org/2992/diff/1/3#newcode363
Line 363: int setup_exit_code = 3;
If we add return value #3 do we clash with the return values of first run
chrome?

[kuchhal, 2008-10-01 01:24:58]

I have fixed all the issues. I started doing the registry hack instead of
command line params but realized we will need command line support for
distribution installer anyway which is coming next (home page set, search engine
etc). So I added check to see if the first param is for current executable or
not and kept the command line params pass through to setup.exe.

Please take another look.

[cpu_(ooo_6.6-7.5), 2008-10-03 03:19:23]

On 2008/10/01 01:24:58, kuchhal wrote:
> I have fixed all the issues. I started doing the registry hack instead of
> command line params but realized we will need command line support for
> distribution installer anyway which is coming next (home page set, search
engine
> etc). So I added check to see if the first param is for current executable or
> not and kept the command line params pass through to setup.exe.
> 
> Please take another look.

Ok, I'll review tomorrow. I was not thinking on doing all the possible
configuration in the command line. I think is too much, for example the
prepopulated bookmarks. I think we are going to need to process a 'command
file', well at least in chrome. I was thinking a json file for this.

[cpu_(ooo_6.6-7.5), 2008-10-08 20:00:32]

Sorry this is taking so long.

http://codereview.chromium.org/2992/diff/401/602
File chrome/installer/mini_installer/mini_installer.cc (right):

http://codereview.chromium.org/2992/diff/401/602#newcode97
Line 97: wchar_t *tmp = StrStr(str, end_str);
As a generic function, it does not seem to work. StrStr seems (in my
understanding) to return the first match of end_str in str.
I think what you want is to do a simple strcmp from the right offsets.

http://codereview.chromium.org/2992/diff/401/602#newcode274
Line 274: return;

1) In the rest of this file we use :: for windows calls so GetModuleFileName is
used here as ::GetModuleFileNameW

2) I would do the GetModuleFileNameW work only if args_num > 0

[cpu_(ooo_6.6-7.5), 2008-10-09 00:48:16]

LGTM