Fix Panel to always detect when a tab is added.

chrome/browser/ui/panels/panel.cc

 // Copyright (c) 2011 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/browser/ui/panels/panel.h"
 
 #include "base/logging.h"
 #include "chrome/browser/extensions/extension_prefs.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
-#include "content/browser/renderer_host/render_view_host.h"
+#include "chrome/browser/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/panels/native_panel.h"
 #include "chrome/browser/ui/panels/panel_manager.h"
+#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
 #include "chrome/browser/ui/window_sizer.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/extensions/extension.h"
+#include "content/browser/renderer_host/render_view_host.h"
 #include "content/browser/tab_contents/tab_contents.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
 #include "ui/gfx/rect.h"
 
 // static
 const Extension* Panel::GetExtensionFromBrowser(Browser* browser) {
   // Find the extension. When we create a panel from an extension, the extension
   // ID is passed as the app name to the Browser.
   ExtensionService* extension_service =
       browser->GetProfile()->GetExtensionService();
   return extension_service->GetExtensionById(
       web_app::GetExtensionIdFromApplicationName(browser->app_name()), false);
 }
 
 Panel::Panel(Browser* browser, const gfx::Rect& bounds)
     : min_size_(bounds.size()),
       max_size_(bounds.size()),
       native_panel_(NULL),
       expansion_state_(EXPANDED),
       restored_height_(bounds.height()) {
   native_panel_ = CreateNativePanel(browser, this, bounds);
-
-  registrar_.Add(this,
-                 content::NOTIFICATION_TAB_ADDED,
-                 content::Source<TabContentsDelegate>(browser));
+  browser->tabstrip_model()->AddObserver(this);
 }
 
 Panel::~Panel() {
   // Invoked by native panel so do not access native_panel_ here.

[jianli, 2011/11/11 22:31:23]

Could you please consider adding "destructor" after "native panel" to make this
comment more clear?

[jennb, 2011/11/11 22:42:50]

Done.

 }
 
+void Panel::OnNativePanelClosed() {
+  native_panel_->GetPanelBrowser()->tabstrip_model()->RemoveObserver(this);
+  manager()->Remove(this);
+}
+
 PanelManager* Panel::manager() const {
   return PanelManager::GetInstance();
 }
 
-
 const Extension* Panel::GetExtension() const {
   return GetExtensionFromBrowser(browser());
 }
 
 void Panel::SetPanelBounds(const gfx::Rect& bounds) {
   if (expansion_state_ == Panel::EXPANDED)
     restored_height_ = bounds.height();
 
   native_panel_->SetPanelBounds(bounds);
 }
 
 void Panel::SetMaxSize(const gfx::Size& max_size) {
   if (max_size_ == max_size)
     return;
   max_size_ = max_size;
 
   // Note: |render_view_host| might be NULL if the tab has not been created.
-  // If so, we will do it when NOTIFICATION_TAB_ADDED is received.
+  // If so, we will do it when TabInsertedAt() is invoked.
   RenderViewHost* render_view_host = GetRenderViewHost();
   if (render_view_host)
     RequestRenderViewHostToDisableScrollbars(render_view_host);
 }
 
 void Panel::SetExpansionState(ExpansionState new_state) {
   if (expansion_state_ == new_state)
     return;
 
   ExpansionState old_state = expansion_state_;
@@ skipping 76 matching lines @@
 
 void Panel::SetBounds(const gfx::Rect& bounds) {
   // Ignore any SetBounds requests since the bounds are completely controlled
   // by panel manager.
 }
 
 // Close() may be called multiple times if the browser window is not ready to
 // close on the first attempt.
 void Panel::Close() {
   native_panel_->ClosePanel();
-
-// TODO(dimich): Only implemented fully async on Mac. Need to update other
-// platforms. The panel should be removed from PanelManager when and if it
-// was actually closed. The closing can be cancelled because of onbeforeunload
-// handler on the web page. http://crbug.com/102720
-#if !defined(OS_MACOSX)
-  manager()->Remove(this);
-#endif
 }
 
 void Panel::Activate() {
   // Make sure the panel is expanded when activated programmatically,
   // so the user input does not go into collapsed window.
   SetExpansionState(Panel::EXPANDED);
   native_panel_->ActivatePanel();
 }
 
 void Panel::Deactivate() {
@@ skipping 347 matching lines @@
 void Panel::ShowAvatarBubble(TabContents* tab_contents, const gfx::Rect& rect) {
   // Panels will never show a new tab page so this should never be called.
   NOTREACHED();
 }
 
 void Panel::ShowAvatarBubbleFromAvatarButton() {
   // Panels will never show an avatar button so this should never be called.
   NOTREACHED();
 }
 
+void Panel::TabInsertedAt(TabContentsWrapper* contents,
+                   int index,

[jianli, 2011/11/11 22:31:23]

Spacing.

[jennb, 2011/11/11 22:42:50]

Done.

+                   bool foreground) {
+  DCHECK_EQ(0, index);
+  TabContents* tab_contents = contents->tab_contents();
+  EnableAutoResize(tab_contents->render_view_host());
+
+  // We also need to know when the render view host changes in order
+  // to turn on preferred size changed notifications in the new
+  // render view host.
+  registrar_.RemoveAll();

[jianli, 2011/11/11 22:31:23]

Why do we need to call RemoveAll first? Can TabInsertedAt be called multiple
times?

[jennb, 2011/11/11 22:42:50]

When there's a new tab, I don't want notifications about the old tab anymore.

+  registrar_.Add(
+      this,
+      content::NOTIFICATION_TAB_CONTENTS_SWAPPED,
+      content::Source<TabContents>(tab_contents));
+}
+
 void Panel::Observe(int type,
                     const content::NotificationSource& source,
                     const content::NotificationDetails& details) {
-  switch (type) {
-    case content::NOTIFICATION_TAB_ADDED:
-      // We also need to know when the render view host changes in order
-      // to turn on preferred size changed notifications in the new
-      // render view host. However, we cannot register for
-      // NOTIFICATION_TAB_CONTENTS_SWAPPED until we actually have a
-      // tab content so we register for it here.
-      registrar_.Add(
-          this,
-          content::NOTIFICATION_TAB_CONTENTS_SWAPPED,
-          content::Source<TabContents>(browser()->GetSelectedTabContents()));
-      // Fall-thru to update render view host.
-
-    case content::NOTIFICATION_TAB_CONTENTS_SWAPPED: {
-      RenderViewHost* render_view_host = GetRenderViewHost();
-      if (render_view_host) {
-        render_view_host->EnablePreferredSizeMode(
-            kPreferredSizeWidth | kPreferredSizeHeightThisIsSlow);
-        RequestRenderViewHostToDisableScrollbars(render_view_host);
-      }
-      break;
-    }
-    default:
-      NOTREACHED() << "Got a notification we didn't register for!";
-      break;
-  }
+  DCHECK_EQ(type, content::NOTIFICATION_TAB_CONTENTS_SWAPPED);
+  RenderViewHost* render_view_host =
+      content::Source<TabContents>(source).ptr()->render_view_host();
+  if (render_view_host)
+    EnableAutoResize(render_view_host);
 }
 
 RenderViewHost* Panel::GetRenderViewHost() const {
   TabContents* tab_contents = browser()->GetSelectedTabContents();
   if (!tab_contents)
     return NULL;
   return tab_contents->render_view_host();
 }
 
+void Panel::EnableAutoResize(RenderViewHost* render_view_host) {
+  DCHECK(render_view_host);
+  render_view_host->EnablePreferredSizeMode(
+      kPreferredSizeWidth | kPreferredSizeHeightThisIsSlow);
+  RequestRenderViewHostToDisableScrollbars(render_view_host);
+}
+
 void Panel::RequestRenderViewHostToDisableScrollbars(
     RenderViewHost* render_view_host) {
-  if (render_view_host) {
-    render_view_host->DisableScrollbarsForThreshold(
-        native_panel_->ContentSizeFromWindowSize(max_size_));
-  }
+  DCHECK(render_view_host);
+  render_view_host->DisableScrollbarsForThreshold(
+      native_panel_->ContentSizeFromWindowSize(max_size_));
 }
 
 void Panel::OnWindowSizeAvailable() {
-  RequestRenderViewHostToDisableScrollbars(GetRenderViewHost());
+  RenderViewHost* render_view_host = GetRenderViewHost();
+  if (render_view_host)
+    RequestRenderViewHostToDisableScrollbars(render_view_host);
 }
 
 Browser* Panel::browser() const {
   return native_panel_->GetPanelBrowser();
 }
 
 void Panel::DestroyBrowser() {
   native_panel_->DestroyPanelBrowser();
 }