Update Chrome Upstream flow to reflect new UI mocks

ui/views/bubble/bubble_frame_view_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 "ui/views/bubble/bubble_frame_view.h"
 
 #include <memory>
 
 #include "base/macros.h"
 #include "build/build_config.h"
@@ skipping 124 matching lines @@
   gfx::Insets insets = frame.bubble_border()->GetInsets();
   EXPECT_EQ(insets.left() + margin_x, frame.GetBoundsForClientView().x());
   EXPECT_EQ(insets.top() + margin_y, frame.GetBoundsForClientView().y());
 }
 
 TEST_F(BubbleFrameViewTest, RemoveFootnoteView) {
   TestBubbleFrameView frame(this);
   EXPECT_EQ(nullptr, frame.footnote_container_);
   View* footnote_dummy_view = new StaticSizedView(gfx::Size(200, 200));
   frame.SetFootnoteView(footnote_dummy_view);
-  EXPECT_EQ(footnote_dummy_view->parent(), frame.footnote_container_);
+  EXPECT_EQ(footnote_dummy_view->parent(), (View*)frame.footnote_container_);

[msw, 2017/07/05 20:45:30]

nit: this cast shouldn't be needed or should use static_cast.

[Jared Saul, 2017/07/07 20:12:28]

Yep; it's good now that I reverted it to a View*, thanks.

   View* container_view = footnote_dummy_view->parent();
   delete footnote_dummy_view;
   footnote_dummy_view = nullptr;
   EXPECT_FALSE(container_view->visible());
   EXPECT_EQ(nullptr, frame.footnote_container_);
 }
 
 TEST_F(BubbleFrameViewTest, GetBoundsForClientViewWithClose) {
   TestBubbleFrameView frame(this);
   // TestBubbleFrameView::GetWidget() is responsible for creating the widget and
   // widget delegate at first call, so it is called here for that side-effect.
   ignore_result(frame.GetWidget());
   frame.widget_delegate()->SetShouldShowCloseButton(true);
   frame.ResetWindowControls();
   EXPECT_EQ(kArrow, frame.bubble_border()->arrow());
   EXPECT_EQ(kColor, frame.bubble_border()->background_color());
 
   gfx::Insets frame_insets = frame.GetInsets();
   gfx::Insets border_insets = frame.bubble_border()->GetInsets();
   EXPECT_EQ(border_insets.left() + frame_insets.left(),
             frame.GetBoundsForClientView().x());
   EXPECT_EQ(border_insets.top() + frame_insets.top(),
             frame.GetBoundsForClientView().y());
 }
 
+TEST_F(BubbleFrameViewTest,
+       FootnoteContainerViewShouldMatchVisibilityOfFirstChild) {
+  TestBubbleFrameView frame(this);
+  View* footnote_dummy_view = new StaticSizedView(gfx::Size(200, 200));
+  footnote_dummy_view->SetVisible(false);
+  frame.SetFootnoteView(footnote_dummy_view);
+  View* footnote_container_view = (View*)frame.footnote_container_;

[msw, 2017/07/05 20:45:30]

nit: this cast shouldn't be needed or should use static_cast.

[Jared Saul, 2017/07/07 20:12:28]

Done.

+  EXPECT_EQ(footnote_dummy_view->parent(), footnote_container_view);

[msw, 2017/07/05 20:45:30]

Please just retrieve the footnote's container via footnote_dummy_view->parent()
and revert your bubble_frame_view.h changes: make |footnote_container_| a
views::View, remove the FootnoteContainerView forward decl in the header, remove
the friend test, and make the FootnoteContainerView class local to
bubble_frame_view.cc  file (in an anon namespace).

[Jared Saul, 2017/07/07 20:12:28]

Done; thank you for the detailed instructions!

+  EXPECT_FALSE(footnote_container_view->visible());
+  footnote_dummy_view->SetVisible(true);
+  EXPECT_TRUE(footnote_container_view->visible());
+  footnote_dummy_view->SetVisible(false);
+  EXPECT_FALSE(footnote_container_view->visible());
+}

[tapted, 2017/07/06 01:11:59]

the visibility test is good.. 

I think I see the tricky part. For something more compelling, we'd probably want
access to BubbleDialogDelegateView::SizeToContents(), but there's no
BubbleDialogDelegateView in BubbleFrameViewTest.

But we still have access to GetPreferredSize -- there's even a test for that
(see below)

+
 // Tests that the arrow is mirrored as needed to better fit the screen.
 TEST_F(BubbleFrameViewTest, GetUpdatedWindowBounds) {
   TestBubbleFrameView frame(this);
   gfx::Rect window_bounds;
 
   gfx::Insets insets = frame.bubble_border()->GetInsets();
   int xposition = 95 - insets.width();
 
   // Test that the info bubble displays normally when it fits.
   frame.bubble_border()->set_arrow(BubbleBorder::TOP_LEFT);
@@ skipping 285 matching lines @@
 
 TEST_F(BubbleFrameViewTest, GetPreferredSize) {
   TestBubbleFrameView frame(this);
   gfx::Rect preferred_rect(frame.GetPreferredSize());
   // Expect that a border has been added to the preferred size.
   preferred_rect.Inset(frame.bubble_border()->GetInsets());
 
   gfx::Size expected_size(kPreferredClientWidth + kExpectedAdditionalWidth,
                           kPreferredClientHeight + kExpectedAdditionalHeight);
   EXPECT_EQ(expected_size, preferred_rect.size());
 }

[tapted, 2017/07/06 01:11:59]

I'd add some lines here like,

// Adding a footnote should increase the preferred size, but only when the
// footnote is visible.
constexpr int kFootnoteHeight = 20;
const gfx::Size no_footnote_size = expected_size;
View* footnote = new StaticSizedView(gfx::Size(10, kFootnoteHeight));
footnote->SetVisible(false);
frame.AddFootnoteView(footnote);
EXPECT_EQ(no_footnote_size, frame.GetPreferredSize());  // No change.

footnote->Setvisible(true);
expected_size.Enlarge(0, kFootnoteHeight + frame.content_margins().height());
EXPECT_EQ(expected_size, frame.GetPreferredSize());

footnote->Setvisible(false);
EXPECT_EQ(no_footnote_size, frame.GetPreferredSize()); 



(haven't tested those, but that's probably the right gist)

[Jared Saul, 2017/07/07 20:12:28]

Thanks for the code!  There was about a 10x17-18 px area it wasn't account for
that I figure had to do with the borders, so in order to keep it testing
GetPreferredSize I split it into two parts.

However, there's still a 1px different in height when the footnote is added
(GetPreferredSize() is 312 instead of 311).  Do you know off the top of your
head where this might be coming from?

[tapted, 2017/07/10 03:52:04]

Probably the line in SetFootnoteView:

  footnote_container_->SetBorder(
      CreateSolidSidedBorder(1, 0, 0, 0, kFootnoteBorderColor));

[Jared Saul, 2017/07/10 19:24:40]

Hah, I skimmed right over it.  Thanks!

 
 TEST_F(BubbleFrameViewTest, GetMinimumSize) {
   TestBubbleFrameView frame(this);
   gfx::Rect minimum_rect(frame.GetMinimumSize());
   // Expect that a border has been added to the minimum size.
   minimum_rect.Inset(frame.bubble_border()->GetInsets());
 
   gfx::Size expected_size(kMinimumClientWidth + kExpectedAdditionalWidth,
                           kMinimumClientHeight + kExpectedAdditionalHeight);
   EXPECT_EQ(expected_size, minimum_rect.size());
@@ skipping 87 matching lines @@
   // If the DialogDelegate asks not to snap, it should not snap.
   delegate.set_override_snap(false);
   Widget* w2 = BubbleDialogDelegateView::CreateBubble(&delegate);
   w2->Show();
   EXPECT_EQ(delegate.GetPreferredSize().width(),
             w2->GetWindowBoundsInScreen().width());
   w2->CloseNow();
 }
 
 }  // namespace views