Remove a DCHECK that happens on every exit of IE. We appear to be leaking aut...

Remove a DCHECK that happens on every exit of IE. We appear to be leaking automation proxies, which I maintain is a bad thing.

BUG=Start IE with debug CF installed, navigate to a chrome frame page, exit IE, see the DCHECK.


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

[amit, 2009-10-28 15:59:04]

OK with this CL, log a bug for proper cleanup?

On Wed, Oct 28, 2009 at 7:07 AM, <robertshield@chromium.org> wrote:

> Reviewers: stoyan,
>
> Description:
> Remove a DCHECK that happens on every exit of IE. We appear to be leaking
> automation proxies, which I maintain is a bad thing.
>
> BUG=Start IE with debug CF installed, navigate to a chrome frame page, exit
> IE,
> see the DCHECK.
>
>
> Please review this at http://codereview.chromium.org/342018
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src/
>
> Affected files:
>  M     chrome_frame/chrome_frame_automation.cc
>
>
> Index: chrome_frame/chrome_frame_automation.cc
> ===================================================================
> --- chrome_frame/chrome_frame_automation.cc     (revision 30332)
> +++ chrome_frame/chrome_frame_automation.cc     (working copy)
> @@ -164,7 +164,10 @@
>  }
>
>  ProxyFactory::~ProxyFactory() {
> -  DCHECK_EQ(proxies_.container().size(), 0);
> +  if (!proxies_.container().empty()) {
> +    // TODO(stoyan): Proxies should not be leaked on exit.
> +    DLOG(ERROR) << "Proxies leaked on exit: " <<
> proxies_.container().size();
> +  }
>  }
>
>  void* ProxyFactory::GetAutomationServer(int launch_timeout,
>
>
>

[stoyan, 2009-10-28 16:26:27]

I would prefer something like:
for (;i < proxies_.size; ++i)
  DCHECK_EQ(WAIT_OBJECT_0,
            WaitForSingleObject(proxies_[i].thread.thread_handle, 0);

[robertshield, 2009-10-28 17:33:46]

On 2009/10/28 16:26:27, stoyan wrote:
> I would prefer something like:
> for (;i < proxies_.size; ++i)
>   DCHECK_EQ(WAIT_OBJECT_0,
>             WaitForSingleObject(proxies_[i].thread.thread_handle, 0);

How about this new version? Very similar, but I still went with the DLOG because
this constant crash on exit is annoying ;-) (the WFSO version DCHECKS too).

[stoyan, 2009-10-28 17:52:24]

lgtm
do you ever hit this DLOG?

[robert, 2009-10-28 18:00:15]

On Wed, Oct 28, 2009 at 1:52 PM, <stoyan@chromium.org> wrote:

> lgtm
> do you ever hit this DLOG?
>
> Yes.


>
> http://codereview.chromium.org/342018
>

[stoyan, 2009-10-28 18:40:23]

Hmm. Dumps are welcome.