Fix a ChromeFrame crash reported on the crash server while processing an acce...

Fix a ChromeFrame crash reported on the crash server while processing an accelerator message. The crash happens while invoking the
IBrowserService2::v_MayTranslateAccelerator function. It appears from the dump that there are cases in IE8 where this interface
is actually implemented, but this function entry is NULL in the vtable. In any case from the comments in the code this interface
is only implemented till IE7. 

Fixes bug http://code.google.com/p/chromium/issues/detail?id=25457

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

[amit, 2010-04-28 01:10:57]

LGTM,  This might be a good interface to ask for documentation about.
BTW, can't we directly check for bs2->v_MayTranslateAccelerator?

On Tue, Apr 27, 2010 at 4:01 PM, <ananta@chromium.org> wrote:

> Reviewers: amit,
>
> Description:
> Fix a ChromeFrame crash reported on the crash server while processing an
> accelerator message. The crash happens while invoking the
> IBrowserService2::v_MayTranslateAccelerator function. It appears from the
> dump
> that there are cases in IE8 where this interface
> is actually implemented, but this function entry is NULL in the vtable. In
> any
> case from the comments in the code this interface
> is only implemented till IE7.
>
> Fixes bug http://code.google.com/p/chromium/issues/detail?id=25457
>
> Bug=25457
>
>
> Please review this at http://codereview.chromium.org/1718019/show
>
> SVN Base: svn://chrome-svn/chrome/trunk/src/
>
> Affected files:
>  M     chrome_frame/chrome_frame_activex_base.h
>
>
> Index: chrome_frame/chrome_frame_activex_base.h
> ===================================================================
> --- chrome_frame/chrome_frame_activex_base.h    (revision 45481)
> +++ chrome_frame/chrome_frame_activex_base.h    (working copy)
> @@ -914,6 +914,7 @@
>   // sent to the out of proc chromium instance.
>   // Returns S_OK iff the accelerator was handled by the browser.
>   HRESULT AllowFrameToTranslateAccelerator(const MSG& msg) {
> +    static const int kMayTranslateAcceleratorOffset = 0x170;
>     // Although IBrowserService2 is officially deprecated, it's still alive
>     // and well in IE7 and earlier.  We have to use it here to correctly
> give
>     // the browser a chance to handle keyboard shortcuts.
> @@ -925,14 +926,18 @@
>     // owned by the out-of-proc chromium instance so IE doesn't have a
> chance to
>     // fall back on its default behavior.  Instead we give IE a chance to
>     // handle the shortcut here.
> -
>     MSG accel_message = msg;
>     accel_message.hwnd = ::GetParent(m_hWnd);
> -
>     HRESULT hr = S_FALSE;
>     ScopedComPtr<IBrowserService2> bs2;
> +    // The code below explicitly checks for whether the
> +    // IBrowserService2::v_MayTranslateAccelerator function is valid. On
> IE8
> +    // there is one vtable ieframe!c_ImpostorBrowserService2Vtbl where
> this
> +    // function entry is NULL which leads to a crash. We don't know under
> what
> +    // circumstances this vtable is actually used though.
>     if (S_OK == DoQueryService(SID_STopLevelBrowser, m_spInPlaceSite,
> -                               bs2.Receive()) && bs2.get()) {
> +                               bs2.Receive()) && bs2.get() &&
> +                               (bs2 + kMayTranslateAcceleratorOffset)) {
>       hr = bs2->v_MayTranslateAccelerator(&accel_message);
>     } else {
>       // IE8 doesn't support IBrowserService2 unless you enable a special,
>
>
>