Revert r276792 - "Hook PushMessagingMessageFilter up to GCMDriver"

This change caused the sole push_messaging layout test to fail :(.

> Hook PushMessagingMessageFilter up to GCMDriver
>
> Add plumbing for registration from push service-agnostic content layer
> to GCMDriver in chrome layer.
>
> To achieve this, GCMProfileService owns an implementation of a new
> PushMessagingService class, which can be used from the content layer.
>
> Based on mvanouwerkerk's prototype in https://codereview.chromium.org/186023002, but significantly refactored.
>
> BUG=350384
> TBR=benm@chromium.org

TBR=johnme@chromium.org
BUG=350384

Review URL: https://codereview.chromium.org/327423006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@276834 0039d316-1c4b-4281-b951-d872f2087c98

android_webview/browser/aw_browser_context.cc

@@ -318,11 +318,6 @@ quota::SpecialStoragePolicy* AwBrowserContext::GetSpecialStoragePolicy() {
   return NULL;
 }
 
-content::PushMessagingService* AwBrowserContext::GetPushMessagingService() {
-  // TODO(johnme): Support push messaging in WebView.
-  return NULL;
-}
-
 void AwBrowserContext::RebuildTable(
     const scoped_refptr<URLEnumerator>& enumerator) {
   // Android WebView rebuilds from WebChromeClient.getVisitedHistory. The client

android_webview/browser/aw_browser_context.h

@@ -129,7 +129,6 @@ class AwBrowserContext : public content::BrowserContext,
       GetGeolocationPermissionContext() OVERRIDE;
   virtual content::BrowserPluginGuestManager* GetGuestManager() OVERRIDE;
   virtual quota::SpecialStoragePolicy* GetSpecialStoragePolicy() OVERRIDE;
-  virtual content::PushMessagingService* GetPushMessagingService() OVERRIDE;
 
   // visitedlink::VisitedLinkDelegate implementation.
   virtual void RebuildTable(

chrome/browser/profiles/off_the_record_profile_impl.cc

@@ -440,12 +440,6 @@ quota::SpecialStoragePolicy*
   return GetExtensionSpecialStoragePolicy();
 }
 
-content::PushMessagingService*
-OffTheRecordProfileImpl::GetPushMessagingService() {
-  // TODO(johnme): Support push messaging in incognito if possible.
-  return NULL;
-}
-
 bool OffTheRecordProfileImpl::IsSameProfile(Profile* profile) {
   return (profile == this) || (profile == profile_);
 }

chrome/browser/profiles/off_the_record_profile_impl.h

@@ -131,7 +131,6 @@ class OffTheRecordProfileImpl : public Profile {
       GetGeolocationPermissionContext() OVERRIDE;
   virtual content::BrowserPluginGuestManager* GetGuestManager() OVERRIDE;
   virtual quota::SpecialStoragePolicy* GetSpecialStoragePolicy() OVERRIDE;
-  virtual content::PushMessagingService* GetPushMessagingService() OVERRIDE;
 
  private:
   FRIEND_TEST_ALL_PREFIXES(OffTheRecordProfileImplTest, GetHostZoomMap);

chrome/browser/profiles/profile_impl.cc

@@ -69,8 +69,6 @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profile_metrics.h"
 #include "chrome/browser/search_engines/template_url_fetcher.h"
-#include "chrome/browser/services/gcm/gcm_profile_service.h"
-#include "chrome/browser/services/gcm/gcm_profile_service_factory.h"
 #include "chrome/browser/sessions/session_service_factory.h"
 #include "chrome/browser/ui/startup/startup_browser_creator.h"
 #include "chrome/browser/ui/webui/extensions/extension_icon_source.h"
@@ -1091,11 +1089,6 @@ quota::SpecialStoragePolicy* ProfileImpl::GetSpecialStoragePolicy() {
   return GetExtensionSpecialStoragePolicy();
 }
 
-content::PushMessagingService* ProfileImpl::GetPushMessagingService() {
-  return gcm::GCMProfileServiceFactory::GetForProfile(
-      this)->push_messaging_service();
-}
-
 bool ProfileImpl::IsSameProfile(Profile* profile) {
   if (profile == static_cast<Profile*>(this))
     return true;

chrome/browser/profiles/profile_impl.h

@@ -105,7 +105,6 @@ class ProfileImpl : public Profile {
       GetGeolocationPermissionContext() OVERRIDE;
   virtual content::BrowserPluginGuestManager* GetGuestManager() OVERRIDE;
   virtual quota::SpecialStoragePolicy* GetSpecialStoragePolicy() OVERRIDE;
-  virtual content::PushMessagingService* GetPushMessagingService() OVERRIDE;
 
   // Profile implementation:
   virtual scoped_refptr<base::SequencedTaskRunner> GetIOTaskRunner() OVERRIDE;

chrome/browser/services/gcm/gcm_profile_service.cc

@@ -44,8 +44,7 @@ void GCMProfileService::RegisterProfilePrefs(
 
 #if defined(OS_ANDROID)
 GCMProfileService::GCMProfileService(Profile* profile)
-    : profile_(profile),
-      push_messaging_service_(this) {
+    : profile_(profile) {
   DCHECK(!profile->IsOffTheRecord());
 
   driver_.reset(new GCMDriverAndroid);
@@ -54,8 +53,7 @@ GCMProfileService::GCMProfileService(Profile* profile)
 GCMProfileService::GCMProfileService(
     Profile* profile,
     scoped_ptr<GCMClientFactory> gcm_client_factory)
-    : profile_(profile),
-      push_messaging_service_(this) {
+    : profile_(profile) {
   DCHECK(!profile->IsOffTheRecord());
 
   driver_ = CreateGCMDriverDesktop(
@@ -69,9 +67,7 @@ GCMProfileService::GCMProfileService(
 }
 #endif  // defined(OS_ANDROID)
 
-GCMProfileService::GCMProfileService()
-    : profile_(NULL),
-      push_messaging_service_(this) {
+GCMProfileService::GCMProfileService() : profile_(NULL) {
 }
 
 GCMProfileService::~GCMProfileService() {

chrome/browser/services/gcm/gcm_profile_service.h

@@ -10,7 +10,6 @@
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
-#include "chrome/browser/services/gcm/push_messaging_service_impl.h"
 // TODO(jianli): include needed for obsolete methods that are going to be
 // removed soon.
 #include "components/gcm_driver/gcm_driver.h"
@@ -59,10 +58,6 @@ class GCMProfileService : public KeyedService {
 
   GCMDriver* driver() const { return driver_.get(); }
 
-  content::PushMessagingService* push_messaging_service() {
-    return &push_messaging_service_;
-  }
-
  protected:
   // Used for constructing fake GCMProfileService for testing purpose.
   GCMProfileService();
@@ -73,9 +68,6 @@ class GCMProfileService : public KeyedService {
 
   scoped_ptr<GCMDriver> driver_;
 
-  // Implementation of content::PushMessagingService using GCMProfileService.
-  PushMessagingServiceImpl push_messaging_service_;
-
   DISALLOW_COPY_AND_ASSIGN(GCMProfileService);
 };
 

chrome/browser/services/gcm/push_messaging_service_impl.cc

-// Copyright 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.
-
-#include "chrome/browser/services/gcm/push_messaging_service_impl.h"
-
-#include <vector>
-
-#include "base/bind.h"
-#include "chrome/browser/services/gcm/gcm_profile_service.h"
-#include "components/gcm_driver/gcm_driver.h"
-
-namespace gcm {
-
-PushMessagingServiceImpl::PushMessagingServiceImpl(
-    GCMProfileService* gcm_profile_service)
-    : gcm_profile_service_(gcm_profile_service),
-      weak_factory_(this) {}
-
-PushMessagingServiceImpl::~PushMessagingServiceImpl() {}
-
-void PushMessagingServiceImpl::Register(
-    const std::string& app_id,
-    const std::string& sender_id,
-    const content::PushMessagingService::RegisterCallback& callback) {
-  // The GCMDriver could be NULL if GCMProfileService has been shut down.
-  if (!gcm_profile_service_->driver())
-    return;
-  std::vector<std::string> sender_ids(1, sender_id);
-  gcm_profile_service_->driver()->Register(
-      app_id,
-      sender_ids,
-      base::Bind(&PushMessagingServiceImpl::DidRegister,
-                 weak_factory_.GetWeakPtr(),
-                 callback));
-}
-
-void PushMessagingServiceImpl::DidRegister(
-    const content::PushMessagingService::RegisterCallback& callback,
-    const std::string& registration_id,
-    GCMClient::Result result) {
-  GURL endpoint = GURL("https://android.googleapis.com/gcm/send");
-  bool error = (result != GCMClient::SUCCESS);
-  callback.Run(endpoint, registration_id, error);
-}
-
-}  // namespace gcm

chrome/browser/services/gcm/push_messaging_service_impl.h

-// Copyright 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_SERVICES_GCM_PUSH_MESSAGING_SERVICE_IMPL_H_
-#define CHROME_BROWSER_SERVICES_GCM_PUSH_MESSAGING_SERVICE_IMPL_H_
-
-#include "base/compiler_specific.h"
-#include "base/memory/weak_ptr.h"
-#include "components/gcm_driver/gcm_client.h"
-#include "content/public/browser/push_messaging_service.h"
-
-namespace gcm {
-
-class GCMProfileService;
-
-class PushMessagingServiceImpl : public content::PushMessagingService {
- public:
-  explicit PushMessagingServiceImpl(GCMProfileService* gcm_profile_service);
-  virtual ~PushMessagingServiceImpl();
-
-  // content::PushMessagingService implementation:
-  virtual void Register(
-      const std::string& app_id,
-      const std::string& sender_id,
-      const content::PushMessagingService::RegisterCallback& callback) OVERRIDE;
-
- private:
-  void DidRegister(
-      const content::PushMessagingService::RegisterCallback& callback,
-      const std::string& registration_id,
-      GCMClient::Result result);
-
-  GCMProfileService* gcm_profile_service_;  // It owns us.
-
-  base::WeakPtrFactory<PushMessagingServiceImpl> weak_factory_;
-
-  DISALLOW_COPY_AND_ASSIGN(PushMessagingServiceImpl);
-};
-
-}  // namespace gcm
-
-#endif  // CHROME_BROWSER_SERVICES_GCM_PUSH_MESSAGING_SERVICE_IMPL_H_

chrome/browser/ui/app_list/test/fake_profile.cc

@@ -102,10 +102,6 @@ quota::SpecialStoragePolicy* FakeProfile::GetSpecialStoragePolicy() {
   return NULL;
 }
 
-content::PushMessagingService* FakeProfile::GetPushMessagingService() {
-  return NULL;
-}
-
 scoped_refptr<base::SequencedTaskRunner>
 FakeProfile::GetIOTaskRunner() {
   return scoped_refptr<base::SequencedTaskRunner>();

chrome/browser/ui/app_list/test/fake_profile.h

@@ -71,7 +71,6 @@ class FakeProfile : public Profile {
       GetGeolocationPermissionContext() OVERRIDE;
   virtual content::BrowserPluginGuestManager* GetGuestManager() OVERRIDE;
   virtual quota::SpecialStoragePolicy* GetSpecialStoragePolicy() OVERRIDE;
-  virtual content::PushMessagingService* GetPushMessagingService() OVERRIDE;
   virtual scoped_refptr<base::SequencedTaskRunner> GetIOTaskRunner() OVERRIDE;
   virtual Profile* GetOffTheRecordProfile() OVERRIDE;
   virtual void DestroyOffTheRecordProfile() OVERRIDE;

chrome/chrome_browser.gypi

@@ -2035,8 +2035,6 @@
         'browser/services/gcm/gcm_profile_service_factory.h',
         'browser/services/gcm/gcm_desktop_utils.cc',
         'browser/services/gcm/gcm_desktop_utils.h',
-        'browser/services/gcm/push_messaging_service_impl.cc',
-        'browser/services/gcm/push_messaging_service_impl.h',
         'browser/service_process/service_process_control.cc',
         'browser/service_process/service_process_control_mac.mm',
         'browser/service_process/service_process_control.h',

chrome/test/base/testing_profile.cc

@@ -838,10 +838,6 @@ content::BrowserPluginGuestManager* TestingProfile::GetGuestManager() {
 #endif
 }
 
-content::PushMessagingService* TestingProfile::GetPushMessagingService() {
-  return NULL;
-}
-
 bool TestingProfile::IsSameProfile(Profile *p) {
   return this == p;
 }

chrome/test/base/testing_profile.h

@@ -219,7 +219,6 @@ class TestingProfile : public Profile {
       GetGeolocationPermissionContext() OVERRIDE;
   virtual content::BrowserPluginGuestManager* GetGuestManager() OVERRIDE;
   virtual quota::SpecialStoragePolicy* GetSpecialStoragePolicy() OVERRIDE;
-  virtual content::PushMessagingService* GetPushMessagingService() OVERRIDE;
 
   virtual TestingProfile* AsTestingProfile() OVERRIDE;
 

content/browser/download/download_manager_impl_unittest.cc

@@ -440,7 +440,6 @@ class MockBrowserContext : public BrowserContext {
                GeolocationPermissionContext* ());
   MOCK_METHOD0(GetGuestManager, BrowserPluginGuestManager* ());
   MOCK_METHOD0(GetSpecialStoragePolicy, quota::SpecialStoragePolicy*());
-  MOCK_METHOD0(GetPushMessagingService, PushMessagingService*());
 };
 
 class MockDownloadManagerObserver : public DownloadManager::Observer {

content/browser/push_messaging_message_filter.cc

@@ -6,20 +6,13 @@
 
 #include <string>
 
-#include "base/bind.h"
-#include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/common/push_messaging_messages.h"
-#include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/push_messaging_service.h"
 
 namespace content {
 
-PushMessagingMessageFilter::PushMessagingMessageFilter(int render_process_id)
-    : BrowserMessageFilter(PushMessagingMsgStart),
-      render_process_id_(render_process_id),
-      service_(NULL),
-      weak_factory_(this) {}
+PushMessagingMessageFilter::PushMessagingMessageFilter()
+    : BrowserMessageFilter(PushMessagingMsgStart) {}
 
 PushMessagingMessageFilter::~PushMessagingMessageFilter() {}
 
@@ -36,33 +29,19 @@ bool PushMessagingMessageFilter::OnMessageReceived(
 void PushMessagingMessageFilter::OnRegister(int routing_id,
                                             int callbacks_id,
                                             const std::string& sender_id) {
+  // TODO(mvanouwerkerk): Really implement, the below simply returns an error.
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  // TODO(mvanouwerkerk): Validate arguments?
-  // TODO(mvanouwerkerk): A WebContentsObserver could avoid this PostTask
-  //                      by receiving the IPC on the UI thread.
+  GURL endpoint = GURL("https://android.googleapis.com/gcm/send");
   BrowserThread::PostTask(BrowserThread::UI,
                           FROM_HERE,
-                          base::Bind(&PushMessagingMessageFilter::DoRegister,
-                                     weak_factory_.GetWeakPtr(),
+                          base::Bind(&PushMessagingMessageFilter::DidRegister,
+                                     this,
                                      routing_id,
                                      callbacks_id,
-                                     sender_id));
-}
+                                     endpoint,
+                                     "",
+                                     true));
 
-void PushMessagingMessageFilter::DoRegister(int routing_id,
-                                            int callbacks_id,
-                                            const std::string& sender_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (!service())
-    return;
-  // TODO(mvanouwerkerk): Pass in app ID based on Service Worker ID.
-  std::string app_id = "unknown-app-id";
-  service_->Register(app_id,
-                     sender_id,
-                     base::Bind(&PushMessagingMessageFilter::DidRegister,
-                                weak_factory_.GetWeakPtr(),
-                                routing_id,
-                                callbacks_id));
 }
 
 void PushMessagingMessageFilter::DidRegister(int routing_id,
@@ -81,15 +60,4 @@ void PushMessagingMessageFilter::DidRegister(int routing_id,
   }
 }
 
-PushMessagingService* PushMessagingMessageFilter::service() {
-  if (!service_) {
-    RenderProcessHostImpl* host = static_cast<RenderProcessHostImpl*>(
-        RenderProcessHost::FromID(render_process_id_));
-    if (!host)
-      return NULL;
-    service_ = host->GetBrowserContext()->GetPushMessagingService();
-  }
-  return service_;
-}
-
 }  // namespace content

content/browser/push_messaging_message_filter.h

@@ -7,17 +7,14 @@
 
 #include <string>
 
-#include "base/memory/weak_ptr.h"
 #include "content/public/browser/browser_message_filter.h"
 #include "url/gurl.h"
 
 namespace content {
 
-class PushMessagingService;
-
 class PushMessagingMessageFilter : public BrowserMessageFilter {
  public:
-  explicit PushMessagingMessageFilter(int render_process_id);
+  PushMessagingMessageFilter();
 
  private:
   virtual ~PushMessagingMessageFilter();
@@ -29,23 +26,12 @@ class PushMessagingMessageFilter : public BrowserMessageFilter {
                   int callbacks_id,
                   const std::string& sender_id);
 
-  void DoRegister(int routing_id,
-                  int callbacks_id,
-                  const std::string& sender_id);
-
   void DidRegister(int routing_id,
                    int callbacks_id,
                    const GURL& endpoint,
                    const std::string& registration_id,
                    bool error);
 
-  PushMessagingService* service();
-
-  int render_process_id_;
-  PushMessagingService* service_; // Not owned.
-
-  base::WeakPtrFactory<PushMessagingMessageFilter> weak_factory_;
-
   DISALLOW_COPY_AND_ASSIGN(PushMessagingMessageFilter);
 };
 

content/browser/renderer_host/render_process_host_impl.cc

@@ -888,7 +888,7 @@ void RenderProcessHostImpl::CreateMessageFilters() {
   AddFilter(new VibrationMessageFilter());
   screen_orientation_dispatcher_host_ = new ScreenOrientationDispatcherHost();
   AddFilter(screen_orientation_dispatcher_host_);
-  AddFilter(new PushMessagingMessageFilter(GetID()));
+  AddFilter(new PushMessagingMessageFilter());
   AddFilter(new BatteryStatusMessageFilter());
 }
 

content/public/browser/browser_context.h

@@ -37,7 +37,6 @@ class DownloadManager;
 class DownloadManagerDelegate;
 class GeolocationPermissionContext;
 class IndexedDBContext;
-class PushMessagingService;
 class ResourceContext;
 class SiteInstance;
 class StoragePartition;
@@ -184,11 +183,6 @@ class CONTENT_EXPORT BrowserContext : public base::SupportsUserData {
 
   // Returns a special storage policy implementation, or NULL.
   virtual quota::SpecialStoragePolicy* GetSpecialStoragePolicy() = 0;
-
-  // Returns a push messaging service. The embedder owns the service, and is
-  // responsible for ensuring that it outlives RenderProcessHost. It's valid to
-  // return NULL.
-  virtual PushMessagingService* GetPushMessagingService() = 0;
 };
 
 }  // namespace content

content/public/browser/push_messaging_service.h

-// Copyright 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 CONTENT_PUBLIC_BROWSER_PUSH_MESSAGING_SERVICE_H_
-#define CONTENT_PUBLIC_BROWSER_PUSH_MESSAGING_SERVICE_H_
-
-#include <string>
-
-#include "base/callback.h"
-#include "content/common/content_export.h"
-#include "url/gurl.h"
-
-namespace content {
-
-// A push service-agnostic interface that the Push API uses for talking to
-// push messaging services like GCM.
-class CONTENT_EXPORT PushMessagingService {
- public:
-  typedef base::Callback<void(const GURL& /* endpoint */,
-                              const std::string& /* registration_id */,
-                              bool /* error */)>
-      RegisterCallback;
-
-  virtual ~PushMessagingService() {}
-  virtual void Register(const std::string& app_id,
-                        const std::string& sender_id,
-                        const RegisterCallback& callback) = 0;
-};
-
-}  // namespace content
-
-#endif  // CONTENT_PUBLIC_BROWSER_PUSH_MESSAGING_SERVICE_H_

content/public/test/test_browser_context.cc

@@ -151,8 +151,4 @@ quota::SpecialStoragePolicy* TestBrowserContext::GetSpecialStoragePolicy() {
   return special_storage_policy_.get();
 }
 
-PushMessagingService* TestBrowserContext::GetPushMessagingService() {
-  return NULL;
-}
-
 }  // namespace content

content/public/test/test_browser_context.h

@@ -66,7 +66,6 @@ class TestBrowserContext : public BrowserContext {
       GetGeolocationPermissionContext() OVERRIDE;
   virtual BrowserPluginGuestManager* GetGuestManager() OVERRIDE;
   virtual quota::SpecialStoragePolicy* GetSpecialStoragePolicy() OVERRIDE;
-  virtual PushMessagingService* GetPushMessagingService() OVERRIDE;
 
  private:
   FRIEND_TEST_ALL_PREFIXES(DOMStorageTest, SessionOnly);

content/shell/browser/shell_browser_context.cc

@@ -243,8 +243,4 @@ quota::SpecialStoragePolicy* ShellBrowserContext::GetSpecialStoragePolicy() {
   return NULL;
 }
 
-PushMessagingService* ShellBrowserContext::GetPushMessagingService() {
-  return NULL;
-}
-
 }  // namespace content

content/shell/browser/shell_browser_context.h

@@ -74,7 +74,6 @@ class ShellBrowserContext : public BrowserContext {
       GetGeolocationPermissionContext() OVERRIDE;
   virtual BrowserPluginGuestManager* GetGuestManager() OVERRIDE;
   virtual quota::SpecialStoragePolicy* GetSpecialStoragePolicy() OVERRIDE;
-  virtual PushMessagingService* GetPushMessagingService() OVERRIDE;
 
   net::URLRequestContextGetter* CreateRequestContext(
       ProtocolHandlerMap* protocol_handlers,