Public Sessions - prompt the user for pageCapture requests

chrome/browser/chromeos/extensions/public_session_permission_helper_impl.cc

+// Copyright 2017 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.
+
+#include "chrome/browser/chromeos/extensions/public_session_permission_helper_impl.h"
+
+#include <utility>
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/memory/ptr_util.h"
+#include "content/public/browser/web_contents.h"
+#include "extensions/common/permissions/api_permission_set.h"
+#include "extensions/common/permissions/manifest_permission_set.h"
+#include "extensions/common/permissions/permission_set.h"
+#include "extensions/common/url_pattern_set.h"
+
+namespace extensions {
+namespace permission_helper {
+
+RequestCallback::RequestCallback(
+    const base::Closure& success_callback,
+    const base::Closure& failure_callback,
+    const PermissionHelperSet& permission_list)
+    : success_callback(success_callback),
+      failure_callback(failure_callback),
+      permission_list(permission_list) {}
+
+RequestCallback::RequestCallback(
+    const RequestCallback& other)
+    : success_callback(other.success_callback),
+      failure_callback(other.failure_callback),
+      permission_list(other.permission_list) {}
+
+RequestCallback::~RequestCallback() {}
+
+PublicSessionPermissionHelperImpl::PublicSessionPermissionHelperImpl() {}
+
+PublicSessionPermissionHelperImpl::PublicSessionPermissionHelperImpl(
+    PublicSessionPermissionHelperImpl&& other) = default;
+
+PublicSessionPermissionHelperImpl::~PublicSessionPermissionHelperImpl() {}
+
+void PublicSessionPermissionHelperImpl::HandlePermissionRequest(
+    const Extension* extension,
+    PermissionHelperSet requested_permissions,
+    content::WebContents* web_contents,
+    const base::Closure& success_callback,
+    const base::Closure& failure_callback) {
+  CHECK(web_contents);
+
+  PermissionIDSet requested_permission_set;
+  for (auto permission : requested_permissions) {
+    requested_permission_set.insert(permission);
+  }
+
+  PermissionIDSet unresolved_permissions = PermissionIDSet::Difference(
+      requested_permission_set, allowed_permission_set_);
+  if (unresolved_permissions.empty()) {
+    success_callback.Run();
+    return;
+  }
+  unresolved_permissions = PermissionIDSet::Difference(
+      unresolved_permissions, denied_permission_set_);
+  if (unresolved_permissions.empty()) {
+    failure_callback.Run();

[Devlin, 2017/01/23 22:59:48]

It's a little weird that a request can request multiple permissions and have
some granted, and some not.  When can this happen?  Do we anticipate calling
this with multiple permissions?  If not, removing this could simplify some of
the other code.

[Ivan Šandrk, 2017/01/24 19:57:22]

PublicSessionMediaAccessHandler may ask up to two permissions at the same time.

Request #1 - Audio -> Prompt is shown only for audio
Request #2 - Audio+Video -> Prompt is shown only for video (because we already
prompted audio)

User allows #1, but not #2.

+    return;
+  }
+
+  // Since not all permissions are resolved yet, queue the callback to be called
+  // when all of them are resolved.
+  callbacks_.push_back(RequestCallback(
+      success_callback, failure_callback, requested_permissions));
+
+  PermissionIDSet unprompted_permissions = PermissionIDSet::Difference(
+      unresolved_permissions, prompted_permission_set_);
+  if (unprompted_permissions.empty()) return;

[Devlin, 2017/01/25 16:00:31]

return; should be on a new line

[Ivan Šandrk, 2017/01/26 18:53:21]

Style guide allows this
https://google.github.io/styleguide/cppguide.html#Conditionals: (plus it's used
in other places like this)

Short conditional statements may be written on one line if this enhances
readability. You may use this only when the line is brief and the statement does
not use the else clause.

But then you and I may not agree on the readability part :)

[Devlin, 2017/01/30 17:04:16]

In this case, I'm just going to lean on the fact that it's very rare to do this
in chromium code, and ask to be consistent (since there's no functional
difference).

[Ivan Šandrk, 2017/01/30 18:14:20]

Done.

+
+  APIPermission::ID first_unprompted_permission =
+      unprompted_permissions.begin()->id();
+  APIPermissionSet new_apis;
+  for (const auto& permission : unprompted_permissions) {
+    prompted_permission_set_.insert(permission.id());
+    new_apis.insert(permission.id());
+  }
+  auto permission_set = base::MakeUnique<PermissionSet>(
+      new_apis, ManifestPermissionSet(), URLPatternSet(), URLPatternSet());
+  auto prompt = base::MakeUnique<ExtensionInstallPrompt>(web_contents);
+  prompt->ShowDialog(

[Devlin, 2017/01/23 22:59:48]

Here, it looks like we're anticipating showing multiple of these prompts and
being able to queue them up.  But right now, do we just spam-open them all at
once if an extension requests multiple permissions in separate requests?

[Ivan Šandrk, 2017/01/24 19:57:22]

Yes, spam-open.

+      base::Bind(&PublicSessionPermissionHelperImpl::ResolvePermissionPrompt,
+                 base::Unretained(this), extension,
+                 std::move(unprompted_permissions)),
+      extension,
+      nullptr,  // Use the extension icon.
+      base::MakeUnique<ExtensionInstallPrompt::Prompt>(
+          ExtensionInstallPrompt::PERMISSIONS_PROMPT),
+      std::move(permission_set),
+      ExtensionInstallPrompt::GetDefaultShowDialogCallback());
+  prompt_map_[first_unprompted_permission] = std::move(prompt);
+}
+
+PermissionState PublicSessionPermissionHelperImpl::GetUserChoice(
+    APIPermission::ID permission_id) {
+  if (allowed_permission_set_.ContainsID(permission_id))
+    return PermissionState::ALLOWED;
+  if (denied_permission_set_.ContainsID(permission_id))
+    return PermissionState::DENIED;
+  if (prompted_permission_set_.ContainsID(permission_id))
+    return PermissionState::SHOWN_PROMPT;
+  return PermissionState::NOT_PROMPTED;
+}
+
+void PublicSessionPermissionHelperImpl::ResolvePermissionPrompt(
+    const Extension* extension,
+    PermissionIDSet unprompted_permissions,
+    ExtensionInstallPrompt::Result prompt_result) {
+  // Dispose of the prompt as it's not needed anymore.
+  prompt_map_.erase(unprompted_permissions.begin()->id());
+
+  bool allowed = prompt_result == ExtensionInstallPrompt::Result::ACCEPTED;
+  for (const auto& permission : unprompted_permissions) {
+    prompted_permission_set_.erase(permission.id());
+    if (allowed)
+      allowed_permission_set_.insert(permission.id());
+    else
+      denied_permission_set_.insert(permission.id());
+  }
+
+  for (auto callback = callbacks_.begin();
+       callback != callbacks_.end(); ) {

[Devlin, 2017/01/25 16:00:31]

nit: no space after ;

[Ivan Šandrk, 2017/01/26 18:53:22]

https://google.github.io/styleguide/cppguide.html#Horizontal_Whitespace - Loops
and Conditionals:

// For loops always have a space after the semicolon.  They may have a space
// before the semicolon, but this is rare.

Altho a quick code search shows both cases, with no space coming up more often.

I personally find it more readable this way, but then again you're the owner :)

+    if (prompted_permission_set_.ContainsAnyID(callback->permission_list)) {
+      callback++;
+      continue;
+    }
+    if (denied_permission_set_.ContainsAnyID(callback->permission_list))
+      callback->failure_callback.Run();
+    else
+      callback->success_callback.Run();
+    callbacks_.erase(callback);
+  }
+}
+
+}  // namespace permission_helper
+}  // namespace extensions