Remove explicit singletonness of ArcBridgeService part 2.

chrome/browser/chromeos/arc/fileapi/arc_content_file_system_file_stream_reader_unittest.cc

 // Copyright 2016 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 <memory>
 #include <string>
 #include <utility>
 
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/location.h"
+#include "base/memory/ptr_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/chromeos/arc/fileapi/arc_content_file_system_file_stream_reader.h"
+#include "components/arc/arc_service_manager.h"
 #include "components/arc/test/fake_arc_bridge_service.h"
 #include "components/arc/test/fake_file_system_instance.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "mojo/edk/embedder/embedder.h"
 #include "net/base/io_buffer.h"
 #include "net/base/test_completion_callback.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace arc {
 
@@ skipping 50 matching lines @@
   ~ArcContentFileSystemFileStreamReaderTest() override = default;
 
   void SetUp() override {
     mojo::edk::Init();
 
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
 
     base::FilePath path = temp_dir_.GetPath().AppendASCII("bar");
     ASSERT_TRUE(base::WriteFile(path, kData, arraysize(kData)));
 
-    file_system_.reset(new ArcFileSystemInstanceTestImpl(path));
+    file_system_ = base::MakeUnique<ArcFileSystemInstanceTestImpl>(path);
 
-    fake_arc_bridge_service_.file_system()->SetInstance(file_system_.get());
+    ArcServiceManager::SetArcBridgeServiceForTesting(
+        base::MakeUnique<FakeArcBridgeService>());
+    arc_service_manager_ = base::MakeUnique<ArcServiceManager>(nullptr);
+    arc_service_manager_->arc_bridge_service()->file_system()->SetInstance(
+        file_system_.get());
+  }
+
+  void TearDown() override {
+    arc_service_manager_->arc_bridge_service()->file_system()->SetInstance(

[hashimoto, 2016/12/07 04:15:50]

Why do we need to do this?
The same goes for the other test.

[hidehiko, 2016/12/08 08:13:21]

Hmm, I wanted to emulate the real tear down situation, but indeed it'd be ok to
skip it as long as the test works well. Removed.

+        nullptr);
+    arc_service_manager_.reset();

[hashimoto, 2016/12/07 04:15:50]

Why can't we let the dtor handle resetting these unique_ptrs?
The same goes for the other test.

[hidehiko, 2016/12/08 08:13:21]

Acknowledged.

+
+    file_system_.reset();

[Luis Héctor Chávez, 2016/12/06 19:14:37]

nit: Try adding mojo::edk::test::Shutdown()[1] here. If it's not trivial to call
it or it introduces test failures, please file a bug to call it in all our
tests.

1:
https://cs.chromium.org/chromium/src/mojo/edk/embedder/test_embedder.h?sq=pac...

[hidehiko, 2016/12/08 08:13:21]

I haven't investigated yet, but causes a crash error.
Filed a bug: crbug.com/672385

   }
 
  private:
   base::ScopedTempDir temp_dir_;
   content::TestBrowserThreadBundle thread_bundle_;
-  FakeArcBridgeService fake_arc_bridge_service_;
   std::unique_ptr<ArcFileSystemInstanceTestImpl> file_system_;
+  std::unique_ptr<ArcServiceManager> arc_service_manager_;

[hashimoto, 2016/12/07 04:15:50]

In the other test, arc_service_manager_ is placed before file_system_.
Please stick to one rule to be consistent.

[hidehiko, 2016/12/08 08:13:21]

Done.

 
   DISALLOW_COPY_AND_ASSIGN(ArcContentFileSystemFileStreamReaderTest);
 };
 
 }  // namespace
 
 TEST_F(ArcContentFileSystemFileStreamReaderTest, Read) {
   ArcContentFileSystemFileStreamReader reader(GURL(kArcUrl), 0);
 
   auto base_buffer =
@@ skipping 14 matching lines @@
 
 TEST_F(ArcContentFileSystemFileStreamReaderTest, GetLength) {
   ArcContentFileSystemFileStreamReader reader(GURL(kArcUrl), 0);
 
   net::TestInt64CompletionCallback callback;
   EXPECT_EQ(static_cast<int64_t>(arraysize(kData)),
             callback.GetResult(reader.GetLength(callback.callback())));
 }
 
 }  // namespace arc