Implement the load chrome.dll before file version for Google Chrome builds.

Implement the load chrome.dll before file version for Google Chrome builds.

[Dean McNamee, 2008-11-11 19:16:54]

Not super into how I did this, but oh well.  Haven't tried to compile it yet.

[Nicolas Sylvain, 2008-11-11 19:24:00]

http://codereview.chromium.org/10300/diff/1/2
File chrome/app/google_update_client.cc (left):

http://codereview.chromium.org/10300/diff/1/2#oldcode80
Line 80: client_util::DLL_MAIN entry = reinterpret_cast<client_util::DLL_MAIN>(
what do you think about doing a : " if (!dll_handle_) Load() " ?

http://codereview.chromium.org/10300/diff/1/2
File chrome/app/google_update_client.cc (right):

http://codereview.chromium.org/10300/diff/1/2#newcode35
Line 35: GoogleUpdateClient::GoogleUpdateClient() : version_(NULL),
dll_handle_(NULL) {
I think you forgot to define dll_handle_

[Erik does not do reviews, 2008-11-11 19:25:00]

http://codereview.chromium.org/10300/diff/1/2
File chrome/app/google_update_client.cc (right):

http://codereview.chromium.org/10300/diff/1/2#newcode35
Line 35: GoogleUpdateClient::GoogleUpdateClient() : version_(NULL),
dll_handle_(NULL) {
dll_handle_ doesn't exist in the header file

http://codereview.chromium.org/10300/diff/1/2#newcode61
Line 61: HINSTANCE dll_handle_ = ::LoadLibraryEx(dll_.c_str(), NULL,
and when it does, you'll need to remove the HINSTANCE here

[Dean McNamee, 2008-11-14 18:32:31]

Ok, updated.  Still haven't tried to compile it, is there an easy way?

On 2008/11/11 19:25:00, Erik Kay wrote:
> http://codereview.chromium.org/10300/diff/1/2
> File chrome/app/google_update_client.cc (right):
> 
> http://codereview.chromium.org/10300/diff/1/2#newcode35
> Line 35: GoogleUpdateClient::GoogleUpdateClient() : version_(NULL),
> dll_handle_(NULL) {
> dll_handle_ doesn't exist in the header file
> 
> http://codereview.chromium.org/10300/diff/1/2#newcode61
> Line 61: HINSTANCE dll_handle_ = ::LoadLibraryEx(dll_.c_str(), NULL,
> and when it does, you'll need to remove the HINSTANCE here

[cpu_(ooo_6.6-7.5), 2008-11-17 18:46:43]

Sorry for being late to this party. Here is my take:

- Installing the crash handler after chrome.dll has loaded is a bug. I don't
want to make that a permanent feature.

- Getting the version info before loading the chrome.dll is stupidly slow so I
agree with the spirit of the fix as well.

So my proposal is to install the handler before loading the dll but stuff the
version info into the static variables after chrome is loaded. I am going to bet
that with this approach the separate thread is not necessary as well greatly
simplifying this code.

[Dean McNamee, 2008-11-19 02:01:31]

Dropped the patch and filed 4564.

On 2008/11/17 18:46:43, cpu wrote:
> Sorry for being late to this party. Here is my take:
> 
> - Installing the crash handler after chrome.dll has loaded is a bug. I don't
> want to make that a permanent feature.
> 
> - Getting the version info before loading the chrome.dll is stupidly slow so I
> agree with the spirit of the fix as well.
> 
> So my proposal is to install the handler before loading the dll but stuff the
> version info into the static variables after chrome is loaded. I am going to
bet
> that with this approach the separate thread is not necessary as well greatly
> simplifying this code.