[Extensions][TaskScheduler] Update ActivityLog for scheduling migration

chrome/browser/extensions/activity_log/activity_database_unittest.cc

 // Copyright (c) 2012 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 <stddef.h>
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/test/simple_test_clock.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/extensions/activity_log/activity_action_constants.h"
 #include "chrome/browser/extensions/activity_log/activity_database.h"
+#include "chrome/browser/extensions/activity_log/activity_log_task_runner.h"
 #include "chrome/browser/extensions/activity_log/fullstream_ui_policy.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/test_browser_thread.h"
+#include "content/public/test/test_utils.h"
 #include "extensions/common/dom_action_types.h"
 #include "sql/statement.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using content::BrowserThread;
+#define TEST_ON_ACTIVITY_LOG_RUNNER(test_class, test_name)                 \

[Devlin, 2017/07/12 19:30:36]

gab@, I'm curious what you think of a pattern like this.  It allows us to still
write tests like
TEST_MACRO() {
  <test stuff>
}

But I'm not sure if it's either too magical, or risks some bad interaction with
task scheduling.

+  class test_class##_##test_name##_Runner : public test_class {            \
+   public:                                                                 \
+    void RunTest();                                                        \
+  };                                                                       \
+  TEST_F(test_class##_##test_name##_Runner, test_name) {                   \
+    base::RunLoop run_loop;                                                \
+    GetActivityLogTaskRunner()->PostTask(                                  \
+        FROM_HERE, base::Bind(&test_class##_##test_name##_Runner::RunTest, \
+                              base::Unretained(this)));                    \
+    content::RunAllBlockingPoolTasksUntilIdle();                           \
+  }                                                                        \
+  void test_class##_##test_name##_Runner::RunTest()
 
 namespace constants = activity_log_constants;
 
 namespace extensions {
 
 // A dummy implementation of ActivityDatabase::Delegate, sufficient for
 // the unit tests.
 class ActivityDatabaseTestPolicy : public ActivityDatabase::Delegate {
  public:
   ActivityDatabaseTestPolicy() {}
@@ skipping 83 matching lines @@
                                      const std::string& api_name) const {
     scoped_refptr<Action> action(
         new Action("punky", time, Action::ACTION_API_CALL, api_name));
     return action;
   }
 
   void Record(ActivityDatabase* db, scoped_refptr<Action> action) {
     db_delegate_->Record(db, action);
   }
 
+  int CountActions(ActivityDatabase* db, const std::string& api_name_pattern) {
+    return CountActions(&db->db_, api_name_pattern);
+  }
+
   int CountActions(sql::Connection* db, const std::string& api_name_pattern) {
     if (!db->DoesTableExist(ActivityDatabaseTestPolicy::kTableName))
       return -1;
     std::string sql_str = "SELECT COUNT(*) FROM " +
                           std::string(ActivityDatabaseTestPolicy::kTableName) +
                           " WHERE api_name LIKE ?";
     sql::Statement statement(db->GetCachedStatement(
         sql::StatementID(SQL_FROM_HERE), sql_str.c_str()));
     statement.BindString(0, api_name_pattern);
     if (!statement.Step())
       return -1;
     return statement.ColumnInt(0);
   }
 
+  void RecordBatchedActions(ActivityDatabase* db) {
+    db->RecordBatchedActionsWhileTesting();
+  }
+
  private:
   ActivityDatabaseTestPolicy* db_delegate_;
 };
 
 // Check that the database is initialized properly.
-TEST_F(ActivityDatabaseTest, Init) {
+TEST_ON_ACTIVITY_LOG_RUNNER(ActivityDatabaseTest, Init) {
   base::ScopedTempDir temp_dir;
   base::FilePath db_file;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   db_file = temp_dir.GetPath().AppendASCII("ActivityInit.db");
   sql::Connection::Delete(db_file);
 
   ActivityDatabase* activity_db = OpenDatabase(db_file);
   activity_db->Close();
 
   sql::Connection db;
   ASSERT_TRUE(db.Open(db_file));
   ASSERT_TRUE(db.DoesTableExist(ActivityDatabaseTestPolicy::kTableName));
   db.Close();
 }
 
 // Check that actions are recorded in the db.
-TEST_F(ActivityDatabaseTest, RecordAction) {
+TEST_ON_ACTIVITY_LOG_RUNNER(ActivityDatabaseTest, RecordAction) {
   base::ScopedTempDir temp_dir;
   base::FilePath db_file;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   db_file = temp_dir.GetPath().AppendASCII("ActivityRecord.db");
   sql::Connection::Delete(db_file);
 
   ActivityDatabase* activity_db = OpenDatabase(db_file);
   activity_db->SetBatchModeForTesting(false);
   scoped_refptr<Action> action = CreateAction(base::Time::Now(), "brewster");
   Record(activity_db, action);
   activity_db->Close();
 
   sql::Connection db;
   ASSERT_TRUE(db.Open(db_file));
 
   ASSERT_EQ(1, CountActions(&db, "brewster"));
 }
 
-TEST_F(ActivityDatabaseTest, BatchModeOff) {
+TEST_ON_ACTIVITY_LOG_RUNNER(ActivityDatabaseTest, BatchModeOff) {
   base::ScopedTempDir temp_dir;
   base::FilePath db_file;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   db_file = temp_dir.GetPath().AppendASCII("ActivityRecord.db");
   sql::Connection::Delete(db_file);
 
   // Record some actions
   ActivityDatabase* activity_db = OpenDatabase(db_file);
   activity_db->SetBatchModeForTesting(false);
 
   scoped_refptr<Action> action = CreateAction(base::Time::Now(), "brewster");
   Record(activity_db, action);
-  ASSERT_EQ(1, CountActions(&activity_db->db_, "brewster"));
+  ASSERT_EQ(1, CountActions(activity_db, "brewster"));
 
   activity_db->Close();
 }
 
-TEST_F(ActivityDatabaseTest, BatchModeOn) {
+TEST_ON_ACTIVITY_LOG_RUNNER(ActivityDatabaseTest, BatchModeOn) {
   base::ScopedTempDir temp_dir;
   base::FilePath db_file;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   db_file = temp_dir.GetPath().AppendASCII("ActivityRecord.db");
   sql::Connection::Delete(db_file);
 
   // Record some actions
   ActivityDatabase* activity_db = OpenDatabase(db_file);
   activity_db->SetBatchModeForTesting(true);
   scoped_refptr<Action> action = CreateAction(base::Time::Now(), "brewster");
   Record(activity_db, action);
-  ASSERT_EQ(0, CountActions(&activity_db->db_, "brewster"));
+  ASSERT_EQ(0, CountActions(activity_db, "brewster"));
 
   // Artificially trigger and then stop the timer.
-  activity_db->SetTimerForTesting(0);
-  base::RunLoop().RunUntilIdle();

[Devlin, 2017/07/12 19:30:36]

We can remove this here because SetTimerForTesting() just kicked off a timer to
execute RecordBatchedActionsWhileTesting(), so doing that immediately followed
by a RunLoop().RunUntilIdle() was equivalent to just calling the record method
directly.

But, it raised a question.  The reason I changed this was because RunLoops don't
seem to work on other threads (or at least, not whichever thread the task
scheduler put us on).  The migration doc says

"If test code was previously using RunLoop to execute things off the main thread
(as TestBrowserThreadBundle grouped everything under a single MessageLoop),
flushing tasks will now require asking for that explicitly."  Silly question -
how do we "ask for that explicitly"?  The usual suspects of
content::RunAllBlockingPoolTasksUntilIdle() and TaskScheduler::FlushForTesting()
don't work.  Even though we don't strictly need it here, it seems like a useful
thing to be able to do; is there a canonical blocking pool run loop equivalent?

-  ASSERT_EQ(1, CountActions(&activity_db->db_, "brewster"));
+  RecordBatchedActions(activity_db);
+  ASSERT_EQ(1, CountActions(activity_db, "brewster"));
 
   activity_db->Close();
 }
 
-TEST_F(ActivityDatabaseTest, BatchModeFlush) {
+TEST_ON_ACTIVITY_LOG_RUNNER(ActivityDatabaseTest, BatchModeFlush) {
   base::ScopedTempDir temp_dir;
   base::FilePath db_file;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   db_file = temp_dir.GetPath().AppendASCII("ActivityFlush.db");
   sql::Connection::Delete(db_file);
 
   // Record some actions
   ActivityDatabase* activity_db = OpenDatabase(db_file);
   activity_db->SetBatchModeForTesting(true);
   scoped_refptr<Action> action = CreateAction(base::Time::Now(), "brewster");
   Record(activity_db, action);
-  ASSERT_EQ(0, CountActions(&activity_db->db_, "brewster"));
+  ASSERT_EQ(0, CountActions(activity_db, "brewster"));
 
   // Request an immediate database flush.
   activity_db->AdviseFlush(ActivityDatabase::kFlushImmediately);
-  ASSERT_EQ(1, CountActions(&activity_db->db_, "brewster"));
+  ASSERT_EQ(1, CountActions(activity_db, "brewster"));
 
   activity_db->Close();
 }
 
 // Check that nothing explodes if the DB isn't initialized.
-TEST_F(ActivityDatabaseTest, InitFailure) {
+TEST_ON_ACTIVITY_LOG_RUNNER(ActivityDatabaseTest, InitFailure) {
   base::ScopedTempDir temp_dir;
   base::FilePath db_file;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   db_file = temp_dir.GetPath().AppendASCII("ActivityRecord.db");
   sql::Connection::Delete(db_file);
 
   ActivityDatabaseTestPolicy* delegate = new ActivityDatabaseTestPolicy();
   ActivityDatabase* activity_db = new ActivityDatabase(delegate);
   scoped_refptr<Action> action = new Action(
       "punky", base::Time::Now(), Action::ACTION_API_CALL, "brewster");
   action->mutable_args()->AppendString("woof");
   delegate->Record(activity_db, action);
   activity_db->Close();
 }
 
 }  // namespace extensions