PlzNavigate: Refactor unit test helper functions

content/test/browser_side_navigation_test_utils.cc

Index: content/test/browser_side_navigation_test_utils.cc
diff --git a/content/test/browser_side_navigation_test_utils.cc b/content/test/browser_side_navigation_test_utils.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9111071f98110e89af3a2169da69ab74f4a5be7f
--- /dev/null
+++ b/content/test/browser_side_navigation_test_utils.cc
@@ -0,0 +1,65 @@
+// Copyright (c) 2014 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 "content/test/browser_side_navigation_test_utils.h"
+
+#include "base/command_line.h"
+#include "base/guid.h"
+#include "base/lazy_instance.h"
+#include "content/browser/streams/stream.h"
+#include "content/browser/streams/stream_registry.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/stream_handle.h"
+#include "content/public/common/content_switches.h"
+#include "content/test/test_navigation_url_loader_factory.h"
+
+namespace content {
+
+namespace {
+
+base::LazyInstance<scoped_ptr<BrowserSideNavigationTestUtils>>

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

As we need to define an Init method anyway to properly initialize the scoped
pointer with the instance why wrap it with a LazyInstance? What's the benefit it
brings in this case?

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

It allows to have a global variable of non POD type. Basically it ensures that
you won't run into weird global initialization bugs.

+    browser_side_navigation_test_utils;
+
+}  // namespace
+
+// static
+BrowserSideNavigationTestUtils* BrowserSideNavigationTestUtils::Get() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  return browser_side_navigation_test_utils.Get().get();
+}
+
+// static
+void BrowserSideNavigationTestUtils::Init() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  browser_side_navigation_test_utils.Get().reset(
+      new BrowserSideNavigationTestUtils);
+}
+
+// static
+void BrowserSideNavigationTestUtils::TearDown() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  browser_side_navigation_test_utils.Get().reset(nullptr);
+}
+
+BrowserSideNavigationTestUtils::~BrowserSideNavigationTestUtils() {
+}
+
+scoped_ptr<StreamHandle> BrowserSideNavigationTestUtils::MakeEmptyStream() {
+  GURL url(std::string(url::kBlobScheme) + "://" + base::GenerateGUID());
+  scoped_refptr<Stream> stream(new Stream(stream_registry_.get(), NULL, url));
+  stream->Finalize();
+  return stream->CreateHandle();
+}
+
+void BrowserSideNavigationTestUtils::EnableBrowserSideNavigation() {
+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kEnableBrowserSideNavigation);
+}
+
+BrowserSideNavigationTestUtils::BrowserSideNavigationTestUtils()

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

nit: Constructor should probably live closer to the destructor. I know the
ordering in the header file is different, but looks weird here. Your call.

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

Changed the ordering in the header file due to all methods now being static, so
they are close together.

+    : stream_registry_(new StreamRegistry),
+      loader_factory_(new TestNavigationURLLoaderFactory) {
+}
+
+}  // namespace content