Restore logic to unsubscribe from permission changes on frame change.

In r441491 I neglected to notice that the lifetime of the
PermissionManager is longer than that of PermissionServiceContext. On
frame changes existing subscriptions should be cleared as they were
before that patch.

PermissionServiceImpl::AddPermissionObserver removes redundant
early return logic checking that
browser_context->GetPermissionManager() is non null.
CreateSubscription already does that.

BUG=677774,678664
TBR=mlamouri@chromium.org

Review-Url: https://codereview.chromium.org/2617863003
Cr-Commit-Position: refs/heads/master@{#442042}

content/browser/permissions/permission_service_context.cc

@@ -27,7 +27,15 @@ class PermissionServiceContext::PermissionSubscription {
         &PermissionSubscription::OnConnectionError, base::Unretained(this)));
   }
 
-  ~PermissionSubscription() = default;
+  ~PermissionSubscription() {
+    DCHECK_NE(id_, 0);
+    BrowserContext* browser_context = context_->GetBrowserContext();
+    DCHECK(browser_context);
+    if (browser_context->GetPermissionManager()) {
+      browser_context->GetPermissionManager()
+          ->UnsubscribePermissionStatusChange(id_);
+    }
+  }
 
   void OnConnectionError() {
     DCHECK_NE(id_, 0);
@@ -105,13 +113,6 @@ void PermissionServiceContext::ServiceHadConnectionError(
 }
 
 void PermissionServiceContext::ObserverHadConnectionError(int subscription_id) {
-  BrowserContext* browser_context = GetBrowserContext();
-  DCHECK(browser_context);
-  if (browser_context->GetPermissionManager()) {
-    browser_context->GetPermissionManager()->UnsubscribePermissionStatusChange(
-        subscription_id);
-  }
-
   auto it = subscriptions_.find(subscription_id);
   DCHECK(it != subscriptions_.end());
   subscriptions_.erase(it);
@@ -139,13 +140,15 @@ void PermissionServiceContext::DidNavigateAnyFrame(
 }
 
 void PermissionServiceContext::CancelPendingOperations(
-    RenderFrameHost* render_frame_host) const {
+    RenderFrameHost* render_frame_host) {
   DCHECK(render_frame_host_);
   if (render_frame_host != render_frame_host_)
     return;
 
   for (const auto& service : services_)
     service->CancelPendingOperations();
+
+  subscriptions_.clear();
 }
 
 BrowserContext* PermissionServiceContext::GetBrowserContext() const {

content/browser/permissions/permission_service_context.h

@@ -70,7 +70,7 @@ class PermissionServiceContext : public WebContentsObserver {
                            const LoadCommittedDetails& details,
                            const FrameNavigateParams& params) override;
 
-  void CancelPendingOperations(RenderFrameHost*) const;
+  void CancelPendingOperations(RenderFrameHost*);
 
   RenderFrameHost* render_frame_host_;
   RenderProcessHost* render_process_host_;

content/browser/permissions/permission_service_impl.cc

@@ -257,11 +257,6 @@ void PermissionServiceImpl::AddPermissionObserver(
     last_known_status = current_status;
   }
 
-  BrowserContext* browser_context = context_->GetBrowserContext();
-  DCHECK(browser_context);
-  if (!browser_context->GetPermissionManager())
-    return;
-
   context_->CreateSubscription(PermissionDescriptorToPermissionType(permission),
                                origin, std::move(observer));
 }