Don't close infobar on first chrome.debugger.detach
Steps to reproduce: Call chrome.debugger.attach for two different
targets (different tabs), then call chrome.debugger.detach for
only one of the two targets, test that only the one target specified
is detached.
Fixed: 1489587
Change-Id: I3a77fbb79a8275feba7896440d348283907cda4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4910999
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1207292}
diff --git a/chrome/browser/extensions/api/debugger/debugger_api.cc b/chrome/browser/extensions/api/debugger/debugger_api.cc
index 09694b7..b5a5b6d3 100644
--- a/chrome/browser/extensions/api/debugger/debugger_api.cc
+++ b/chrome/browser/extensions/api/debugger/debugger_api.cc
@@ -396,7 +396,6 @@
}
ExtensionDevToolsClientHost::~ExtensionDevToolsClientHost() {
- ExtensionDevToolsInfoBarDelegate::NotifyExtensionDetached(extension_id());
g_attached_client_hosts.Get().erase(this);
// Decrement the associated worker keepalive, if any.
diff --git a/chrome/browser/extensions/api/debugger/debugger_apitest.cc b/chrome/browser/extensions/api/debugger/debugger_apitest.cc
index 92b635b..9f769f86 100644
--- a/chrome/browser/extensions/api/debugger/debugger_apitest.cc
+++ b/chrome/browser/extensions/api/debugger/debugger_apitest.cc
@@ -464,6 +464,92 @@
EXPECT_EQ(0u, manager->infobar_count());
}
+IN_PROC_BROWSER_TEST_F(DebuggerApiTest,
+ InfoBarIsNotRemovedWhenAnotherDebuggerAttached) {
+ const int tab_id1 = sessions::SessionTabHelper::IdForTab(
+ browser()->tab_strip_model()->GetActiveWebContents())
+ .id();
+ infobars::ContentInfoBarManager* manager =
+ infobars::ContentInfoBarManager::FromWebContents(
+ browser()->tab_strip_model()->GetActiveWebContents());
+
+ ASSERT_TRUE(embedded_test_server()->Start());
+ ASSERT_TRUE(ui_test_utils::NavigateToURLWithDisposition(
+ browser(), embedded_test_server()->GetURL("/simple.html"),
+ WindowOpenDisposition::NEW_FOREGROUND_TAB,
+ ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP));
+ const int tab_id2 = sessions::SessionTabHelper::IdForTab(
+ browser()->tab_strip_model()->GetActiveWebContents())
+ .id();
+
+ // Attaching to a tab should create an infobar.
+ {
+ auto attach_function = base::MakeRefCounted<DebuggerAttachFunction>();
+ attach_function->set_extension(extension());
+ ASSERT_TRUE(api_test_utils::RunFunction(
+ attach_function.get(),
+ base::StringPrintf("[{\"tabId\": %d}, \"1.1\"]", tab_id1), profile()));
+ }
+
+ EXPECT_EQ(1u, manager->infobar_count());
+
+ // Attaching to a 2nd tab, to have another attached debugger.
+ {
+ auto attach_function = base::MakeRefCounted<DebuggerAttachFunction>();
+ attach_function->set_extension(extension());
+ ASSERT_TRUE(api_test_utils::RunFunction(
+ attach_function.get(),
+ base::StringPrintf("[{\"tabId\": %d}, \"1.1\"]", tab_id2), profile()));
+ }
+
+ EXPECT_EQ(1u, manager->infobar_count());
+
+ // Detaching from the tab should not remove the infobar after 5 seconds, as
+ // another debugger is still attached.
+ {
+ auto detach_function = base::MakeRefCounted<DebuggerDetachFunction>();
+ detach_function->set_extension(extension());
+ ASSERT_TRUE(api_test_utils::RunFunction(
+ detach_function.get(), base::StringPrintf("[{\"tabId\": %d}]", tab_id1),
+ profile()));
+ }
+
+ // Advance the clock by 5 seconds.
+ {
+ base::RunLoop run_loop;
+ base::SingleThreadTaskRunner::GetCurrentDefault()->PostDelayedTask(
+ FROM_HERE, run_loop.QuitClosure(),
+ ExtensionDevToolsInfoBarDelegate::kAutoCloseDelay);
+ AdvanceClock(ExtensionDevToolsInfoBarDelegate::kAutoCloseDelay);
+ run_loop.Run();
+ }
+
+ // Verify inforbar not removed.
+ EXPECT_EQ(1u, manager->infobar_count());
+
+ // Now detach the last debugger.
+ {
+ auto detach_function = base::MakeRefCounted<DebuggerDetachFunction>();
+ detach_function->set_extension(extension());
+ ASSERT_TRUE(api_test_utils::RunFunction(
+ detach_function.get(), base::StringPrintf("[{\"tabId\": %d}]", tab_id2),
+ profile()));
+ }
+
+ // Advance the clock by 5 seconds, once again.
+ {
+ base::RunLoop run_loop;
+ base::SingleThreadTaskRunner::GetCurrentDefault()->PostDelayedTask(
+ FROM_HERE, run_loop.QuitClosure(),
+ ExtensionDevToolsInfoBarDelegate::kAutoCloseDelay);
+ AdvanceClock(ExtensionDevToolsInfoBarDelegate::kAutoCloseDelay);
+ run_loop.Run();
+ }
+
+ // Verify inforbar removed.
+ EXPECT_EQ(0u, manager->infobar_count());
+}
+
class CrossProfileDebuggerApiTest : public DebuggerApiTest {
protected:
Profile* other_profile() { return other_profile_; }
diff --git a/chrome/browser/extensions/api/debugger/extension_dev_tools_infobar_delegate.cc b/chrome/browser/extensions/api/debugger/extension_dev_tools_infobar_delegate.cc
index 88dcd56..da679fc 100644
--- a/chrome/browser/extensions/api/debugger/extension_dev_tools_infobar_delegate.cc
+++ b/chrome/browser/extensions/api/debugger/extension_dev_tools_infobar_delegate.cc
@@ -61,18 +61,6 @@
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_.get(),
- &GlobalConfirmInfoBar::Close);
- }
-}
-
infobars::InfoBarDelegate::InfoBarIdentifier
ExtensionDevToolsInfoBarDelegate::GetIdentifier() const {
return EXTENSION_DEV_TOOLS_INFOBAR_DELEGATE;
@@ -114,7 +102,11 @@
std::string extension_id,
const std::string& extension_name)
: extension_id_(std::move(extension_id)),
- extension_name_(base::UTF8ToUTF16(extension_name)) {}
+ extension_name_(base::UTF8ToUTF16(extension_name)) {
+ callback_list_.set_removal_callback(base::BindRepeating(
+ &ExtensionDevToolsInfoBarDelegate::MaybeStartAutocloseTimer,
+ base::Unretained(this)));
+}
base::CallbackListSubscription
ExtensionDevToolsInfoBarDelegate::RegisterDestroyedCallback(
@@ -122,4 +114,12 @@
return callback_list_.Add(std::move(destroyed_callback));
}
+void ExtensionDevToolsInfoBarDelegate::MaybeStartAutocloseTimer() {
+ if (callback_list_.empty()) {
+ // infobar_ was set in Create() which makes the following access safe.
+ timer_.Start(FROM_HERE, kAutoCloseDelay, infobar_.get(),
+ &GlobalConfirmInfoBar::Close);
+ }
+}
+
} // namespace extensions
diff --git a/chrome/browser/extensions/api/debugger/extension_dev_tools_infobar_delegate.h b/chrome/browser/extensions/api/debugger/extension_dev_tools_infobar_delegate.h
index 48fb3aa..9363aac 100644
--- a/chrome/browser/extensions/api/debugger/extension_dev_tools_infobar_delegate.h
+++ b/chrome/browser/extensions/api/debugger/extension_dev_tools_infobar_delegate.h
@@ -47,9 +47,6 @@
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);
@@ -58,6 +55,9 @@
base::CallbackListSubscription RegisterDestroyedCallback(
base::OnceClosure destroyed_callback);
+ // Autocloses the infobar_ after 5 seconds, only when no callbacks remain.
+ void MaybeStartAutocloseTimer();
+
const ExtensionId extension_id_;
const std::u16string extension_name_;
// infobar_ is set after attaching an extension and is deleted 5 seconds after
