Fix DCHECK in GCF quick-enable.

Fix DCHECK in GCF quick-enable.

Make the determination of whether or not Chrome Frame is in use on-demand rather than during initialization.

BUG=92146
TEST=GCF quick-enable should work again.


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

[erikwright (departed), 2011-08-10 15:31:29]

LGTM, but I can only guess at what the nature of the regression was based on the
fix. I wouldn't mind having more details.

In general, this only reduces the window of the race condition. Would it not be
ideal to insert a work item that actually tried the overwrite and failed over to
'new_chrome.exe' after the failure? Is it complicated to insert such a custom
work item?

[grt (UTC plus 2), 2011-08-11 10:41:19]

On 2011/08/10 15:31:29, erikwright wrote:
> LGTM, but I can only guess at what the nature of the regression was based on
the
> fix. I wouldn't mind having more details.

The previous change to installer_state.cc was doing work in
InstallerState::Initialize that was only valid and only useful when an install
was being performed.  By doing that work on-demand from within the install flow,
we avoid the bad conditions.

> In general, this only reduces the window of the race condition. Would it not
be
> ideal to insert a work item that actually tried the overwrite and failed over
to
> 'new_chrome.exe' after the failure? Is it complicated to insert such a custom
> work item?

This change wasn't meant to address the race condition; it's just as racy as it
was before.  That race wasn't what caused the DCHECK.  As far as overwriting
goes, npchrome_frame.dll isn't overwritten during an update.  It sounds
reasonable to use the fancy new conditional work item stuff to defer the
IsChromeFrameInUse check to the point where the work item list is being
evaluated.  This is something Robert (CCd) can consider when he's back, since he
has the best grasp of that stuff.

Thanks.

[commit-bot: I haz the power, 2011-08-11 12:37:29]

Change committed as 96370