Add BubbleManager to manage bubbles and ChromeBubbleManager for events.

components/bubble/bubble_controller.h

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_BUBBLE_BUBBLE_CONTROLLER_H__

[msw, 2015/08/13 03:37:22]

nit: remove extra trailing underscore here and below*2.

[hcarmona, 2015/08/15 02:03:19]

Done.

+#define COMPONENTS_BUBBLE_BUBBLE_CONTROLLER_H__
+
+#include "base/callback_forward.h"

[msw, 2015/08/13 03:37:22]

nit: remove if not needed.

[hcarmona, 2015/08/15 02:03:19]

Done.

+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "components/bubble/bubble_ui.h"

[msw, 2015/08/13 03:37:22]

nit: forward declare BubbleUI (like BubbleDelegate) instead, if possible.

[hcarmona, 2015/08/15 02:03:19]

Done.

+#include "ui/gfx/native_widget_types.h"

[msw, 2015/08/13 03:37:22]

nit: remove if not needed.

[hcarmona, 2015/08/15 02:03:19]

Done.

+
+class BubbleDelegate;
+
+enum BubbleCloseReason {
+  // Bubble was closed without any user interaction.
+  BUBBLE_CLOSE_IGNORE,
+
+  // User did not interact with the bubble, but changed tab.

[msw, 2015/08/13 03:37:22]

What's the value of distinguishing this from *_IGNORE? If there is appreciable
value here, maybe we should do the same for switching browser windows, etc.?

[groby-ooo-7-16, 2015/08/13 18:44:38]

There's potentially a different meaning to this - _IGNORE means you kept
interacting with the content, but ignored the request, TABSWITCH means you
switched content. Erring on the side of caution and spelling out separate
dismissal reasons for now, since we'll have metrics based on it.

[msw, 2015/08/13 19:18:58]

Okay, reasonable, but perhaps better generalized as a context switch (there's no
notion of a tab in this component). Also aren't clicking away from the browser
window, navigating the tab, closing the tab, etc. also effectively context
switches? Tab switching while a permission bubble is shown might be more
appropriate for the permission code to track. I'm not too adamant about this
point, if tab switching really is an important metric to measure and relevant to
all bubbles (but I doubt it).

+  BUBBLE_CLOSE_TABSWITCH,
+
+  // User dismissed the bubble. (ESC, close, etc)

[msw, 2015/08/13 03:37:22]

nit: period after "etc"

[hcarmona, 2015/08/15 02:03:19]

Done.

+  BUBBLE_CLOSE_CLOSE,
+
+  // User selected the "Allow" option in a bubble.

[msw, 2015/08/13 03:37:22]

Most bubbles won't have "Allow" and "Deny" options, those sound a little overly
specific for general responses; many bubbles may not even have any clear
response options at all (and I assume that's fine). Still, consider using more
generic terminology here (like views::DialogDelegate's Accept/Cancel, or follow
another more generic patterns like OK/Cancel, Accept/Deny)... Regardless, maybe
revise the comments to indicate that the bubble reports some sort of generic
positive or negative response, not just the literals "Allow" and "Deny".

[groby-ooo-7-16, 2015/08/13 18:44:38]

This definitely refers to the positive/negative reply, not the literal concepts.
Would removing the quotes in the comments be enough to make that clear?

[msw, 2015/08/13 19:18:58]

That might be okay, but allow/deny is much more permission inspired than the
options I outlined above; I'd vote for generic Accept/Cancel dialog terminology.
(I hope someday bubbles and dialogs will share much more code)

[hcarmona, 2015/08/15 02:03:19]

Done.

+  BUBBLE_CLOSE_ALLOW,
+
+  // User selected the "Deny" option in a bubble.
+  BUBBLE_CLOSE_DENY,
+};
+
+// BubbleController is responisble for the lifetime of the delegate and it's UI.

[msw, 2015/08/13 03:37:22]

nit: no apostrophe in "its" here.

[msw, 2015/08/13 19:18:58]

nit: responsible spelling.

[hcarmona, 2015/08/15 02:03:19]

Done.

[hcarmona, 2015/08/15 02:03:19]

Done.

+class BubbleController {
+ public:
+  explicit BubbleController(scoped_ptr<BubbleDelegate> delegate);
+  virtual ~BubbleController();
+
+  void Show();
+  void Hide(BubbleCloseReason reason);
+  void UpdateAnchorPosition();
+  bool ShouldUpdateBubble();

[msw, 2015/08/13 03:37:22]

nit: add comments here and for other non-obvious functions.

[hcarmona, 2015/08/15 02:03:19]

Done.

+
+  bool ShouldClose() const;

[msw, 2015/08/13 03:37:22]

nit: ShouldCloseOnHide? add comments here and for other non-obvious functions.

[hcarmona, 2015/08/15 02:03:19]

Acknowledged.

+  void Close();
+
+  bool MatchesContext(void* context) const;

[msw, 2015/08/13 03:37:22]

nit: add comments here and for other non-obvious functions.

[hcarmona, 2015/08/15 02:03:19]

Done.

+
+  base::WeakPtr<BubbleController> AsWeakPtr();
+
+ private:
+  scoped_ptr<BubbleDelegate> delegate_;
+  scoped_ptr<BubbleUI> bubble_ui_;
+
+  base::WeakPtrFactory<BubbleController> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(BubbleController);
+};
+
+#endif  // COMPONENTS_BUBBLE_BUBBLE_CONTROLLER_H__