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

Side by Side 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: Clean up 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 unified diff | Download patch
OLDNEW
1 // Copyright 2015 The Chromium Authors. All rights reserved. 1 // Copyright 2015 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/bubble/bubble_manager.h" 5 #include "components/bubble/bubble_manager.h"
6 6
7 #include <utility> 7 #include <utility>
8 #include <vector> 8 #include <vector>
9 9
10 #include "components/bubble/bubble_controller.h" 10 #include "components/bubble/bubble_controller.h"
(...skipping 29 matching lines...) Expand all
40 break; 40 break;
41 } 41 }
42 42
43 return bubble_ref; 43 return bubble_ref;
44 } 44 }
45 45
46 bool BubbleManager::CloseBubble(BubbleReference bubble, 46 bool BubbleManager::CloseBubble(BubbleReference bubble,
47 BubbleCloseReason reason) { 47 BubbleCloseReason reason) {
48 DCHECK(thread_checker_.CalledOnValidThread()); 48 DCHECK(thread_checker_.CalledOnValidThread());
49 DCHECK_NE(manager_state_, ITERATING_BUBBLES); 49 DCHECK_NE(manager_state_, ITERATING_BUBBLES);
50 return CloseAllMatchingBubbles(bubble.get(), reason); 50 return CloseAllMatchingBubbles(bubble.get(), nullptr, reason);
51 } 51 }
52 52
53 void BubbleManager::CloseAllBubbles(BubbleCloseReason reason) { 53 void BubbleManager::CloseAllBubbles(BubbleCloseReason reason) {
54 // The following close reasons don't make sense for multiple bubbles: 54 // The following close reasons don't make sense for multiple bubbles:
55 DCHECK_NE(reason, BUBBLE_CLOSE_ACCEPTED); 55 DCHECK_NE(reason, BUBBLE_CLOSE_ACCEPTED);
56 DCHECK_NE(reason, BUBBLE_CLOSE_CANCELED); 56 DCHECK_NE(reason, BUBBLE_CLOSE_CANCELED);
57 DCHECK(thread_checker_.CalledOnValidThread()); 57 DCHECK(thread_checker_.CalledOnValidThread());
58 DCHECK_NE(manager_state_, ITERATING_BUBBLES); 58 DCHECK_NE(manager_state_, ITERATING_BUBBLES);
59 CloseAllMatchingBubbles(nullptr, reason); 59 CloseAllMatchingBubbles(nullptr, nullptr, reason);
60 } 60 }
61 61
62 void BubbleManager::UpdateAllBubbleAnchors() { 62 void BubbleManager::UpdateAllBubbleAnchors() {
63 DCHECK(thread_checker_.CalledOnValidThread()); 63 DCHECK(thread_checker_.CalledOnValidThread());
64 DCHECK_NE(manager_state_, ITERATING_BUBBLES); 64 DCHECK_NE(manager_state_, ITERATING_BUBBLES);
65 65
66 // Guard against bubbles being added or removed while iterating the bubbles. 66 // Guard against bubbles being added or removed while iterating the bubbles.
67 ManagerState original_state = manager_state_; 67 ManagerState original_state = manager_state_;
68 manager_state_ = ITERATING_BUBBLES; 68 manager_state_ = ITERATING_BUBBLES;
69 for (auto controller : controllers_) 69 for (auto controller : controllers_)
(...skipping 12 matching lines...) Expand all
82 82
83 void BubbleManager::FinalizePendingRequests() { 83 void BubbleManager::FinalizePendingRequests() {
84 // Return if already "Finalized". 84 // Return if already "Finalized".
85 if (manager_state_ == NO_MORE_BUBBLES) 85 if (manager_state_ == NO_MORE_BUBBLES)
86 return; 86 return;
87 87
88 manager_state_ = NO_MORE_BUBBLES; 88 manager_state_ = NO_MORE_BUBBLES;
89 CloseAllBubbles(BUBBLE_CLOSE_FORCED); 89 CloseAllBubbles(BUBBLE_CLOSE_FORCED);
90 } 90 }
91 91
92 bool BubbleManager::CloseAllMatchingBubbles(BubbleController* match, 92 void BubbleManager::CloseBubblesOwnedBy(const content::RenderFrameHost* frame) {
93 BubbleCloseReason reason) { 93 CloseAllMatchingBubbles(nullptr, frame, BUBBLE_CLOSE_FRAME_DESTROYED);
94 }
95
96 bool BubbleManager::CloseAllMatchingBubbles(
97 BubbleController* match,
98 const content::RenderFrameHost* frame,
99 BubbleCloseReason reason) {
94 ScopedVector<BubbleController> close_queue; 100 ScopedVector<BubbleController> close_queue;
95 101
96 // Guard against bubbles being added or removed while iterating the bubbles. 102 // Guard against bubbles being added or removed while iterating the bubbles.
97 ManagerState original_state = manager_state_; 103 ManagerState original_state = manager_state_;
98 manager_state_ = ITERATING_BUBBLES; 104 manager_state_ = ITERATING_BUBBLES;
99 for (auto iter = controllers_.begin(); iter != controllers_.end();) { 105 for (auto iter = controllers_.begin(); iter != controllers_.end();) {
100 if ((!match || match == *iter) && (*iter)->ShouldClose(reason)) { 106 bool matches = true;
hcarmona 2016/02/05 22:48:24 If both |match| and |frame| are passed in, should
Jeffrey Yasskin 2016/02/08 23:59:09 Right now, only one is ever specified. I lean towa
hcarmona 2016/02/09 00:31:05 Agreed, lambda would be over-engineering. Can we
Peter Kasting 2016/02/09 00:42:41 Hmm. I'm a little uneasy about using DCHECK like
hcarmona 2016/02/09 00:59:44 I can't think of a reason we would specify a speci
107 if (match && match != *iter)
108 matches = false;
109 if (frame && !(*iter)->OwningFrameIs(frame))
110 matches = false;
111
112 if (matches && (*iter)->ShouldClose(reason)) {
Peter Kasting 2016/02/06 04:13:08 Nit: Shorter and, IMO, more readable due to omitti
Jeffrey Yasskin 2016/02/08 23:59:09 I had some trouble decoding the long boolean expre
101 close_queue.push_back(*iter); 113 close_queue.push_back(*iter);
102 iter = controllers_.weak_erase(iter); 114 iter = controllers_.weak_erase(iter);
103 } else { 115 } else {
104 ++iter; 116 ++iter;
105 } 117 }
106 } 118 }
107 manager_state_ = original_state; 119 manager_state_ = original_state;
108 120
109 for (auto controller : close_queue) { 121 for (auto controller : close_queue) {
110 controller->DoClose(); 122 controller->DoClose();
111 123
112 FOR_EACH_OBSERVER(BubbleManagerObserver, observers_, 124 FOR_EACH_OBSERVER(BubbleManagerObserver, observers_,
113 OnBubbleClosed(controller->AsWeakPtr(), reason)); 125 OnBubbleClosed(controller->AsWeakPtr(), reason));
114 } 126 }
115 127
116 return !close_queue.empty(); 128 return !close_queue.empty();
117 } 129 }
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698