The ChromeFrameAutomationClient class needs to be refcounted as it implements...

The ChromeFrameAutomationClient class needs to be refcounted as it implements the PluginRequestHandler
interface which is maintained by individual requests which can outlive the active document/activex instances.
I ran into a crash where UrlmonUrlRequest was calling into an invalid PluginRequestHandler pointer which
had been destroyed just before.

We also need to ensure that UrlmonUrlRequest and ChromeFrameActiveXBase select the multi threaded model as
AddRef/Release can be invoked from multiple threads.

I also removed the CleanupAsyncRequests function in ChromeFrameAutomationClient and moved all the code to CleanupRequests, which ensures that we treat synchronous
and asynchronous requests similarly. 

There are instances where an automation client instance is created and destroyed without being initialized which causes a spurious assert to fire in the Uninitialize function. I added a check in the Uninitialize function to return if the state is uninitialized.

Bug=none


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

[tommi (sloooow) - chröme, 2009-11-12 16:21:33]

lgtm

http://codereview.chromium.org/386014/diff/3030/2023
File chrome_frame/chrome_frame_automation.cc (right):

http://codereview.chromium.org/386014/diff/3030/2023#newcode479
Line 479: return;
DLOG(WARNING)?

http://codereview.chromium.org/386014/diff/3030/2029
File chrome_frame/test/chrome_frame_unittests.cc (right):

http://codereview.chromium.org/386014/diff/3030/2029#newcode1161
Line 1161: scoped_refptr<ChromeFrameAutomationClient> client_;  // the victim of
all tests
80

[ananta, 2009-11-12 17:30:16]

http://codereview.chromium.org/386014/diff/3030/2023
File chrome_frame/chrome_frame_automation.cc (right):

http://codereview.chromium.org/386014/diff/3030/2023#newcode479
Line 479: return;
On 2009/11/12 16:21:33, tommi wrote:
> DLOG(WARNING)?

Done.

http://codereview.chromium.org/386014/diff/3030/2029
File chrome_frame/test/chrome_frame_unittests.cc (right):

http://codereview.chromium.org/386014/diff/3030/2029#newcode1161
Line 1161: scoped_refptr<ChromeFrameAutomationClient> client_;  // the victim of
all tests
On 2009/11/12 16:21:33, tommi wrote:
> 80

Done.

[amit, 2009-11-12 17:44:07]

The change LGTM and thanks for discovering the corner case and a quick fix.

However, things have become quite complicated because of a separate thread for
urlmon requests. It's going to hard to write good tests for this state machine.
It's also very hard to catch regressions. Let's talk about it.