Replace deprecated base::NonThreadSafe in chrome/browser/extensions in favor of SequenceChecker.

chrome/browser/chromeos/printing/printers_manager_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 "chrome/browser/chromeos/printing/printers_manager.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 68 matching lines @@
  private:
   base::Optional<Printer> last_added_;
   base::Optional<Printer> last_updated_;
   base::Optional<Printer> last_removed_;
 };
 
 }  // namespace
 
 class PrintersManagerTest : public testing::Test {
  protected:
-  PrintersManagerTest() : profile_(base::MakeUnique<TestingProfile>()) {
-    thread_bundle_ = base::MakeUnique<content::TestBrowserThreadBundle>();
-
-    auto sync_bridge = base::MakeUnique<PrintersSyncBridge>(
-        base::Bind(&syncer::ModelTypeStore::CreateInMemoryStoreForTest,
-                   syncer::PRINTERS),
-        base::BindRepeating(
-            base::IgnoreResult(&base::debug::DumpWithoutCrashing)));
-
-    manager_ = base::MakeUnique<PrintersManager>(profile_.get(),
-                                                 std::move(sync_bridge));
-
+  PrintersManagerTest()
+      : manager_(

[skau, 2017/05/31 19:22:09]

Can you note that member initialization happens before initializer lists are
evaluated?  I don't remember what the spec says and it's not obvious that this
is doing the right thing.

[gab, 2017/05/31 21:19:25]

This is not what's happening, members are initialized in their declaration order
regardless of whether value is specified on the member (default or inline member
init) or in the initializer list.

I don't think I want to add a comment on that because this is used all over the
codebase and explaining C++ everywhere this is used seems overkill.

+            &profile_,
+            base::MakeUnique<PrintersSyncBridge>(
+                base::Bind(&syncer::ModelTypeStore::CreateInMemoryStoreForTest,
+                           syncer::PRINTERS),
+                base::BindRepeating(
+                    base::IgnoreResult(&base::debug::DumpWithoutCrashing)))) {
     base::RunLoop().RunUntilIdle();
   }
 
-  ~PrintersManagerTest() override {
-    manager_.reset();
+  // Must be first to be alive throughout |profile_|'s lifetime.

[Lei Zhang, 2017/05/31 19:14:23]

Just say "Must outlive |profile_|" ?

[gab, 2017/05/31 21:19:25]

Done.

+  content::TestBrowserThreadBundle thread_bundle_;
 
-    // Explicitly release the profile before the thread_bundle.  Otherwise, the
-    // profile destructor throws an error.
-    profile_.reset();
-    thread_bundle_.reset();
-  }
-
-  std::unique_ptr<TestingProfile> profile_;
-  std::unique_ptr<PrintersManager> manager_;
-
- private:
-  std::unique_ptr<content::TestBrowserThreadBundle> thread_bundle_;
+  TestingProfile profile_;

[Lei Zhang, 2017/05/31 19:14:23]

Add a comment to say this must outlive |manager_| ?

[gab, 2017/05/31 21:19:25]

Done.

+  PrintersManager manager_;
 };
 
 TEST_F(PrintersManagerTest, AddPrinter) {
   LoggingObserver observer;
-  manager_->AddObserver(&observer);
-  manager_->RegisterPrinter(base::MakeUnique<Printer>(kPrinterId));
+  manager_.AddObserver(&observer);
+  manager_.RegisterPrinter(base::MakeUnique<Printer>(kPrinterId));
 
-  auto printers = manager_->GetPrinters();
+  auto printers = manager_.GetPrinters();
   ASSERT_EQ(1U, printers.size());
   EXPECT_EQ(kPrinterId, printers[0]->id());
   EXPECT_EQ(Printer::Source::SRC_USER_PREFS, printers[0]->source());
 
   EXPECT_TRUE(observer.AddCalled());
   EXPECT_FALSE(observer.UpdateCalled());
 }
 
 TEST_F(PrintersManagerTest, UpdatePrinterAssignsId) {
-  manager_->RegisterPrinter(base::MakeUnique<Printer>());
+  manager_.RegisterPrinter(base::MakeUnique<Printer>());
 
-  auto printers = manager_->GetPrinters();
+  auto printers = manager_.GetPrinters();
   ASSERT_EQ(1U, printers.size());
   EXPECT_FALSE(printers[0]->id().empty());
 }
 
 TEST_F(PrintersManagerTest, UpdatePrinter) {
-  manager_->RegisterPrinter(base::MakeUnique<Printer>(kPrinterId));
+  manager_.RegisterPrinter(base::MakeUnique<Printer>(kPrinterId));
   auto updated_printer = base::MakeUnique<Printer>(kPrinterId);
   updated_printer->set_uri(kUri);
 
   // Register observer so it only receives the update event.
   LoggingObserver observer;
-  manager_->AddObserver(&observer);
+  manager_.AddObserver(&observer);
 
-  manager_->RegisterPrinter(std::move(updated_printer));
+  manager_.RegisterPrinter(std::move(updated_printer));
 
-  auto printers = manager_->GetPrinters();
+  auto printers = manager_.GetPrinters();
   ASSERT_EQ(1U, printers.size());
   EXPECT_EQ(kUri, printers[0]->uri());
 
   EXPECT_TRUE(observer.UpdateCalled());
   EXPECT_FALSE(observer.AddCalled());
 }
 
 TEST_F(PrintersManagerTest, RemovePrinter) {
-  manager_->RegisterPrinter(base::MakeUnique<Printer>("OtherUUID"));
-  manager_->RegisterPrinter(base::MakeUnique<Printer>(kPrinterId));
-  manager_->RegisterPrinter(base::MakeUnique<Printer>());
+  manager_.RegisterPrinter(base::MakeUnique<Printer>("OtherUUID"));
+  manager_.RegisterPrinter(base::MakeUnique<Printer>(kPrinterId));
+  manager_.RegisterPrinter(base::MakeUnique<Printer>());
 
-  manager_->RemovePrinter(kPrinterId);
+  manager_.RemovePrinter(kPrinterId);
 
-  auto printers = manager_->GetPrinters();
+  auto printers = manager_.GetPrinters();
   ASSERT_EQ(2U, printers.size());
   EXPECT_NE(kPrinterId, printers.at(0)->id());
   EXPECT_NE(kPrinterId, printers.at(1)->id());
 }
 
 // Tests for policy printers
 
 TEST_F(PrintersManagerTest, RecommendedPrinters) {
   std::string first_printer =
       R"json({
       "display_name": "Color Laser",
       "description": "The printer next to the water cooler.",
       "manufacturer": "Printer Manufacturer",
       "model":"Color Laser 2004",
       "uri":"ipps://print-server.intranet.example.com:443/ipp/cl2k4",
       "uuid":"1c395fdb-5d93-4904-b246-b2c046e79d12",
       "ppd_resource":{
           "effective_manufacturer": "MakesPrinters",
           "effective_model":"ColorLaser2k4"
        }
       })json";
 
   std::string second_printer = kLexJson;
 
   auto value = base::MakeUnique<base::ListValue>();
   value->AppendString(first_printer);
   value->AppendString(second_printer);
 
   sync_preferences::TestingPrefServiceSyncable* prefs =
-      profile_->GetTestingPrefService();
+      profile_.GetTestingPrefService();
   // TestingPrefSyncableService assumes ownership of |value|.
   prefs->SetManagedPref(prefs::kRecommendedNativePrinters, std::move(value));
 
-  auto printers = manager_->GetRecommendedPrinters();
+  auto printers = manager_.GetRecommendedPrinters();
   ASSERT_EQ(2U, printers.size());
   EXPECT_EQ("Color Laser", printers[0]->display_name());
   EXPECT_EQ("ipp://192.168.1.5", printers[1]->uri());
   EXPECT_EQ(Printer::Source::SRC_POLICY, printers[1]->source());
 }
 
 TEST_F(PrintersManagerTest, GetRecommendedPrinter) {
   std::string printer = kLexJson;
   auto value = base::MakeUnique<base::ListValue>();
   value->AppendString(printer);
 
   sync_preferences::TestingPrefServiceSyncable* prefs =
-      profile_->GetTestingPrefService();
+      profile_.GetTestingPrefService();
   // TestingPrefSyncableService assumes ownership of |value|.
   prefs->SetManagedPref(prefs::kRecommendedNativePrinters, std::move(value));
 
-  auto printers = manager_->GetRecommendedPrinters();
+  auto printers = manager_.GetRecommendedPrinters();
 
   const Printer& from_list = *(printers.front());
-  std::unique_ptr<Printer> retrieved = manager_->GetPrinter(from_list.id());
+  std::unique_ptr<Printer> retrieved = manager_.GetPrinter(from_list.id());
 
   EXPECT_EQ(from_list.id(), retrieved->id());
   EXPECT_EQ("LexaPrint", from_list.display_name());
   EXPECT_EQ(Printer::Source::SRC_POLICY, from_list.source());
 }
 
 TEST_F(PrintersManagerTest, PrinterNotInstalled) {
   Printer printer(kPrinterId, base::Time::FromInternalValue(1000));
-  EXPECT_FALSE(manager_->IsConfigurationCurrent(printer));
+  EXPECT_FALSE(manager_.IsConfigurationCurrent(printer));
 }
 
 TEST_F(PrintersManagerTest, PrinterIsInstalled) {
   Printer printer(kPrinterId, base::Time::FromInternalValue(1000));
-  manager_->PrinterInstalled(printer);
-  EXPECT_TRUE(manager_->IsConfigurationCurrent(printer));
+  manager_.PrinterInstalled(printer);
+  EXPECT_TRUE(manager_.IsConfigurationCurrent(printer));
 }
 
 TEST_F(PrintersManagerTest, UpdatedPrinterConfiguration) {
   Printer printer(kPrinterId, base::Time::FromInternalValue(1000));
-  manager_->PrinterInstalled(printer);
+  manager_.PrinterInstalled(printer);
 
   Printer updated_printer(kPrinterId, base::Time::FromInternalValue(2000));
-  EXPECT_FALSE(manager_->IsConfigurationCurrent(updated_printer));
+  EXPECT_FALSE(manager_.IsConfigurationCurrent(updated_printer));
 }
 
 }  // namespace chromeos