Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(348)

Unified Diff: components/bubble/bubble_manager.cc

Issue 1572743002: Make sure bubbles in Views default to close before their RenderFrameHosts. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@lkcr
Patch Set: Add a test and only allow one filter in CloseAllMatchingBubbles. Created 4 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « components/bubble/bubble_manager.h ('k') | components/bubble/bubble_manager_mocks.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/bubble/bubble_manager.cc
diff --git a/components/bubble/bubble_manager.cc b/components/bubble/bubble_manager.cc
index 36902a37d4f398d224f138b6cb2c33ea6a5123f3..db27d93b2447212b3c0993c2caf079b8faf5f3c0 100644
--- a/components/bubble/bubble_manager.cc
+++ b/components/bubble/bubble_manager.cc
@@ -47,7 +47,7 @@ bool BubbleManager::CloseBubble(BubbleReference bubble,
BubbleCloseReason reason) {
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK_NE(manager_state_, ITERATING_BUBBLES);
- return CloseAllMatchingBubbles(bubble.get(), reason);
+ return CloseAllMatchingBubbles(bubble.get(), nullptr, reason);
}
void BubbleManager::CloseAllBubbles(BubbleCloseReason reason) {
@@ -56,7 +56,7 @@ void BubbleManager::CloseAllBubbles(BubbleCloseReason reason) {
DCHECK_NE(reason, BUBBLE_CLOSE_CANCELED);
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK_NE(manager_state_, ITERATING_BUBBLES);
- CloseAllMatchingBubbles(nullptr, reason);
+ CloseAllMatchingBubbles(nullptr, nullptr, reason);
}
void BubbleManager::UpdateAllBubbleAnchors() {
@@ -89,19 +89,29 @@ void BubbleManager::FinalizePendingRequests() {
CloseAllBubbles(BUBBLE_CLOSE_FORCED);
}
-bool BubbleManager::CloseAllMatchingBubbles(BubbleController* match,
- BubbleCloseReason reason) {
+void BubbleManager::CloseBubblesOwnedBy(const content::RenderFrameHost* frame) {
+ CloseAllMatchingBubbles(nullptr, frame, BUBBLE_CLOSE_FRAME_DESTROYED);
+}
+
+bool BubbleManager::CloseAllMatchingBubbles(
+ BubbleController* bubble,
+ const content::RenderFrameHost* owner,
+ BubbleCloseReason reason) {
+ DCHECK(!bubble || !owner) << "It doesn't make sense to close a particular "
+ "bubble iff it's owned by a particular frame.";
Peter Kasting 2016/02/09 02:10:52 Nit: Rather than an output string here, I would le
Jeffrey Yasskin 2016/02/09 20:54:44 Done; how's this comment look?
+
ScopedVector<BubbleController> close_queue;
// Guard against bubbles being added or removed while iterating the bubbles.
ManagerState original_state = manager_state_;
manager_state_ = ITERATING_BUBBLES;
- for (auto iter = controllers_.begin(); iter != controllers_.end();) {
- if ((!match || match == *iter) && (*iter)->ShouldClose(reason)) {
- close_queue.push_back(*iter);
- iter = controllers_.weak_erase(iter);
+ for (auto i = controllers_.begin(); i != controllers_.end();) {
+ if ((!bubble || bubble == *i) && (!owner || (*i)->OwningFrameIs(owner)) &&
+ (*i)->ShouldClose(reason)) {
+ close_queue.push_back(*i);
+ i = controllers_.weak_erase(i);
} else {
- ++iter;
+ ++i;
}
}
manager_state_ = original_state;
« no previous file with comments | « components/bubble/bubble_manager.h ('k') | components/bubble/bubble_manager_mocks.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698