Preview CL for adding an Infobar facility to Google Chrome Frame....

chrome_frame/infobars/internal/host_window.cc

Index: chrome_frame/infobars/internal/host_window.cc
===================================================================
--- chrome_frame/infobars/internal/host_window.cc	(revision 0)
+++ chrome_frame/infobars/internal/host_window.cc	(revision 0)
@@ -0,0 +1,160 @@
+// 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.
+
+#include "chrome_frame/infobars/internal/host_window.h"
+
+#include "chrome_frame/infobars/internal/displaced_window.h"
+#include "chrome_frame/utils.h"
+
+namespace {
+
+const wchar_t kIeTabContentParentWindowClass[] = L"Shell DocObject View";
+
+}  // namespace
+
+class HostWindowManager::DisplacedWindowDelegate
+    : public DisplacedWindowManager::Delegate {
+ public:
+  explicit DisplacedWindowDelegate(HostWindowManager* manager);
+  void AdjustDisplacedWindowDimensions(RECT *rect);
+  void OnDisplacedWindowDestroyed();
+ private:
+  HostWindowManager* manager_;
+  DISALLOW_COPY_AND_ASSIGN(DisplacedWindowDelegate);
+};
+
+
+// Callback function for EnumChildWindows (looks for a window with class
+// kIeTabContentParentWindowClass).
+//
+// lparam must point to an HWND that is either NULL or the HWND of the displaced
+// window that is being destroyed. We will ignore that window if we come across
+// it, and update lparam to point to the new displaced window if it is found.
+static BOOL CALLBACK FindDisplacedWindowProc(HWND hwnd, LPARAM lparam) {
+  DCHECK(lparam != NULL);
+  HWND* window_handle = reinterpret_cast<HWND*>(lparam);
+
+  if (hwnd == *window_handle)
+    return TRUE;  // Skip this, it's the old displaced window.
+
+// Variable to hold the class name. The size does not matter as long as it

[grt (UTC plus 2), 2010/11/25 18:00:13]

indent

[erikwright (departed), 2010/12/01 20:05:52]

Done.

+  // is at least can hold kIeTabContentParentWindowClass.
+  wchar_t class_name[100];
+  if (::GetClassName(hwnd, class_name, arraysize(class_name)) &&
+      lstrcmpi(kIeTabContentParentWindowClass, class_name) == 0) {
+    // We found the window. Return its handle and stop enumeration.
+    *window_handle = hwnd;
+    return FALSE;
+  }
+  return TRUE;
+}
+
+HostWindowManager::Delegate* HostWindowManager::GetDelegateForHostHwnd(
+    HWND host_window) {
+  LRESULT result = ::SendMessage(host_window, WM_GET_DELEGATE, NULL, NULL);
+  return reinterpret_cast<Delegate*>(result);
+}
+
+HostWindowManager::HostWindowManager() : displaced_window_manager_(NULL) {
+}
+
+HostWindowManager::~HostWindowManager() {
+  // If we are holding a displaced_window_manager_, it means that
+  // OnDisplacedWindowManagerDestroyed has not been called yet, and therefore
+  // our DisplacedWindowDelegate might still be around, ready to invoke us.
+  // Fail fast to prevent a call into lala-land.
+  CHECK(displaced_window_manager_ == NULL);

[tommi (sloooow) - chröme, 2010/11/24 16:08:24]

to be clear - CHECK() will kill IE for the user if this ever hits - you sure
that's intended?

[erikwright (departed), 2010/12/01 20:05:52]

I think we have to. The alternative is to accept a call into an already
destroyed object, which is a crash in the best case and an exploit in the worst.

+}
+
+bool HostWindowManager::Initialize(HWND host_window, Delegate* delegate) {
+  DCHECK(delegate != NULL);
+  DCHECK(delegate_ == NULL);
+  delegate_.reset(delegate);
+
+  PinModule();

[grt (UTC plus 2), 2010/11/25 18:00:13]

Are both calls to PinModule (here and in DisplacedWindowManager) needed?  Should
PinModule only happen if the subclass succeeds?

[erikwright (departed), 2010/12/01 20:05:52]

Only one is required, but for the purposes of decoupling, I prefer for the
DisplacedWindowManager to not know that the HostWindowManager has also done
this.

Agreed, moving it after SubclassWindow success makes sense.

Relevant code moved to subclassing_window.h

+  if (!SubclassWindow(host_window)) {
+    LOG(DFATAL) << "Failed to subclass HWND for infobar installation.";
+    return false;
+  }
+
+  return true;
+}
+
+void HostWindowManager::OnFinalMessage(HWND /*hWnd*/) {

[tommi (sloooow) - chröme, 2010/11/24 16:08:24]

remove /* */

[erikwright (departed), 2010/12/01 20:05:52]

According to coding style we comment parameter names if unused:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...

+  delete this;
+}
+
+LRESULT HostWindowManager::OnGetDelegate(UINT message,
+                            WPARAM wparam,

[tommi (sloooow) - chröme, 2010/11/24 16:08:24]

fix indentation

[erikwright (departed), 2010/12/01 20:05:52]

Done.

+                            LPARAM lparam,
+                            BOOL& handled) {
+  return reinterpret_cast<LRESULT>(delegate_.get());
+}
+
+bool HostWindowManager::UpdateLayout() {
+  if (FindDisplacedWindow(NULL)) {
+    displaced_window_manager_->UpdateLayout();
+    return true;
+  } else {
+    return false;
+  }
+}
+
+bool HostWindowManager::FindDisplacedWindow(HWND old_window) {
+  if (displaced_window_manager_ == NULL ||
+      *displaced_window_manager_ == old_window) {
+    if (IsWindow()) {
+      // Find the window which is the container for the HTML view (parent of
+      // the content). When the displaced window is destroyed, the new one might
+      // already exist, so we say "find a displaced window that is not this
+      // (old) one".
+      HWND displaced_window = old_window;
+      ::EnumChildWindows(*this, FindDisplacedWindowProc,
+                         reinterpret_cast<LPARAM>(&displaced_window));
+
+      if (displaced_window == old_window) {
+        LOG(DFATAL) << "Failed to locate IE renderer HWND to displace for "
+                    << "Infobar installation.";
+      } else {
+        scoped_ptr<DisplacedWindowManager> displaced_window_manager(
+            new DisplacedWindowManager());
+        if (displaced_window_manager->Initialize(
+                displaced_window, new DisplacedWindowDelegate(this))) {
+          displaced_window_manager_ = displaced_window_manager.release();
+        }
+      }
+    }
+  }
+
+  return displaced_window_manager_ != NULL;
+}
+
+void HostWindowManager::AdjustDisplacedWindowDimensions(RECT* rect) {
+  if (rect == NULL)

[grt (UTC plus 2), 2010/11/25 18:00:13]

why not simply:
if (rect != NULL)
  delegate_->AdjustDisplacedWindowDimensions(rect);
?

[erikwright (departed), 2010/12/01 20:05:52]

In fact, I think it's clear from the contract that the pointer should not be
null. I added a check on the surface of this call-path
(DisplacedWindowManager::OnNcCalcSize) to make sure it's not NULL there. There
is also a DCHECK down in InfobarWindow::ReserveSpace (where the pointer is
actually de-referenced).

Moved up to the top of this file, FYI.

+    return;
+  delegate_->AdjustDisplacedWindowDimensions(rect);
+}
+
+void HostWindowManager::OnDisplacedWindowDestroyed() {
+  HWND old_window = *displaced_window_manager_;
+  // Will be deleted in its OnFinalMessage
+  displaced_window_manager_ = NULL;
+
+  // Check to see if a new window has already been created.
+  if (FindDisplacedWindow(old_window))
+    UpdateLayout();
+}
+
+HostWindowManager::DisplacedWindowDelegate::
+    DisplacedWindowDelegate(HostWindowManager* manager) : manager_(manager) {
+}
+
+void HostWindowManager::DisplacedWindowDelegate::
+    AdjustDisplacedWindowDimensions(RECT *rect) {
+  manager_->AdjustDisplacedWindowDimensions(rect);
+}
+
+void HostWindowManager::DisplacedWindowDelegate::OnDisplacedWindowDestroyed() {
+  manager_->OnDisplacedWindowDestroyed();
+}
Property changes on: chrome_frame\infobars\internal\host_window.cc
___________________________________________________________________
Added: svn:eol-style
   + LF