Change the GetSelectedTab method of the NativeTabbedPaneView to return the tab contents.

ui/views/controls/tabbed_pane/tabbed_pane_unittest.cc

 // Copyright (c) 2012 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 "base/message_loop.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/utf_string_conversions.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/views/controls/tabbed_pane/tabbed_pane.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_delegate.h"
 
 namespace views {
 
 // A view for testing that takes a fixed preferred size upon construction.
 class FixedSizeView : public View {
  public:
   explicit FixedSizeView(const gfx::Size& size)
     : size_(size) {}
 
   // Overridden from View:
   virtual gfx::Size GetPreferredSize() OVERRIDE {
     return size_;
   }
 
  private:
   const gfx::Size size_;
 
   DISALLOW_COPY_AND_ASSIGN(FixedSizeView);
 };
 
-class TabbedPaneTest : public ViewsTestBase,
-                       public WidgetDelegate {
+#if defined(OS_WIN) && !defined(USE_AURA)
+class WidgetDelegateImpl : public WidgetDelegate {

[msw, 2012/07/26 19:29:30]

None of this Widget and contents view wrangling should be necessary. The default
WidgetDelegate::GetContentsView impl supplies a new View owned by the Widget,
which you can use via widget->GetContentsView()->AddChildView(tabbed_pane_).

If I'm wrong, please add a comment describing the purpose of this class.

[markusheintz_, 2012/07/26 21:33:52]

The default implementation of WidgetDelegate is an abstract class. The following
methods are pure virtual:
  virtual views::Widget* GetWidget() OVERRIDE;
  virtual const views::Widget* GetWidget() const OVERRIDE;


BTW:
If I add both tabbed panes to the same view then I get the following warning
when I run the test:
"[6356:6924:0726/231848:95945840:ERROR:window_impl.cc(55)] Failed to unregister
class Chrome_WidgetWin_0. Error = 1412"

I didn't yet figure out if this is an issue or not. Therefor I use two different
views.

[msw, 2012/07/26 22:19:05]

Try following the pattern in ui/views/view_unittest.cc where using void
Widget::Init(const InitParams& params) uses DefaultWidgetDelegate, or I suppose
you can try using DefaultWidgetDelegate directly.

Hmm, I'm not sure why adding the second TabbedPane view causes that error...
Debug it or try a quick and dirty fix like adding container view(s) to the
GetContentsView. That said, using the Widget in just the NTPW case seems fine,
since it's on the chopping block; but try to at least remove this
WidgetDelegateImpl.

+ public:
+  WidgetDelegateImpl(View* contents_view) : contents_view_(contents_view) {}
+  virtual ~WidgetDelegateImpl() {}
+
+  virtual views::View* GetContentsView() OVERRIDE {
+    return contents_view_.get();
+  }
+  virtual views::Widget* GetWidget() OVERRIDE {
+    return contents_view_->GetWidget();
+  }
+  virtual const views::Widget* GetWidget() const OVERRIDE {
+    return contents_view_->GetWidget();
+  }
+
+ private:
+  scoped_ptr<View> contents_view_;
+};
+#endif
+
+class TabbedPaneTest : public ViewsTestBase {
  public:
   TabbedPaneTest() {}
 
-  TabbedPane* tabbed_pane_;
+  void TestSizeAndLayout(TabbedPane* tabbed_pane);
+
+  void TestAddRemove(TabbedPane* tabbed_pane);
+
+  TabbedPane* tabbed_pane_;  // Owned by |tabbed_pane_container_|

[msw, 2012/07/26 19:29:30]

nit: append a period.

[markusheintz_, 2012/07/26 21:33:52]

Done.

+
+#if defined(OS_WIN) && !defined(USE_AURA)
+  TabbedPane* tabbed_pane_win_;  // Owned by the |window_|s |WidgetDelegate|.

[msw, 2012/07/26 19:29:30]

nit: "|window_|'s"

[markusheintz_, 2012/07/26 21:33:52]

Done.

+  Widget* window_;
+#endif
 
  private:
   virtual void SetUp() OVERRIDE {

[msw, 2012/07/26 19:29:30]

Move this definition out-of-line.

[markusheintz_, 2012/07/26 21:33:52]

Done.

     ViewsTestBase::SetUp();
     tabbed_pane_ = new TabbedPane();
-    tabbed_pane_->set_use_native_win_control(true);
-    window_ = Widget::CreateWindowWithBounds(this, gfx::Rect(0, 0, 100, 100));
+    // In order to proper initialize the |TabbedPane| it must be added to a

[msw, 2012/07/26 19:29:30]

nit: "properly" and "|TabbedPane|, "

[markusheintz_, 2012/07/26 21:33:52]

Done.

+    // parent view (see the ViewHierarchyChanged method of the |TabbedPane|.

[msw, 2012/07/26 19:29:30]

nit: close the parentheses.

[markusheintz_, 2012/07/26 21:33:52]

Done.

+    tabbed_pane_container_.reset(new View());
+    tabbed_pane_container_->AddChildView(tabbed_pane_);
+#if defined(OS_WIN) && !defined(USE_AURA)
+    tabbed_pane_win_ = new TabbedPane();
+    tabbed_pane_win_->set_use_native_win_control(true);
+    window_ = Widget::CreateWindowWithBounds(

[msw, 2012/07/26 19:29:30]

The Widget should be safe to use regardless of platform.

+        new WidgetDelegateImpl(tabbed_pane_win_), gfx::Rect(0, 0, 100, 100));
     window_->Show();
+#endif
   }
 
+#if defined(OS_WIN) && !defined(USE_AURA)
   virtual void TearDown() OVERRIDE {
     window_->Close();
     ViewsTestBase::TearDown();
   }
+#endif
 
-  virtual views::View* GetContentsView() OVERRIDE {
-    return tabbed_pane_;
-  }
-  virtual views::Widget* GetWidget() OVERRIDE {
-    return tabbed_pane_->GetWidget();
-  }
-  virtual const views::Widget* GetWidget() const OVERRIDE {
-    return tabbed_pane_->GetWidget();
-  }
-
-  Widget* window_;
+  scoped_ptr<View> tabbed_pane_container_;

[msw, 2012/07/26 19:29:30]

See the comment above about why this isn't necessary.
Remove the scoped_ptr.h include along with this.

 
   DISALLOW_COPY_AND_ASSIGN(TabbedPaneTest);
 };
 
-// Tests that TabbedPane::GetPreferredSize() and TabbedPane::Layout().
-TEST_F(TabbedPaneTest, SizeAndLayout) {
+void TabbedPaneTest::TestSizeAndLayout(TabbedPane* tabbed_pane) {
   View* child1 = new FixedSizeView(gfx::Size(20, 10));
-  tabbed_pane_->AddTab(ASCIIToUTF16("tab1"), child1);
+  tabbed_pane->AddTab(ASCIIToUTF16("tab1"), child1);
   View* child2 = new FixedSizeView(gfx::Size(5, 5));
-  tabbed_pane_->AddTab(ASCIIToUTF16("tab2"), child2);
-  tabbed_pane_->SelectTabAt(0);
+  tabbed_pane->AddTab(ASCIIToUTF16("tab2"), child2);
+  tabbed_pane->SelectTabAt(0);
 
   // Check that the preferred size is larger than the largest child.
-  gfx::Size pref(tabbed_pane_->GetPreferredSize());
-  EXPECT_GT(pref.width(), 20);
+  gfx::Size pref(tabbed_pane->GetPreferredSize());
+  // The |tabbed_pane_| has no border. Therefore it should be as wide as the
+  // widest tab.
+  EXPECT_GE(pref.width(), 20);
   EXPECT_GT(pref.height(), 10);
 
   // The bounds of our children should be smaller than the tabbed pane's bounds.
-  tabbed_pane_->SetBounds(0, 0, 100, 200);
+  tabbed_pane->SetBounds(0, 0, 100, 200);
   RunPendingMessages();
   gfx::Rect bounds(child1->bounds());
   EXPECT_GT(bounds.width(), 0);
-  EXPECT_LT(bounds.width(), 100);
+  // The |tabbed_pane_| has no border. Therefore the children should be as wide
+  // as the |tabbed_pane_|.
+  EXPECT_LE(bounds.width(), 100);
   EXPECT_GT(bounds.height(), 0);
   EXPECT_LT(bounds.height(), 200);
 
   // If we switch to the other tab, it should get assigned the same bounds.
-  tabbed_pane_->SelectTabAt(1);
+  tabbed_pane->SelectTabAt(1);
   EXPECT_EQ(bounds, child2->bounds());
 }
 
-TEST_F(TabbedPaneTest, AddRemove) {
+void TabbedPaneTest::TestAddRemove(TabbedPane* tabbed_pane) {
   View* tab0 = new View;
-  tabbed_pane_->AddTab(ASCIIToUTF16("tab0"), tab0);
-  EXPECT_EQ(tab0, tabbed_pane_->GetSelectedTab());
-  EXPECT_EQ(0, tabbed_pane_->GetSelectedTabIndex());
+  tabbed_pane->AddTab(ASCIIToUTF16("tab0"), tab0);
+  EXPECT_EQ(tab0, tabbed_pane->GetSelectedTab());
+  EXPECT_EQ(0, tabbed_pane->GetSelectedTabIndex());
 
   // Add more 3 tabs.
-  tabbed_pane_->AddTab(ASCIIToUTF16("tab1"), new View);
-  tabbed_pane_->AddTab(ASCIIToUTF16("tab2"), new View);
-  tabbed_pane_->AddTab(ASCIIToUTF16("tab3"), new View);
-  EXPECT_EQ(4, tabbed_pane_->GetTabCount());
+  tabbed_pane->AddTab(ASCIIToUTF16("tab1"), new View);
+  tabbed_pane->AddTab(ASCIIToUTF16("tab2"), new View);
+  tabbed_pane->AddTab(ASCIIToUTF16("tab3"), new View);
+  EXPECT_EQ(4, tabbed_pane->GetTabCount());
 
   // Note: AddTab() doesn't select a tab if the tabbed pane isn't empty.
 
   // Select the last one.
-  tabbed_pane_->SelectTabAt(tabbed_pane_->GetTabCount() - 1);
-  EXPECT_EQ(3, tabbed_pane_->GetSelectedTabIndex());
+  tabbed_pane->SelectTabAt(tabbed_pane->GetTabCount() - 1);
+  EXPECT_EQ(3, tabbed_pane->GetSelectedTabIndex());
 
   // Remove the last one.
-  delete tabbed_pane_->RemoveTabAtIndex(3);
-  EXPECT_EQ(3, tabbed_pane_->GetTabCount());
+  delete tabbed_pane->RemoveTabAtIndex(3);
+  EXPECT_EQ(3, tabbed_pane->GetTabCount());
 
   // After removing the last tab, check if the tabbed pane selected the previous
   // tab.
-  EXPECT_EQ(2, tabbed_pane_->GetSelectedTabIndex());
+  EXPECT_EQ(2, tabbed_pane->GetSelectedTabIndex());
 
-  tabbed_pane_->AddTabAtIndex(0, ASCIIToUTF16("tab4"), new View, true);
+  tabbed_pane->AddTabAtIndex(0, ASCIIToUTF16("tab4"), new View, true);
 
   // Assert that even adding a new tab, the tabbed pane doesn't change the
   // selection, i.e., it doesn't select the new one.
   // The last tab should remains selected, instead of the tab added at index 0.
-  EXPECT_EQ(3, tabbed_pane_->GetSelectedTabIndex());
+  EXPECT_EQ(3, tabbed_pane->GetSelectedTabIndex());
 
   // Now change the selected tab.
-  tabbed_pane_->SelectTabAt(1);
-  EXPECT_EQ(1, tabbed_pane_->GetSelectedTabIndex());
+  tabbed_pane->SelectTabAt(1);
+  EXPECT_EQ(1, tabbed_pane->GetSelectedTabIndex());
 
   // Remove the first one.
-  delete tabbed_pane_->RemoveTabAtIndex(0);
-  EXPECT_EQ(0, tabbed_pane_->GetSelectedTabIndex());
+  delete tabbed_pane->RemoveTabAtIndex(0);
+  EXPECT_EQ(0, tabbed_pane->GetSelectedTabIndex());
+}
+
+// Tests that TabbedPane::GetPreferredSize() and TabbedPane::Layout().
+TEST_F(TabbedPaneTest, SizeAndLayout) {
+  TestSizeAndLayout(tabbed_pane_);
+#if defined(OS_WIN) && !defined(USE_AURA)

[msw, 2012/07/26 19:29:30]

Please add a comment here (or maybe somewhere else?) that the test class
function TabbedPaneTest::TestSizeAndLayout should be restored to a normal test
once NTPW in deprecated. Ditto for AddRemove.

[markusheintz_, 2012/07/26 21:33:52]

Done.

+  TestSizeAndLayout(tabbed_pane_win_);
+#endif
+}
+
+TEST_F(TabbedPaneTest, AddRemove) {
+  TestAddRemove(tabbed_pane_);
+#if defined(OS_WIN) && !defined(USE_AURA)
+  TestAddRemove(tabbed_pane_win_);
+#endif
 }
 
 } // namespace views