Include the Chrome Frame binary in Chrome installers. Note that it will only ...

Include the Chrome Frame binary in Chrome installers. Note that it will only be registered (and thus used) if the installer is run with the --chrome-frame flag.

Remove a bunch of CF-specific code in the process.

BUG=46818
TEST=Can use the same mini_installer.exe to install either Chrome or Chrome Frame.


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

[robertshield, 2010-06-17 20:52:33]

Hi Huan, 

 Please could you take a look at this. It removes the CF-specific
mini_installer.exe.

  CC'ing Anthony to let him know this is coming soon, and that the build
infrastructure may need some tweaking before this lands.

Thanks,
Robert

[huanr, 2010-06-18 18:07:53]

Add mal as this impacts installer download size.

What is the benefit of this change?

http://codereview.chromium.org/2839009/diff/5002/11003
File chrome/installer/mini_installer/chrome.release (right):

http://codereview.chromium.org/2839009/diff/5002/11003#newcode58
chrome/installer/mini_installer/chrome.release:58: servers\*.exe:
%(VersionDir)s\
How large are those files? What is the impact of download size?

http://codereview.chromium.org/2839009/diff/5002/11007
File chrome/installer/setup/setup_main.cc (right):

http://codereview.chromium.org/2839009/diff/5002/11007#newcode86
chrome/installer/setup/setup_main.cc:86:
file_util::AppendToPath(&existing_archive, installer::kChromeArchive);
Is this chrome archive? It may not be compatible for the first time we release
this change.

http://codereview.chromium.org/2839009/diff/5002/11007#newcode230
chrome/installer/setup/setup_main.cc:230: 
This is under temp directory, right?

[robertshield, 2010-06-18 19:01:58]

> What is the benefit of this change?

The main benefit is maintainability, it is becoming an increasingly large hassle
to have two separate release pipelines for binaries that are 99.99% identical.

http://codereview.chromium.org/2839009/diff/5002/11003
File chrome/installer/mini_installer/chrome.release (right):

http://codereview.chromium.org/2839009/diff/5002/11003#newcode58
chrome/installer/mini_installer/chrome.release:58: servers\*.exe:
%(VersionDir)s\
On 2010/06/18 18:07:53, huanr wrote:
> How large are those files? What is the impact of download size?

It adds about 100KB to the download size. On disk, the files are around 1.2 MB.

http://codereview.chromium.org/2839009/diff/5002/11007
File chrome/installer/setup/setup_main.cc (right):

http://codereview.chromium.org/2839009/diff/5002/11007#newcode86
chrome/installer/setup/setup_main.cc:86:
file_util::AppendToPath(&existing_archive, installer::kChromeArchive);
On 2010/06/18 18:07:53, huanr wrote:
> Is this chrome archive? It may not be compatible for the first time we release
> this change. 

This is true, it won't be compatible when this is first released. For the first
iteration of this, I plan to keep differential updates deactivated for Chrome
Frame and the allow this to settle. 

I could instead add another special case to handle the transition. What do you
think?

http://codereview.chromium.org/2839009/diff/5002/11007#newcode230
chrome/installer/setup/setup_main.cc:230: 
On 2010/06/18 18:07:53, huanr wrote:
> This is under temp directory, right?

Yes. Note that in practice, afaict the --install-archive flag is always given.
That explicitly references the archive in a temp directory and is picked up in
the next few lines below.

[huanr, 2010-06-23 18:13:01]

We need to well test the patch given its potential impact. For example, the
chrome_frame_appid.cc and corresponding kAppGuid in mini_installer need to be
changed. Can we work with QA to have a solid test plan in place? 

On 2010/06/18 19:01:58, robertshield wrote:
> > What is the benefit of this change?
> 
> The main benefit is maintainability, it is becoming an increasingly large
hassle
> to have two separate release pipelines for binaries that are 99.99% identical.
> 
> http://codereview.chromium.org/2839009/diff/5002/11003
> File chrome/installer/mini_installer/chrome.release (right):
> 
> http://codereview.chromium.org/2839009/diff/5002/11003#newcode58
> chrome/installer/mini_installer/chrome.release:58: servers\*.exe:
> %(VersionDir)s\
> On 2010/06/18 18:07:53, huanr wrote:
> > How large are those files? What is the impact of download size?
> 
> It adds about 100KB to the download size. On disk, the files are around 1.2
MB.
> 
> http://codereview.chromium.org/2839009/diff/5002/11007
> File chrome/installer/setup/setup_main.cc (right):
> 
> http://codereview.chromium.org/2839009/diff/5002/11007#newcode86
> chrome/installer/setup/setup_main.cc:86:
> file_util::AppendToPath(&existing_archive, installer::kChromeArchive);
> On 2010/06/18 18:07:53, huanr wrote:
> > Is this chrome archive? It may not be compatible for the first time we
release
> > this change. 
> 
> This is true, it won't be compatible when this is first released. For the
first
> iteration of this, I plan to keep differential updates deactivated for Chrome
> Frame and the allow this to settle. 
> 
> I could instead add another special case to handle the transition. What do you
> think?
> 
> http://codereview.chromium.org/2839009/diff/5002/11007#newcode230
> chrome/installer/setup/setup_main.cc:230: 
> On 2010/06/18 18:07:53, huanr wrote:
> > This is under temp directory, right?
> 
> Yes. Note that in practice, afaict the --install-archive flag is always given.
> That explicitly references the archive in a temp directory and is picked up in
> the next few lines below.

[robertshield, 2010-06-25 22:09:20]

I updated this to include the mini_installer.exe changes regarding app id
selection that you suggested. Please take a look.

I will sit on this while I figure out the full set of build / archiving script
changes that this will entail and talk to QA about validating the installer
unification.

Thanks!

[huanr, 2010-06-26 00:19:07]

LGTM