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

Side by Side Diff: components/bubble/bubble_manager.cc

Issue 1251633002: Add BubbleManager to manage bubbles and ChromeBubbleManager for events. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Mac Changes to test trybots Created 5 years, 4 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
(Empty)
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
3 // found in the LICENSE file.
4
5 #include "components/bubble/bubble_manager.h"
6
7 #include <vector>
8
9 #include "components/bubble/bubble_controller.h"
10
11 BubbleManager::~BubbleManager() {}
12
13 BubbleReference BubbleManager::ShowBubble(scoped_ptr<BubbleDelegate> bubble) {
14 DCHECK(bubble);
15 scoped_ptr<BubbleController> controller(new BubbleController(bubble.Pass()));
16
17 BubbleReference bubble_ref = controller->AsWeakPtr();
18 ShowBubbleUI(bubble_ref);
19
20 controllers_.push_back(controller.Pass());
21 return bubble_ref;
22 }
23
24 bool BubbleManager::CloseBubble(BubbleReference bubble,
25 BubbleCloseReason reason) {
26 for (auto iter = controllers_.begin(); iter != controllers_.end(); ++iter) {
27 if (*iter == bubble.get()) {
28 if (ShouldClose((*iter)->AsWeakPtr(), reason)) {
29 // Remove the controller before attempting to delete because some
30 // delegates may chain showing more bubbles.
31 BubbleController* controller = *iter;
32 controllers_.weak_erase(iter);
msw 2015/08/18 17:26:19 This should update iter, otherwise it'll be invali
hcarmona 2015/08/18 23:08:48 Done.
33 delete controller;
34 return true;
35 }
36 return false;
37 }
38 }
39
40 // Attempting to close a bubble that is already closed or that this manager
41 // doesn't own is a bug.
42 NOTREACHED();
43 return false;
44 }
45
46 void BubbleManager::UpdateAllBubbleAnchors() {
47 for (auto controller : controllers_) {
msw 2015/08/18 17:26:19 nit: curly braces not needed here.
hcarmona 2015/08/18 23:08:48 Done.
48 UpdateAnchorPosition(controller->AsWeakPtr());
49 }
50 }
51
52 BubbleManager::BubbleManager() {}
53
54 void BubbleManager::CloseAllBubbles(BubbleCloseReason reason) {
55 std::vector<BubbleController*> closing;
56
57 for (auto iter = controllers_.begin(); iter != controllers_.end();) {
58 if (ShouldClose((*iter)->AsWeakPtr(), reason)) {
59 // Remove the controllers before attempting to delete because some
msw 2015/08/18 17:26:19 What happens on BubbleManager destruction (browser
hcarmona 2015/08/18 23:08:48 Yes, I'll create a test for this scenario. Added
msw 2015/08/19 00:18:11 sgtm, thanks.
60 // delegates may chain showing more bubbles.
61 closing.push_back(*iter);
62 iter = controllers_.weak_erase(iter);
63 continue;
64 }
65 ++iter;
66 }
67
68 for (auto iter : closing) {
msw 2015/08/18 17:26:20 Use STLDeleteElements or STLElementDeleter.
hcarmona 2015/08/18 23:08:48 Done.
69 delete iter;
70 // No need to remove from the vector after deleting.
71 }
72 }
73
74 void BubbleManager::ShowBubbleUI(base::WeakPtr<BubbleController> controller) {
msw 2015/08/18 17:26:20 Why not just inline this?
hcarmona 2015/08/18 23:08:48 Can't inline virtual functions. These functions ar
msw 2015/08/19 00:18:11 Sorry, what I meant is that we shouldn't define a
hcarmona 2015/08/20 02:34:31 Done. Moved the UI thread check to here.
75 controller->Show();
76 }
77
78 bool BubbleManager::ShouldClose(base::WeakPtr<BubbleController> controller,
msw 2015/08/18 17:26:20 Ditto: Inline this.
hcarmona 2015/08/18 23:08:48 Same: virtual
79 BubbleCloseReason reason) {
80 return controller->ShouldClose(reason);
81 }
82
83 void BubbleManager::UpdateAnchorPosition(
msw 2015/08/18 17:26:20 Ditto: Inline this.
hcarmona 2015/08/18 23:08:48 Same: virtual
84 base::WeakPtr<BubbleController> controller) {
85 controller->UpdateAnchorPosition();
86 }
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698