[Media Router] Auto-dimiss non-blocking issues part 1.

This patch implements the following:
- auto-dismiss notification issues after 1 minute
- auto-dismiss non-notification non-blocking issue after 5 minutes

This is done by posting a task to clear a given issue after it has been
added to the IssueManager.

Part 2 will be to dismiss non-blocking issues following a successful
cast from the Media Router UI.

Bug: 763432
Change-Id: I6c55d4c272b8fc09b48935c6d4011d893e0cea58
Reviewed-on: https://chromium-review.googlesource.com/658138
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501334}

chrome/browser/media/router/issue_manager.cc

@@ -11,16 +11,48 @@
 
 namespace media_router {
 
-IssueManager::IssueManager() : top_issue_(nullptr) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+namespace {
+
+// The number of minutes a NOTIFICATION Issue stays in the IssueManager
+// before it is auto-dismissed.
+constexpr int kNotificationAutoDismissMins = 1;
+
+// The number of minutes a WARNING Issue stays in the IssueManager before it
+// is auto-dismissed.
+constexpr int kWarningAutoDismissMins = 5;
+
+}  // namespace
+
+// static
+base::TimeDelta IssueManager::GetAutoDismissTimeout(
+    const IssueInfo& issue_info) {
+  if (issue_info.is_blocking)
+    return base::TimeDelta();
+
+  switch (issue_info.severity) {
+    case IssueInfo::Severity::NOTIFICATION:
+      return base::TimeDelta::FromMinutes(kNotificationAutoDismissMins);
+    case IssueInfo::Severity::WARNING:
+      return base::TimeDelta::FromMinutes(kWarningAutoDismissMins);
+    case IssueInfo::Severity::FATAL:
+      NOTREACHED() << "FATAL issues should be blocking";
+      return base::TimeDelta();
+  }
+  NOTREACHED();
+  return base::TimeDelta();
 }
 
+IssueManager::IssueManager()
+    : top_issue_(nullptr),
+      task_runner_(content::BrowserThread::GetTaskRunnerForThread(
+          content::BrowserThread::UI)) {}
+
 IssueManager::~IssueManager() {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }
 
 void IssueManager::AddIssue(const IssueInfo& issue_info) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto it = std::find_if(issues_.begin(), issues_.end(),
                          [&issue_info](const std::unique_ptr<Issue>& issue) {
                            return issue_info == issue->info();
@@ -28,23 +60,36 @@ void IssueManager::AddIssue(const IssueInfo& issue_info) {
   if (it != issues_.end())
     return;
 
-  issues_.push_back(base::MakeUnique<Issue>(issue_info));
+  auto issue = base::MakeUnique<Issue>(issue_info);
+  base::TimeDelta timeout = GetAutoDismissTimeout(issue_info);
+  if (!timeout.is_zero()) {
+    const Issue::Id& issue_id = issue->id();
+    auto auto_dismiss_issue_cb = base::Bind(
+        &IssueManager::ClearIssue, base::Unretained(this), issue->id());
+    auto_dismiss_issue_callbacks_.emplace(
+        issue_id,
+        base::MakeUnique<base::CancelableClosure>(auto_dismiss_issue_cb));
+    task_runner_->PostDelayedTask(FROM_HERE, auto_dismiss_issue_cb, timeout);
+  }
+
+  issues_.push_back(std::move(issue));
   MaybeUpdateTopIssue();
 }
 
 void IssueManager::ClearIssue(const Issue::Id& issue_id) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   issues_.erase(
       std::remove_if(issues_.begin(), issues_.end(),
                      [&issue_id](const std::unique_ptr<Issue>& issue) {
                        return issue_id == issue->id();
                      }),
       issues_.end());
+  auto_dismiss_issue_callbacks_.erase(issue_id);
   MaybeUpdateTopIssue();
 }
 
 void IssueManager::RegisterObserver(IssuesObserver* observer) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(observer);
   DCHECK(!issues_observers_.HasObserver(observer));
 
@@ -55,7 +100,7 @@ void IssueManager::RegisterObserver(IssuesObserver* observer) {
 }
 
 void IssueManager::UnregisterObserver(IssuesObserver* observer) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   issues_observers_.RemoveObserver(observer);
 }
 

chrome/browser/media/router/issue_manager.h

@@ -10,9 +10,12 @@
 #include <memory>
 #include <vector>
 
-#include "base/containers/hash_tables.h"
+#include "base/cancelable_callback.h"
 #include "base/macros.h"
+#include "base/memory/ref_counted.h"
 #include "base/observer_list.h"
+#include "base/sequence_checker.h"
+#include "base/single_thread_task_runner.h"
 #include "chrome/browser/media/router/issues_observer.h"
 #include "chrome/common/media_router/issue.h"
 
@@ -20,12 +23,16 @@ namespace media_router {
 
 // IssueManager keeps track of current issues related to casting
 // connectivity and quality. It lives on the UI thread.
-// TODO(apacible): Determine what other issues will be handled here.
 class IssueManager {
  public:
   IssueManager();
   ~IssueManager();
 
+  // Returns the amount of time before |issue_info| is dismissed after it is
+  // added to the IssueManager. Returns base::TimeDelta() if the given IssueInfo
+  // is not auto-dismissed.
+  static base::TimeDelta GetAutoDismissTimeout(const IssueInfo& issue_info);
+
   // Adds an issue. No-ops if the issue already exists.
   // |issue_info|: Info of issue to be added.
   void AddIssue(const IssueInfo& issue_info);
@@ -45,6 +52,11 @@ class IssueManager {
   // |observer|: IssuesObserver to be unregistered.
   void UnregisterObserver(IssuesObserver* observer);
 
+  void set_task_runner_for_test(
+      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
+    task_runner_ = task_runner;
+  }
+
  private:
   // Checks if the current top issue has changed. Updates |top_issue_|.
   // If |top_issue_| has changed, observers in |issues_observers_| will be
@@ -59,6 +71,21 @@ class IssueManager {
   // ID of the top Issue in |issues_|, or |nullptr| if there are no issues.
   const Issue* top_issue_;
 
+  // The SingleThreadTaskRunner that this IssueManager runs on, and is used
+  // for posting issue auto-dismissal tasks.
+  // When a non-blocking issues is added to the IssueManager, a delayed task
+  // will be added to remove the issue. This is done to automatically clean up
+  // issues that are no longer relevant.
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+
+  // Tracks auto-dimiss callbacks that are posted to |task_runner_|. If an
+  // Issue is cleared by the user before the auto-dimiss timeout, then the
+  // callback will be canceled.
+  std::map<Issue::Id, std::unique_ptr<base::CancelableClosure>>
+      auto_dismiss_issue_callbacks_;
+
+  SEQUENCE_CHECKER(sequence_checker_);
+
   DISALLOW_COPY_AND_ASSIGN(IssueManager);
 };
 

chrome/browser/media/router/issue_manager_unittest.cc

@@ -6,6 +6,8 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "base/test/test_mock_time_task_runner.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/media/router/issue_manager.h"
 #include "chrome/browser/media/router/mock_media_router.h"
 #include "chrome/browser/media/router/test_helper.h"
@@ -29,15 +31,17 @@ IssueInfo CreateTestIssue(IssueInfo::Severity severity) {
 
 class IssueManagerTest : public ::testing::Test {
  protected:
-  IssueManagerTest() {}
+  IssueManagerTest()
+      : task_runner_(new base::TestMockTimeTaskRunner()),
+        runner_handler_(task_runner_) {
+    manager_.set_task_runner_for_test(task_runner_);
+  }
   ~IssueManagerTest() override {}
 
-  content::TestBrowserThreadBundle thread_bundle_;
+  scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
+  base::ThreadTaskRunnerHandle runner_handler_;
   IssueManager manager_;
   MockMediaRouter router_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(IssueManagerTest);
 };
 
 TEST_F(IssueManagerTest, AddAndClearIssue) {
@@ -96,4 +100,74 @@ TEST_F(IssueManagerTest, AddSameIssueInfoHasNoEffect) {
   manager_.UnregisterObserver(&observer);
 }
 
+TEST_F(IssueManagerTest, NonBlockingIssuesGetAutoDismissed) {
+  MockIssuesObserver observer(&router_);
+  manager_.RegisterObserver(&observer);
+
+  EXPECT_CALL(observer, OnIssue(_)).Times(1);
+  IssueInfo issue_info1 = CreateTestIssue(IssueInfo::Severity::NOTIFICATION);
+  manager_.AddIssue(issue_info1);
+
+  EXPECT_CALL(observer, OnIssuesCleared()).Times(1);
+  base::TimeDelta timeout = IssueManager::GetAutoDismissTimeout(issue_info1);
+  EXPECT_FALSE(timeout.is_zero());
+  EXPECT_TRUE(task_runner_->HasPendingTask());
+  task_runner_->FastForwardBy(timeout);
+
+  EXPECT_CALL(observer, OnIssue(_)).Times(1);
+  IssueInfo issue_info2 = CreateTestIssue(IssueInfo::Severity::WARNING);
+  manager_.AddIssue(issue_info2);
+
+  EXPECT_CALL(observer, OnIssuesCleared()).Times(1);
+  timeout = IssueManager::GetAutoDismissTimeout(issue_info2);
+  EXPECT_FALSE(timeout.is_zero());
+  EXPECT_TRUE(task_runner_->HasPendingTask());
+  task_runner_->FastForwardBy(timeout);
+  EXPECT_FALSE(task_runner_->HasPendingTask());
+}
+
+TEST_F(IssueManagerTest, IssueAutoDismissNoopsIfAlreadyCleared) {
+  MockIssuesObserver observer(&router_);
+  manager_.RegisterObserver(&observer);
+
+  Issue issue1((IssueInfo()));
+  EXPECT_CALL(observer, OnIssue(_)).Times(1).WillOnce(SaveArg<0>(&issue1));
+  IssueInfo issue_info1 = CreateTestIssue(IssueInfo::Severity::NOTIFICATION);
+  manager_.AddIssue(issue_info1);
+
+  EXPECT_CALL(observer, OnIssuesCleared()).Times(1);
+  manager_.ClearIssue(issue1.id());
+
+  EXPECT_CALL(observer, OnIssuesCleared()).Times(0);
+  base::TimeDelta timeout = IssueManager::GetAutoDismissTimeout(issue_info1);
+  EXPECT_FALSE(timeout.is_zero());
+  EXPECT_TRUE(task_runner_->HasPendingTask());
+  task_runner_->FastForwardBy(timeout);
+  EXPECT_FALSE(task_runner_->HasPendingTask());
+}
+
+TEST_F(IssueManagerTest, BlockingIssuesDoNotGetAutoDismissed) {
+  MockIssuesObserver observer(&router_);
+  manager_.RegisterObserver(&observer);
+
+  EXPECT_CALL(observer, OnIssue(_)).Times(1);
+  IssueInfo issue_info1 = CreateTestIssue(IssueInfo::Severity::WARNING);
+  issue_info1.is_blocking = true;
+  manager_.AddIssue(issue_info1);
+
+  EXPECT_CALL(observer, OnIssuesCleared()).Times(0);
+
+  base::TimeDelta timeout = IssueManager::GetAutoDismissTimeout(issue_info1);
+  EXPECT_TRUE(timeout.is_zero());
+  EXPECT_FALSE(task_runner_->HasPendingTask());
+
+  // FATAL issues are always blocking.
+  IssueInfo issue_info2 = CreateTestIssue(IssueInfo::Severity::FATAL);
+  manager_.AddIssue(issue_info2);
+
+  timeout = IssueManager::GetAutoDismissTimeout(issue_info2);
+  EXPECT_TRUE(timeout.is_zero());
+  EXPECT_FALSE(task_runner_->HasPendingTask());
+}
+
 }  // namespace media_router