PlzNavigate: Refactor unit test helper functions

content/test/browser_side_navigation_test_utils.h

+// Copyright (c) 2014 The Chromium Authors. All rights reserved.

[nasko, 2014/11/15 00:32:51]

No "(c)" needed.

[clamy, 2014/11/17 12:51:13]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_TEST_BROWSER_SIDE_NAVIGATION_TEST_UTILS_H_
+#define CONTENT_TEST_BROWSER_SIDE_NAVIGATION_TEST_UTILS_H_
+
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+
+namespace content {
+
+class StreamHandle;
+class StreamRegistry;
+class TestNavigationURLLoaderFactory;
+
+// PlzNavigate
+// A UI thread singleton helper class for browser side navigations. When browser
+// side navigations are enabled, initialize this class before doing any
+// operation that may start a navigation request on the UI thread.
+class BrowserSideNavigationTestUtils {
+ public:
+  // Returns a pointer to the BrowserSideNavigationTestUtils. Note: this will be

[nasko, 2014/11/15 00:32:51]

nit: Note: on new line.

[clamy, 2014/11/17 12:51:13]

Made all the method static and removed the singleton getter.

+  // NULL if Init was not called (or TearDown was called).
+  static BrowserSideNavigationTestUtils* Get();
+
+  // Initializes the BrowserSideNavigationsTestUtils. Following this call, all
+  // NavigationURLLoader objects created will be TestNavigationURLLoaders
+  // instead of NavigationURLloaderImpls. This should be called before any call
+  // in the UI thread unit tests that will start a navigation (eg.
+  // TestWebContents::NavigateAndCommit).

[clamy, 2014/11/14 15:45:26]

I am not sure if a global LazyInstance is destroyed at the end of each unit
test, hence the Init and TearDown functions (we don't want this class to exist
for the NavigationURLLoaderImpl unit tests). If it is, then I'll get rid of them
and there will just be a static Get method.

[nasko, 2014/11/15 00:32:51]

Unittests don't tear down the process, so any global LazyInstance objects will
be shared across tests. I guess you don't want that behavior, right?

[carlosk, 2014/11/17 10:46:56]

There was a similar situation a while ago where I had a statically initialized
value that was indeed reset between tests. So I'd say confirming this is in good
order.

[clamy, 2014/11/17 12:51:13]

Yes. In particular, we need no to have the singleton during the unit tests of
the NavigationURLLoader (because we need not to have a
TestNavigationURLLoaderFactory registered).

+  static void Init();

[nasko, 2014/11/15 00:32:51]

If you are using TearDown, let's use SetUp instead of Init, so it matches other
test code.

[clamy, 2014/11/17 12:51:13]

Done.

+
+  // Destroys the BrowserSideNavigationTestUtils singleton.
+  static void TearDown();
+
+  ~BrowserSideNavigationTestUtils();
+
+  // Returns an empty stream. Used when faking a navigation commit notification
+  // from the IO thread with a TestNavigationURLLoader.
+  scoped_ptr<StreamHandle> MakeEmptyStream();
+
+  void EnableBrowserSideNavigation();
+
+ private:
+  BrowserSideNavigationTestUtils();
+
+  scoped_ptr<StreamRegistry> stream_registry_;
+  scoped_ptr<TestNavigationURLLoaderFactory> loader_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserSideNavigationTestUtils);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_TEST_BROWSER_SIDE_NAVIGATION_TEST_UTILS_H_