Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(365)

Issue 244002: This fixes a crash in IE8 with ChromeFrame when a new tab was created. ... (Closed)

Created:
11 years, 2 months ago by ananta
Modified:
9 years, 6 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

This fixes a crash in IE8 with ChromeFrame when a new tab was created. ChromeFrame VTable patches the IInternetProtocol interface for the CLSID_HttpProtocol and CLSID_HttpSProtocol handlers. However we were using the same VTable information to patch both the handlers essentially overwriting the first one. While this all worked purely by chance, it exposed a bug in IE8 where every new tab initially goes into a new process and if the chromeframe is unloaded we would leave behind an IInternetProtocol interface in urlmon patched, which would crash when dereferenced. Added a check in the VTable patching code for this case. This fixes bug http://code.google.com/p/chromium/issues/detail?id=22768 Bug=22768 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=27191

Patch Set 1 #

Total comments: 8

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Total comments: 2

Patch Set 5 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+120 lines, -30 lines) Patch
MM chrome_frame/bho.cc View 1 2 3 chunks +2 lines, -7 lines 0 comments Download
MM chrome_frame/protocol_sink_wrap.h View 1 2 3 4 4 chunks +23 lines, -3 lines 0 comments Download
MM chrome_frame/protocol_sink_wrap.cc View 1 2 3 4 6 chunks +85 lines, -19 lines 0 comments Download
MM chrome_frame/vtable_patch_manager.cc View 2 chunks +10 lines, -1 line 0 comments Download

Messages

Total messages: 8 (0 generated)
ananta
11 years, 2 months ago (2009-09-25 04:55:04 UTC) #1
stoyan
Please add some DCHECK(it->stub_ == NULL) in the loop in PatchInterfaceMethods in vtable_patch_manager.cc, so we ...
11 years, 2 months ago (2009-09-25 05:01:15 UTC) #2
ananta
On 2009/09/25 05:01:15, stoyan wrote: > Please add some DCHECK(it->stub_ == NULL) in the loop ...
11 years, 2 months ago (2009-09-25 05:07:43 UTC) #3
amit
Amazing find! Glad that you tracked this down. Basically we were incorrectly using vtable patch ...
11 years, 2 months ago (2009-09-25 05:13:44 UTC) #4
tommi (sloooow) - chröme
lgtm! nice find
11 years, 2 months ago (2009-09-25 05:24:46 UTC) #5
ananta
http://codereview.chromium.org/244002/diff/1/4 File chrome_frame/bho.cc (right): http://codereview.chromium.org/244002/diff/1/4#newcode219 Line 219: ProtocolSinkWrap::PatchProtocolHandler(); On 2009/09/25 05:13:44, amit wrote: > PatchProtocolHandlers()? ...
11 years, 2 months ago (2009-09-25 05:37:15 UTC) #6
amit
lgtm++ http://codereview.chromium.org/244002/diff/6/1008 File chrome_frame/protocol_sink_wrap.cc (right): http://codereview.chromium.org/244002/diff/6/1008#newcode82 Line 82: HRESULT hr = CreateProtocolHandlerInstance(CLSID_HttpProtocol, how about moving ...
11 years, 2 months ago (2009-09-25 05:46:11 UTC) #7
ananta
11 years, 2 months ago (2009-09-25 05:52:51 UTC) #8
http://codereview.chromium.org/244002/diff/6/1008
File chrome_frame/protocol_sink_wrap.cc (right):

http://codereview.chromium.org/244002/diff/6/1008#newcode82
Line 82: HRESULT hr = CreateProtocolHandlerInstance(CLSID_HttpProtocol,
On 2009/09/25 05:46:11, amit wrote:
> how about moving this into PatchProtocolMethods?

Done.

Powered by Google App Engine
This is Rietveld 408576698