[WebsiteSettings] API for permissions bubbles.

chrome/browser/ui/website_settings/permission_bubble_manager.h

Index: chrome/browser/ui/website_settings/permission_bubble_manager.h
diff --git a/chrome/browser/ui/website_settings/permission_bubble_manager.h b/chrome/browser/ui/website_settings/permission_bubble_manager.h
new file mode 100644
index 0000000000000000000000000000000000000000..21b1c70efb7e6dfce32db27c79978b61887949a6
--- /dev/null
+++ b/chrome/browser/ui/website_settings/permission_bubble_manager.h
@@ -0,0 +1,50 @@
+// Copyright (c) 2014 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 CHROME_BROWSER_UI_WEBSITE_SETTINGS_PERMISSION_BUBBLE_MANAGER_H_
+#define CHROME_BROWSER_UI_WEBSITE_SETTINGS_PERMISSION_BUBBLE_MANAGER_H_
+
+#include "content/public/browser/web_contents_observer.h"
+#include "content/public/browser/web_contents_user_data.h"
+
+class PermissionBubbleDelegate;
+
+// Provides access to permissions bubbles. Allows clients to add a delegate
+// callback interface to the existing permission bubble configuration.
+// Depending on the situation and policy, that may add new UI to an existing
+// permission bubble, create and show a new permission bubble, or provide no
+// visible UI action at all. (In that case, the delegate will be immediately
+// informed that the permission request failed.)
+//
+// A PermissionBubbleManager is associated with a particular WebContents.
+// Delegates attached to a particular WebContents' PBM should arrange to outlive
+// the associated WebContents.

[groby-ooo-7-16, 2014/01/07 00:31:17]

Why should permission requests outlive their WebContents? Aren't they implicitly
tied to WebContents?

[Greg Billock, 2014/01/07 17:59:15]

I botched this. I mean the "WebContents' PBM" but that's a bit of a mouthfull.
I'll reword.

[groby-ooo-7-16, 2014/01/07 19:44:57]

Since the requests are managed by the PBM, I'm not sure why they should outlive
it. Again, this goes to ownership model - I don't think permission requests need
to be owned by anybody but the PBM.

[Greg Billock, 2014/01/07 20:31:19]

Take this case: tab 1 requests a permission. User switches to tab 2. User
switches back to tab 1. User allows the permission request.

Something in the scope of tab 1 has to remain alive during this entire process
to collect the response to the permission request. Something has to re-populate
the bubble contents after the tab change. Currently in infobars the delegate
does this, and the forwards responses to an inner member. This works basically
the same, but leaves that whole object graph within the client. So the UI code
handles the UI object graph, the client code handles the response object graph.
The manager is the API mediator.

[groby-ooo-7-16, 2014/01/07 22:28:41]

PBM is in the scope of tab1, no? It's tied to WebContents.

And I don't _want_ the graph within the client, because it puts burden on the
implementers. The use case, as I see it, is that we have various facilities that
a) Ask for user permissions to do something
b) Do something as the user reacts.

There's no need for them to manage the UI object graph directly, because they
shouldn't be concerned with is. It's a query and a callback. 

+//
+// The PermissionBubbleManager should be addressed on the UI thread.
+class PermissionBubbleManager
+    : public content::WebContentsObserver,
+      public content::WebContentsUserData<PermissionBubbleManager> {
+ public:
+  // Add a new consumer delegate to the permission bubble. Ownership of the
+  // delegate remains with the caller. The caller must arrange for the delegate
+  // to outlive the PermissionBubbleManager.

[groby-ooo-7-16, 2014/01/07 00:31:17]

Why? What is the benefit for the caller to keep ownership?

[Greg Billock, 2014/01/07 17:59:15]

We need some sort of pathway to get back to them. They have to implement an
interface which is kept alive the life of the bubble anyway, so consolidating it
there is convenient and hard to screw up.

In the infobar system, the delegates are owned by the infobar, so they all have
another layer of indirection to reconnect to their target object, with
auto-implicit lifetime arrangements.

[groby-ooo-7-16, 2014/01/07 19:44:57]

Why do we need to get back to them? What is the use case there?

And I'm all for keeping it alive during the bubble duration - that's why PBM
should own them. And manage the bubble, so it can delete the delegates when the
bubble goes away.

[Greg Billock, 2014/01/07 20:31:19]

The "user allowed your permission request" use case. That's universally true,
otherwise, the client wouldn't even bother calling in the first place.
 
Then you have the infobar architecture, which gets the job done, but with N
separate lifetime arrangements between delegate and client. That makes more
sense for infobars, since a lot of the delegates aren't about requests, but
basically go off and call other global browser functions. For permissions, where
each invocation is associated with a particular request, there's no need to
double-joint it.

[groby-ooo-7-16, 2014/01/07 22:28:41]

But the caller doesn't need the object for the pathway back. There will be a
OnCancel/OnSuccess on the delegate, which needs to talk to the caller. When does
the caller need to talk to the delegate? I feel I'm missing something here -
please help me find it :)

As I see it, the caller does something akin to
AskUserPermissionForContent(web_content, new
SpecificPermissionBubbleDelegate(some_state_associated_with_request));

The delegate will do the right thing for both cancel and success. It will
utilize the state associated with the request to do that, whatever that state
is. 

What I fail to see is where the delegate is ever kept around. The current
InfoBar architecture doesn't keep the delegate around, either?

+  virtual void AddPermissionBubbleDelegate(PermissionBubbleDelegate* delegate);

[groby-ooo-7-16, 2014/01/07 00:31:17]

This is requesting a permission - adding the delegate is merely an
implementation detail. I'd prefer the name to reflect the intent, not the
implementation.

[Greg Billock, 2014/01/07 17:59:15]

Yeah, I toyed with a couple more focused names, but didn't find anything I liked
better. One reason is that this is intended to be UI-only -- it's not intended
to encompass all the permissions policy and fussing about with the profile and
such. If we can abstract that to some functional helper, that's more where the
"RequestPermission" type call belongs.

[groby-ooo-7-16, 2014/01/07 19:44:57]

Let's not bikeshed on it right now (mostly because I don't have a good name :),
but the final version hopefully won't talk about type names. (I still like
"AddPermissionRequest", because that's what it does. Maybe
"AddUserPermissionRequest" or somesuch)

[Greg Billock, 2014/01/07 20:31:19]

I think you may have a different idea of the scope of this class. The scope I'm
envisioning for it is extremely confined: show the permissions UI, and that's
it. If there's profile policy negotiation to be done, or permission policy
investigative work to do, the caller has to do it. There may be an abstraction
that's useful for that, but this code doesn't do it. It just puts the pixels on
the screen when it is told.

[groby-ooo-7-16, 2014/01/07 22:28:41]

I think my idea of the scope of the _delegate_ is what differs. The work of
doing policy work is on it. (That's why I suggested in my original e-mail that
we have a Context/Controller that's passed in to the permission request)

+
+  // Remove a consumer delegate from the permission bubble.

[groby-ooo-7-16, 2014/01/07 00:31:17]

In which case would this happen?

[Greg Billock, 2014/01/07 17:59:15]

Not sure, but if history is any guide, it's sure to.

[groby-ooo-7-16, 2014/01/07 19:44:57]

Well, let's add it when it is :) (More lifecycle - if PBM owns the requests, no
need to retract them)

[Greg Billock, 2014/01/07 20:31:19]

Sure, but then retraction becomes awkward, and you end up with ReplaceInfoBar
type of architecture, where you crawl through the existing delegates, using
homebrew RTTI to figure out if you've ever added an equivalent one before.
That's specifically the thing I'm avoiding here -- that logic must all be
external, and if you decide you hate the permission you asked for, and want a
new one, then you just Remove(old); Add(new).

[groby-ooo-7-16, 2014/01/07 22:28:41]

But that means that this is handled individually by each delegate. I don't think
policies are wildly different, and having each delegate implement this increases
the burden on the API consumer. And that is what currently happens in the
current infobar architecture - all the delegates call ReplaceInfoBar. There's no
need for it.

All that needs to happen in PBM is:
* see if there's an existing infobar with equivalent type. if
(newDelegate->IsEquivalent(otherDelegate)...) - yes, it's a bit of RTTI, but no
casting/switching on types.
* If there is, ask the new infobar to subsume the old one, replacing the old
delegate with the new one. otherDelegate->Merge(newDelegate)
* Potentially update the UI

This allows delegates to implement specific logic around replacement/mergin
without being concerned what the visual expression is. (I.e no need to
Add/Remove other delegates)

+  virtual void RemovePermissionBubbleDelegate(
+      PermissionBubbleDelegate* delegate);
+
+ private:
+  friend class content::WebContentsUserData<PermissionBubbleManager>;
+
+  explicit PermissionBubbleManager(content::WebContents* web_contents);
+
+  // contents::WebContentsObserver:
+  virtual void WebContentsDestroyed(
+      content::WebContents* web_contents) OVERRIDE;
+
+  std::vector<PermissionBubbleDelegate*> delegates_;
+};
+
+#endif  // CHROME_BROWSER_UI_WEBSITE_SETTINGS_PERMISSION_BUBBLE_MANAGER_H_