Return empty Budget API buckets if there is no notification permission

chrome/browser/budget_service/budget_service_impl.cc

Index: chrome/browser/budget_service/budget_service_impl.cc
diff --git a/chrome/browser/budget_service/budget_service_impl.cc b/chrome/browser/budget_service/budget_service_impl.cc
index 5bbb9b42ea5e048be45223927a32f6f4ff7e053d..a350220d959743cb19f42ae292459a8a2e49d834 100644
--- a/chrome/browser/budget_service/budget_service_impl.cc
+++ b/chrome/browser/budget_service/budget_service_impl.cc
@@ -7,7 +7,10 @@
 #include "base/memory/ptr_util.h"
 #include "chrome/browser/budget_service/budget_manager.h"
 #include "chrome/browser/budget_service/budget_manager_factory.h"
+#include "chrome/browser/permissions/permission_manager.h"
+#include "chrome/browser/permissions/permission_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "content/public/browser/permission_type.h"
 #include "content/public/browser/render_process_host.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 
@@ -50,8 +53,33 @@ void BudgetServiceImpl::GetBudget(const url::Origin& origin,
       content::RenderProcessHost::FromID(render_process_id_);
   DCHECK(host);
 
-  // Query the BudgetManager for the budget.
   content::BrowserContext* context = host->GetBrowserContext();

[johnme, 2017/06/02 16:06:35]

Nit: just do `Profile* profile =
Profile::FromBrowserContext(host->GetBrowserContext());` here since we're in
chrome/, that fits on one line, and you unconditionally convert it to a Profile*
later anyway.

[Peter Beverloo, 2017/06/02 17:40:56]

Done.

+
+  PermissionManager* permission_manager =
+      PermissionManagerFactory::GetForProfile(
+          Profile::FromBrowserContext(context));
+  DCHECK(permission_manager);
+
+  // By request of the Privacy Team, we only communicate the budget buckets with
+  // the developer when the notification permission has been granted. This is
+  // something the impact of which has to be reconsidered when the feature is
+  // ready to ship for real. See https://crbug.com/710809 for context.
+  if (permission_manager->GetPermissionStatus(

[johnme, 2017/06/02 16:06:35]

It's encouraged to use PermissionManager::GetPermissionStatusForFrame instead in
case feature policy has disabled the feature for a particular frame, though I
guess that's moot since this can be accessed from a SW. I'm surprised there
isn't a WebFeaturePolicyFeature for disabling Service Workers since they can be
used to bypass other feature policy entries :-s

In a similar vein, it's a little odd to pass an empty GURL for embedder origin,
though I guess for now NotificationPermissionContext mostly ignores that 
(https://crbug.com/416894).

[Peter Beverloo, 2017/06/02 17:40:56]

This is inherent to how Service Workers work, part unfortunate, part reality. I
don't think we'll want to change this?

+          content::PermissionType::NOTIFICATIONS, origin.GetURL(), GURL()) !=
+      blink::mojom::PermissionStatus::GRANTED) {
+    blink::mojom::BudgetStatePtr empty_state(blink::mojom::BudgetState::New());
+    empty_state->budget_at = 0;
+    empty_state->time = base::Time::Now().ToDoubleT();
+
+    std::vector<blink::mojom::BudgetStatePtr> predictions;
+    predictions.push_back(std::move(empty_state));
+
+    callback.Run(blink::mojom::BudgetServiceErrorType::NONE,
+                 std::move(predictions));
+    return;
+  }
+
+  // Query the BudgetManager for the budget.
   BudgetManagerFactory::GetForProfile(context)->GetBudget(origin, callback);
 }