Allow non-IE hosts to handle accelerators without having to

implement the huge IBrowserService2 interface.

BUG=none
TEST=none


Review URL: http://codereview.chromium.org/2847071

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@54550 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 69 insertions, 49 deletions

diff --git a/chrome_frame/chrome_active_document.cc b/chrome_frame/chrome_active_document.cc
index 132d529..49439d9 100644
--- a/chrome_frame/chrome_active_document.cc
+++ b/chrome_frame/chrome_active_document.cc
@@ -497,6 +497,18 @@ STDMETHODIMP ChromeActiveDocument::Next(BSTR* url, BSTR* policy,
   return S_OK;
 }
 
+HRESULT ChromeActiveDocument::GetInPlaceFrame(
+    IOleInPlaceFrame** in_place_frame) {
+  DCHECK(in_place_frame);
+  if (in_place_frame_) {
+    *in_place_frame = in_place_frame_.get();
+    (*in_place_frame)->AddRef();
+    return S_OK;
+  } else {
+    return S_FALSE;
+  }
+}
+
 HRESULT ChromeActiveDocument::IOleObject_SetClientSite(
     IOleClientSite* client_site) {
   if (client_site == NULL) {
@@ -515,7 +527,6 @@ HRESULT ChromeActiveDocument::IOleObject_SetClientSite(
       doc_host_handler->HideUI();
 
     doc_site_.Release();
-    in_place_frame_.Release();
   }
 
   if (client_site != m_spClientSite)
diff --git a/chrome_frame/chrome_active_document.h b/chrome_frame/chrome_active_document.h
index 25d49d6..8b5f5e2 100644
--- a/chrome_frame/chrome_active_document.h
+++ b/chrome_frame/chrome_active_document.h
@@ -351,6 +351,9 @@ END_EXEC_COMMAND_MAP()
   STDMETHOD(GetPrivacyImpacted)(BOOL* privacy_impacted);
   STDMETHOD(Next)(BSTR* url, BSTR* policy, LONG* reserved, DWORD* flags);
 
+  // Accessor for InPlaceMenu.  Returns S_OK if set, S_FALSE if NULL.
+  HRESULT GetInPlaceFrame(IOleInPlaceFrame** in_place_frame);
+
  protected:
   // ChromeFrameActivexBase overrides
   virtual void OnOpenURL(int tab_handle, const GURL& url_to_open,
@@ -461,7 +464,6 @@ END_EXEC_COMMAND_MAP()
   // Dimensions of the window. Used only when opening popups.
   gfx::Rect dimensions_;
  public:
-  ScopedComPtr<IOleInPlaceFrame> in_place_frame_;
   OLEINPLACEFRAMEINFO frame_info_;
 };
 
diff --git a/chrome_frame/chrome_frame_activex_base.h b/chrome_frame/chrome_frame_activex_base.h
index 50029b7..c5d17d4 100644
--- a/chrome_frame/chrome_frame_activex_base.h
+++ b/chrome_frame/chrome_frame_activex_base.h
@@ -174,7 +174,8 @@ class ATL_NO_VTABLE ChromeFrameActivexBase :  // NOLINT
 
  public:
   ChromeFrameActivexBase()
-      : ready_state_(READYSTATE_UNINITIALIZED) {
+      : ready_state_(READYSTATE_UNINITIALIZED),
+        failed_to_fetch_in_place_frame_(false) {
     m_bWindowOnly = TRUE;
     url_fetcher_.set_container(static_cast<IDispatch*>(this));
   }
@@ -313,6 +314,10 @@ END_MSG_MAP()
       doc_site_.QueryFrom(client_site);
     }
 
+    if (client_site == NULL) {
+      in_place_frame_.Release();
+    }
+
     return CComControlBase::IOleObject_SetClientSite(client_site);
   }
 
@@ -988,15 +993,40 @@ END_MSG_MAP()
     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() &&
-                               *(*(reinterpret_cast<void***>(bs2.get())) +
-                                   kMayTranslateAcceleratorOffset)) {
+
+    // For non-IE containers, we use the standard IOleInPlaceFrame contract
+    // (which IE does not support). For IE, we try to use IBrowserService2,
+    // but need special handling for IE8 (see below).
+    //
+    // We try to cache an IOleInPlaceFrame for our site.  If we fail, we don't
+    // retry, and we fall back to the IBrowserService2 and PostMessage
+    // approaches below.
+    if (!in_place_frame_ && !failed_to_fetch_in_place_frame_) {
+      ScopedComPtr<IOleInPlaceUIWindow> dummy_ui_window;
+      RECT dummy_pos_rect = {0};
+      RECT dummy_clip_rect = {0};
+      OLEINPLACEFRAMEINFO dummy_frame_info = {0};
+      if (FAILED(m_spInPlaceSite->GetWindowContext(in_place_frame_.Receive(),
+                                                   dummy_ui_window.Receive(),
+                                                   &dummy_pos_rect,
+                                                   &dummy_clip_rect,
+                                                   &dummy_frame_info))) {
+        failed_to_fetch_in_place_frame_ = true;
+      }
+    }
+
+    // The IBrowserService2 code below (second conditional) 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 (in_place_frame_) {
+      hr = in_place_frame_->TranslateAccelerator(&accel_message, 0);
+    } else if (S_OK == DoQueryService(
+        SID_STopLevelBrowser, m_spInPlaceSite,
+        bs2.Receive()) && bs2.get() &&
+        *(*(reinterpret_cast<void***>(bs2.get())) +
+        kMayTranslateAcceleratorOffset)) {
       hr = bs2->v_MayTranslateAccelerator(&accel_message);
     } else {
       // IE8 doesn't support IBrowserService2 unless you enable a special,
@@ -1017,10 +1047,11 @@ END_MSG_MAP()
       // ieframe.dll.  I checked this by scanning for the address of
       // those functions inside the dll and found none, which means that
       // all calls to those functions are relative.
-      // So, for IE8, our approach is very simple.  Just post the message
-      // to our parent window and IE will pick it up if it's an
-      // accelerator. We won't know for sure if the browser handled the
-      // keystroke or not.
+      // So, for IE8 in certain cases, and for other containers that may
+      // support neither IOleInPlaceFrame or IBrowserService2 our approach
+      // is very simple.  Just post the message to our parent window and IE
+      // will pick it up if it's an accelerator. We won't know for sure if
+      // the browser handled the keystroke or not.
       ::PostMessage(accel_message.hwnd, accel_message.message,
                     accel_message.wParam, accel_message.lParam);
     }
@@ -1167,6 +1198,11 @@ END_MSG_MAP()
   ScopedBstr url_;
   ScopedComPtr<IOleDocumentSite> doc_site_;
 
+  // If false, we tried but failed to fetch an IOleInPlaceFrame from our host.
+  // Cached here so we don't try to fetch it every time if we keep failing.
+  bool failed_to_fetch_in_place_frame_;
+  ScopedComPtr<IOleInPlaceFrame> in_place_frame_;
+
   // For more information on the ready_state_ property see:
   // http://msdn.microsoft.com/en-us/library/aa768179(VS.85).aspx#
   READYSTATE ready_state_;
diff --git a/chrome_frame/in_place_menu.h b/chrome_frame/in_place_menu.h
index 9742896..d6b368a 100644
--- a/chrome_frame/in_place_menu.h
+++ b/chrome_frame/in_place_menu.h
@@ -1,31 +1,6 @@
-// Copyright 2008, Google Inc.
-// All rights reserved.
-//
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are
-// met:
-//
-//    * Redistributions of source code must retain the above copyright
-// notice, this list of conditions and the following disclaimer.
-//    * Redistributions in binary form must reproduce the above
-// copyright notice, this list of conditions and the following disclaimer
-// in the documentation and/or other materials provided with the
-// distribution.
-//    * Neither the name of Google Inc. nor the names of its
-// contributors may be used to endorse or promote products derived from
-// this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
 
 #ifndef CHROME_FRAME_IN_PLACE_MENU_H_
 #define CHROME_FRAME_IN_PLACE_MENU_H_
@@ -197,7 +172,7 @@ class InPlaceMenu {
     }
     T* t = static_cast<T*>(this);
     HRESULT hr = E_FAIL;
-    if (!t->in_place_frame_) {
+    if (S_OK != t->GetInPlaceFrame(in_place_frame)) {
       // We weren't given an IOleInPlaceFrame pointer, so
       // we'll have to get it ourselves.
       if (t->m_spInPlaceSite) {
@@ -210,10 +185,6 @@ class InPlaceMenu {
                                                   &position_rect, &clip_rect,
                                                   &t->frame_info_);
       }
-    } else {
-      *in_place_frame = t->in_place_frame_;
-      (*in_place_frame)->AddRef();
-      hr = S_OK;
     }
     return hr;
   }