[Extensions] Close the infobar 5 seconds after detaching the extension.

Based on the feedback we have received from our customers, showing the infobar after detaching the extension is rarely helpful and occasionally
misleading. To alleviates some of the pain associated with displaying the infobar after detaching the extension, this CL removes the infobar
five seconds after the extension has been detached. In a separate CL, we
will update the Cancel button to be in sync with the state.

Bug: 1096262
Change-Id: I55f7b2ca13817683a9ace6cab5118731513b9478
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2386328
Commit-Queue: Ghazale Hosseinabadi <ghazale@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813344}

chrome/browser/devtools/global_confirm_info_bar.cc

@@ -156,14 +156,10 @@ void GlobalConfirmInfoBar::DelegateProxy::Detach() {
 }
 
 // static
-void GlobalConfirmInfoBar::Show(
+GlobalConfirmInfoBar* GlobalConfirmInfoBar::Show(
     std::unique_ptr<ConfirmInfoBarDelegate> delegate) {
   // Owns itself, deleted by Close().
-  new GlobalConfirmInfoBar(std::move(delegate));
-}
-
-void GlobalConfirmInfoBar::Close() {
-  delete this;
+  return new GlobalConfirmInfoBar(std::move(delegate));
 }
 
 GlobalConfirmInfoBar::GlobalConfirmInfoBar(
@@ -215,6 +211,10 @@ void GlobalConfirmInfoBar::OnManagerShuttingDown(
   proxies_.erase(manager);
 }
 
+void GlobalConfirmInfoBar::Close() {
+  delete this;
+}
+
 void GlobalConfirmInfoBar::MaybeAddInfoBar(content::WebContents* web_contents) {
   if (is_closing_)
     return;

chrome/browser/devtools/global_confirm_info_bar.h

@@ -37,12 +37,16 @@ class GlobalConfirmInfoBar : public TabStripModelObserver,
   // the delegate will be deleted synchronously when any of the tabs' infobars
   // is closed via user action.  Note that both of these aspects of lifetime
   // management differ from how typical infobars work.
-  static void Show(std::unique_ptr<ConfirmInfoBarDelegate> delegate);
+  static GlobalConfirmInfoBar* Show(
+      std::unique_ptr<ConfirmInfoBarDelegate> delegate);
 
   // infobars::InfoBarManager::Observer:
   void OnInfoBarRemoved(infobars::InfoBar* info_bar, bool animate) override;
   void OnManagerShuttingDown(infobars::InfoBarManager* manager) override;
 
+  // Closes all the infobars.
+  void Close();
+
  private:
   class DelegateProxy;
 
@@ -62,9 +66,6 @@ class GlobalConfirmInfoBar : public TabStripModelObserver,
   // Adds the info bar to the tab if it is missing.
   void MaybeAddInfoBar(content::WebContents* web_contents);
 
-  // Closes all the infobars.
-  void Close();
-
   std::unique_ptr<ConfirmInfoBarDelegate> delegate_;
   std::map<infobars::InfoBarManager*, DelegateProxy*> proxies_;
   BrowserTabStripTracker browser_tab_strip_tracker_{this, nullptr};

chrome/browser/devtools/global_confirm_info_bar_browsertest.cc

@@ -94,3 +94,27 @@ IN_PROC_BROWSER_TEST_F(GlobalConfirmInfoBarTest, UserInteraction) {
   for (int i = 0; i < tab_strip_model->count(); i++)
     EXPECT_EQ(0u, GetInfoBarServiceFromTabIndex(i)->infobar_count());
 }
+
+IN_PROC_BROWSER_TEST_F(GlobalConfirmInfoBarTest, CreateAndCloseInfobar) {
+  TabStripModel* tab_strip_model = browser()->tab_strip_model();
+  ASSERT_EQ(1, tab_strip_model->count());
+  InfoBarService* infobar_service = GetInfoBarServiceFromTabIndex(0);
+
+  // Make sure the tab has no info bar.
+  EXPECT_EQ(0u, infobar_service->infobar_count());
+
+  auto delegate = std::make_unique<TestConfirmInfoBarDelegate>();
+  TestConfirmInfoBarDelegate* delegate_ptr = delegate.get();
+
+  GlobalConfirmInfoBar* infobar =
+      GlobalConfirmInfoBar::Show(std::move(delegate));
+
+  // Verify that the info bar is shown.
+  ASSERT_EQ(1u, infobar_service->infobar_count());
+  EXPECT_TRUE(
+      infobar_service->infobar_at(0)->delegate()->EqualsDelegate(delegate_ptr));
+
+  // Close the infobar and make sure that the tab has no info bar.
+  infobar->Close();
+  EXPECT_EQ(0u, infobar_service->infobar_count());
+}

chrome/browser/extensions/api/debugger/debugger_api.cc

@@ -270,6 +270,7 @@ bool ExtensionDevToolsClientHost::Attach() {
 }
 
 ExtensionDevToolsClientHost::~ExtensionDevToolsClientHost() {
+  ExtensionDevToolsInfoBarDelegate::NotifyExtensionDetached(extension_id());
   g_attached_client_hosts.Get().erase(this);
 }
 

chrome/browser/extensions/api/debugger/debugger_apitest.cc

@@ -11,7 +11,10 @@
 #include "base/memory/ref_counted.h"
 #include "base/path_service.h"
 #include "base/strings/stringprintf.h"
+#include "base/test/scoped_mock_time_message_loop_task_runner.h"
+#include "base/test/simple_test_tick_clock.h"
 #include "chrome/browser/extensions/api/debugger/debugger_api.h"
+#include "chrome/browser/extensions/api/debugger/extension_dev_tools_infobar_delegate.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_function_test_utils.h"
 #include "chrome/browser/infobars/infobar_service.h"
@@ -37,7 +40,7 @@ namespace extensions {
 
 class DebuggerApiTest : public ExtensionApiTest {
  protected:
-  ~DebuggerApiTest() override {}
+  ~DebuggerApiTest() override = default;
 
   void SetUpCommandLine(base::CommandLine* command_line) override;
   void SetUpOnMainThread() override;
@@ -51,6 +54,8 @@ class DebuggerApiTest : public ExtensionApiTest {
   const Extension* extension() const { return extension_.get(); }
   base::CommandLine* command_line() const { return command_line_; }
 
+  void AdvanceClock(base::TimeDelta time) { clock_.Advance(time); }
+
  private:
   testing::AssertionResult RunAttachFunctionOnTarget(
       const std::string& debuggee_target, const std::string& expected_error);
@@ -65,6 +70,7 @@ class DebuggerApiTest : public ExtensionApiTest {
   // A temporary directory in which to create and load from the
   // |extension_|.
   TestExtensionDir test_extension_dir_;
+  base::SimpleTestTickClock clock_;
 };
 
 void DebuggerApiTest::SetUpCommandLine(base::CommandLine* command_line) {
@@ -326,6 +332,91 @@ IN_PROC_BROWSER_TEST_F(DebuggerApiTest, InfoBar) {
   EXPECT_EQ(1u, service1->infobar_count());
 }
 
+IN_PROC_BROWSER_TEST_F(DebuggerApiTest, InfoBarIsRemovedAfterFiveSeconds) {
+  int tab_id = sessions::SessionTabHelper::IdForTab(
+                   browser()->tab_strip_model()->GetActiveWebContents())
+                   .id();
+  InfoBarService* service = InfoBarService::FromWebContents(
+      browser()->tab_strip_model()->GetActiveWebContents());
+
+  // Attaching to the tab should create an infobar.
+  auto attach_function = base::MakeRefCounted<DebuggerAttachFunction>();
+  attach_function->set_extension(extension());
+  ASSERT_TRUE(extension_function_test_utils::RunFunction(
+      attach_function.get(),
+      base::StringPrintf("[{\"tabId\": %d}, \"1.1\"]", tab_id), browser(),
+      api_test_utils::NONE));
+  EXPECT_EQ(1u, service->infobar_count());
+
+  // Detaching from the tab should remove the infobar after 5 seconds.
+  auto detach_function = base::MakeRefCounted<DebuggerDetachFunction>();
+  detach_function->set_extension(extension());
+  ASSERT_TRUE(extension_function_test_utils::RunFunction(
+      detach_function.get(), base::StringPrintf("[{\"tabId\": %d}]", tab_id),
+      browser(), api_test_utils::NONE));
+
+  // Even though the extension detached, the infobar should not detach
+  // immediately, and should remain visible for 5 seconds to ensure the user
+  // has an opportunity to see it.
+  base::RunLoop run_loop;
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, run_loop.QuitClosure(),
+      ExtensionDevToolsInfoBarDelegate::kAutoCloseDelay);
+  EXPECT_EQ(1u, service->infobar_count());  // Infobar is still shown.
+
+  // Advance the clock by 5 seconds, and verify the infobar is removed.
+  AdvanceClock(ExtensionDevToolsInfoBarDelegate::kAutoCloseDelay);
+  run_loop.Run();
+
+  EXPECT_EQ(0u, service->infobar_count());
+}
+
+IN_PROC_BROWSER_TEST_F(DebuggerApiTest,
+                       InfoBarIsNotRemovedIfAttachAgainBeforeFiveSeconds) {
+  int tab_id = sessions::SessionTabHelper::IdForTab(
+                   browser()->tab_strip_model()->GetActiveWebContents())
+                   .id();
+  InfoBarService* service = InfoBarService::FromWebContents(
+      browser()->tab_strip_model()->GetActiveWebContents());
+
+  // Attaching to the tab should create an infobar.
+  auto attach_function = base::MakeRefCounted<DebuggerAttachFunction>();
+  attach_function->set_extension(extension());
+  ASSERT_TRUE(extension_function_test_utils::RunFunction(
+      attach_function.get(),
+      base::StringPrintf("[{\"tabId\": %d}, \"1.1\"]", tab_id), browser(),
+      api_test_utils::NONE));
+  EXPECT_EQ(1u, service->infobar_count());
+
+  // Detaching from the tab and attaching it again before 5 seconds should not
+  // remove the infobar.
+  auto detach_function = base::MakeRefCounted<DebuggerDetachFunction>();
+  detach_function->set_extension(extension());
+  ASSERT_TRUE(extension_function_test_utils::RunFunction(
+      detach_function.get(), base::StringPrintf("[{\"tabId\": %d}]", tab_id),
+      browser(), api_test_utils::NONE));
+  EXPECT_EQ(1u, service->infobar_count());
+
+  attach_function = base::MakeRefCounted<DebuggerAttachFunction>();
+  attach_function->set_extension(extension());
+  ASSERT_TRUE(extension_function_test_utils::RunFunction(
+      attach_function.get(),
+      base::StringPrintf("[{\"tabId\": %d}, \"1.1\"]", tab_id), browser(),
+      api_test_utils::NONE));
+  // Verify that only one infobar is created.
+  EXPECT_EQ(1u, service->infobar_count());
+
+  // Verify that infobar is not closed after 5 seconds.
+  base::RunLoop run_loop;
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, run_loop.QuitClosure(),
+      ExtensionDevToolsInfoBarDelegate::kAutoCloseDelay);
+  AdvanceClock(ExtensionDevToolsInfoBarDelegate::kAutoCloseDelay);
+  run_loop.Run();
+
+  EXPECT_EQ(1u, service->infobar_count());
+}
+
 class DebuggerExtensionApiTest : public ExtensionApiTest {
  public:
   void SetUpOnMainThread() override {

chrome/browser/extensions/api/debugger/extension_dev_tools_infobar_delegate.cc

@@ -30,22 +30,27 @@ base::LazyInstance<Delegates>::Leaky g_delegates = LAZY_INSTANCE_INITIALIZER;
 }  // namespace
 
 // static
+constexpr base::TimeDelta ExtensionDevToolsInfoBarDelegate::kAutoCloseDelay;
+
 std::unique_ptr<ExtensionDevToolsInfoBarDelegate::CallbackList::Subscription>
 ExtensionDevToolsInfoBarDelegate::Create(const std::string& extension_id,
                                          const std::string& extension_name,
                                          base::OnceClosure destroyed_callback) {
   Delegates& delegates = g_delegates.Get();
   const auto it = delegates.find(extension_id);
-  if (it != delegates.end())
+  if (it != delegates.end()) {
+    it->second->timer_.Stop();
     return it->second->RegisterDestroyedCallback(std::move(destroyed_callback));
+  }
 
   // Can't use std::make_unique<>(), constructor is private.
   auto delegate = base::WrapUnique(
       new ExtensionDevToolsInfoBarDelegate(extension_id, extension_name));
-  delegates[extension_id] = delegate.get();
+  auto* delegate_raw = delegate.get();
+  delegates[extension_id] = delegate_raw;
   std::unique_ptr<CallbackList::Subscription> subscription =
       delegate->RegisterDestroyedCallback(std::move(destroyed_callback));
-  GlobalConfirmInfoBar::Show(std::move(delegate));
+  delegate_raw->infobar_ = GlobalConfirmInfoBar::Show(std::move(delegate));
   return subscription;
 }
 
@@ -55,6 +60,18 @@ ExtensionDevToolsInfoBarDelegate::~ExtensionDevToolsInfoBarDelegate() {
   DCHECK(erased);
 }
 
+void ExtensionDevToolsInfoBarDelegate::NotifyExtensionDetached(
+    const std::string& extension_id) {
+  const Delegates& delegates = g_delegates.Get();
+  const auto iter = delegates.find(extension_id);
+  if (iter != delegates.cend()) {
+    // Infobar_ was set in Create() which makes the following access safe.
+    iter->second->timer_.Start(FROM_HERE, kAutoCloseDelay,
+                               iter->second->infobar_,
+                               &GlobalConfirmInfoBar::Close);
+  }
+}
+
 infobars::InfoBarDelegate::InfoBarIdentifier
 ExtensionDevToolsInfoBarDelegate::GetIdentifier() const {
   return EXTENSION_DEV_TOOLS_INFOBAR_DELEGATE;

chrome/browser/extensions/api/debugger/extension_dev_tools_infobar_delegate.h

@@ -11,14 +11,19 @@
 #include "base/callback_forward.h"
 #include "base/callback_list.h"
 #include "base/strings/string16.h"
+#include "base/timer/timer.h"
 #include "components/infobars/core/confirm_infobar_delegate.h"
 
+class GlobalConfirmInfoBar;
+
 namespace extensions {
 
 // An infobar used to globally warn users that an extension is debugging the
 // browser (which has security consequences).
 class ExtensionDevToolsInfoBarDelegate : public ConfirmInfoBarDelegate {
  public:
+  static constexpr base::TimeDelta kAutoCloseDelay =
+      base::TimeDelta::FromSeconds(5);
   using CallbackList = base::OnceClosureList;
 
   // Ensures a global infobar corresponding to the supplied extension is
@@ -42,6 +47,9 @@ class ExtensionDevToolsInfoBarDelegate : public ConfirmInfoBarDelegate {
   gfx::ElideBehavior GetMessageElideBehavior() const override;
   int GetButtons() const override;
 
+  // Autocloses the infobar_ after 5 seconds.
+  static void NotifyExtensionDetached(const std::string& extension_id);
+
  private:
   ExtensionDevToolsInfoBarDelegate(std::string extension_id,
                                    const std::string& extension_name);
@@ -52,7 +60,12 @@ class ExtensionDevToolsInfoBarDelegate : public ConfirmInfoBarDelegate {
 
   const std::string extension_id_;
   const base::string16 extension_name_;
+  // infobar_ is set after attaching an extension and is deleted 5 seconds after
+  // detaching the extension. |infobar_| owns this object and is therefore
+  // guaranteed to outlive it.
+  GlobalConfirmInfoBar* infobar_ = nullptr;
   CallbackList callback_list_;
+  base::OneShotTimer timer_;
 };
 
 }  // namespace extensions