Broke out layout metric information from ViewsDelegate to LayoutProvider

chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_ash_unittest.cc

Index: chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_ash_unittest.cc
diff --git a/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_ash_unittest.cc b/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_ash_unittest.cc
index 71208cbfc2501deb599f2a50a5ce703e6e5ff9de..23f80ffaacc454803ef1ff3853090b8a422ce615 100644
--- a/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_ash_unittest.cc
+++ b/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_ash_unittest.cc
@@ -3,12 +3,16 @@
 // found in the LICENSE file.
 
 #include "ash/test/ash_test_base.h"
+#include "ash/test/ash_test_helper.h"
+#include "ash/test/ash_test_views_delegate.h"
 #include "base/macros.h"
 #include "chrome/browser/extensions/test_extension_environment.h"
 #include "chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.h"
 #include "chrome/browser/ui/views/apps/app_info_dialog/app_info_footer_panel.h"
+#include "chrome/browser/ui/views/harmony/chrome_views_layout_delegate.h"
 #include "chrome/test/base/testing_profile.h"
 #include "ui/views/controls/button/label_button.h"
+#include "ui/views/test/test_views_delegate.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/dialog_delegate.h"
 
@@ -26,6 +30,8 @@ class AppInfoDialogAshTest : public ash::test::AshTestBase {
   // Overridden from testing::Test:
   void SetUp() override {
     ash::test::AshTestBase::SetUp();
+    ash_test_helper()->views_delegate()->set_layout_delegate(
+        base::MakeUnique<ChromeViewsLayoutDelegate>());

[Peter Kasting, 2017/04/04 02:08:51]

I'm wondering how long this set could theoretically persist.  Maybe TearDown()
should restore the previous layout delegate (would take a bit of API munging to
do, basically set_layout_delegate() would have to return the old unique_ptr). 
This would guarantee that future tests in the same process couldn't be affected.

We could make some helper NeedsChromeLayoutDelegate object that tests like this
can include as a member.  Its default constructor/destructor would take care of
setting and resetting the layout delegate.  Then this could be used in all the
cases where you currently call set_layout_delegate().

This might even allow making set_layout_delegate() private and friending that
class, though I don't know if that's worth it.

[kylix_rd, 2017/04/04 20:28:22]

I don't think that is necessary since ash_test_helper_'s lifetime is tied to the
test class. AIUI, SetUp()/TearDown() are called around each individual test. Of
course new tests on this test class will share the same layout delegate. Is that
what you meant?
 
Hmm... that's an interesting idea. I'll need to think about ensuring the
overridden layout vs the default delegates are set in the correct order. In this
instance, AshTestBase sets the layout delegate in SetUp(), which is clearly
after the class itself is constructed.

[Peter Kasting, 2017/04/06 08:13:36]

Answer: only for one test.  This entire class is completely destroyed at the end
of every individual test.

The gtest FAQ even notes this and basically says everybody should be using
constructors/destructors to do everything, except in a few cases (e.g. needing
to call virtual functions on the current object):
https://github.com/google/googletest/blob/master/googletest/docs/FAQ.md#shoul...

So the thing I was worried about is not a problem.  The only question is whether
some sort of helper object like I suggested would make the code read more
nicely.  Up to you whether that's worth trying to restructure things to do.

     widget_ = views::DialogDelegate::CreateDialogWidget(
         new views::DialogDelegateView(), CurrentContext(), NULL);
     dialog_ = new AppInfoDialog(