Push API: use an enum to indicate status.

BUG=350377

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

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

chrome/browser/services/gcm/push_messaging_service_impl.cc

@@ -142,7 +142,11 @@ void PushMessagingServiceImpl::Register(
 
   if (profile_->GetPrefs()->GetInteger(
           prefs::kPushMessagingRegistrationCount) >= kMaxRegistrations) {
-    DidRegister(app_id, callback, std::string(), GCMClient::UNKNOWN_ERROR);
+    RegisterEnd(
+        app_id,
+        callback,
+        std::string(),
+        content::PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_LIMIT_REACHED);
     return;
   }
 
@@ -177,7 +181,11 @@ void PushMessagingServiceImpl::Register(
       gcm::PushMessagingPermissionContextFactory::GetForProfile(profile_);
 
   if (permission_context == NULL) {
-    DidRegister(app_id, callback, std::string(), GCMClient::UNKNOWN_ERROR);
+    RegisterEnd(
+        app_id,
+        callback,
+        std::string(),
+        content::PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_PERMISSION_DENIED);
     return;
   }
 
@@ -193,15 +201,14 @@ void PushMessagingServiceImpl::Register(
                  callback));
 }
 
-void PushMessagingServiceImpl::DidRegister(
+void PushMessagingServiceImpl::RegisterEnd(
     const std::string& app_id,
     const content::PushMessagingService::RegisterCallback& callback,
     const std::string& registration_id,
-    GCMClient::Result result) {
+    content::PushMessagingStatus status) {
   GURL endpoint = GURL("https://android.googleapis.com/gcm/send");
-  bool success = (result == GCMClient::SUCCESS);
-  callback.Run(endpoint, registration_id, success);
-  if (success) {
+  callback.Run(endpoint, registration_id, status);
+  if (status == content::PUSH_MESSAGING_STATUS_OK) {
     // TODO(johnme): Make sure the pref doesn't get out of sync after crashes.
     int registration_count = profile_->GetPrefs()->GetInteger(
         prefs::kPushMessagingRegistrationCount);
@@ -210,15 +217,29 @@ void PushMessagingServiceImpl::DidRegister(
   }
 }
 
+void PushMessagingServiceImpl::DidRegister(
+    const std::string& app_id,
+    const content::PushMessagingService::RegisterCallback& callback,
+    const std::string& registration_id,
+    GCMClient::Result result) {
+  content::PushMessagingStatus status =
+      result == GCMClient::SUCCESS
+          ? content::PUSH_MESSAGING_STATUS_OK
+          : content::PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_SERVICE_ERROR;
+  RegisterEnd(app_id, callback, registration_id, status);
+}
+
 void PushMessagingServiceImpl::DidRequestPermission(
     const std::string& sender_id,
     const std::string& app_id,
     const content::PushMessagingService::RegisterCallback& register_callback,
     bool allow) {
   if (!allow) {
-    // TODO(miguelg) extend the error enum to allow for pemission failure.
-    DidRegister(app_id, register_callback, std::string(),
-                GCMClient::UNKNOWN_ERROR);
+    RegisterEnd(
+        app_id,
+        register_callback,
+        std::string(),
+        content::PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_PERMISSION_DENIED);
     return;
   }
 

chrome/browser/services/gcm/push_messaging_service_impl.h

@@ -10,6 +10,7 @@
 #include "components/gcm_driver/gcm_app_handler.h"
 #include "components/gcm_driver/gcm_client.h"
 #include "content/public/browser/push_messaging_service.h"
+#include "content/public/common/push_messaging_status.h"
 
 class Profile;
 
@@ -54,6 +55,12 @@ class PushMessagingServiceImpl : public content::PushMessagingService,
       const content::PushMessagingService::RegisterCallback& callback) OVERRIDE;
 
  private:
+  void RegisterEnd(
+      const std::string& app_id,
+      const content::PushMessagingService::RegisterCallback& callback,
+      const std::string& registration_id,
+      content::PushMessagingStatus status);
+
   void DidRegister(
       const std::string& app_id,
       const content::PushMessagingService::RegisterCallback& callback,

content/browser/push_messaging_message_filter.cc

@@ -50,7 +50,10 @@ void PushMessagingMessageFilter::OnRegister(int render_frame_id,
       service_worker_context_->context()->GetProviderHost(
           render_process_id_, service_worker_provider_id);
   if (!service_worker_host || !service_worker_host->active_version()) {
-    Send(new PushMessagingMsg_RegisterError(render_frame_id, callbacks_id));
+    Send(new PushMessagingMsg_RegisterError(
+        render_frame_id,
+        callbacks_id,
+        PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_NO_SERVICE_WORKER));
     return;
   }
   BrowserThread::PostTask(
@@ -75,7 +78,10 @@ void PushMessagingMessageFilter::DoRegister(
     int64 service_worker_registration_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (!service()) {
-    Send(new PushMessagingMsg_RegisterError(render_frame_id, callbacks_id));
+    Send(new PushMessagingMsg_RegisterError(
+        render_frame_id,
+        callbacks_id,
+        PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_SERVICE_NOT_AVAILABLE));
     return;
   }
   // TODO(mvanouwerkerk): Is this the app_id format we want to use?
@@ -97,13 +103,14 @@ void PushMessagingMessageFilter::DidRegister(
     int callbacks_id,
     const GURL& push_endpoint,
     const std::string& push_registration_id,
-    bool success) {
+    PushMessagingStatus status) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (success) {
+  if (status == PUSH_MESSAGING_STATUS_OK) {
     Send(new PushMessagingMsg_RegisterSuccess(
         render_frame_id, callbacks_id, push_endpoint, push_registration_id));
   } else {
-    Send(new PushMessagingMsg_RegisterError(render_frame_id, callbacks_id));
+    Send(new PushMessagingMsg_RegisterError(
+        render_frame_id, callbacks_id, status));
   }
 }
 

content/browser/push_messaging_message_filter.h

@@ -10,6 +10,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "content/public/browser/browser_message_filter.h"
+#include "content/public/common/push_messaging_status.h"
 #include "url/gurl.h"
 
 namespace content {
@@ -46,7 +47,7 @@ class PushMessagingMessageFilter : public BrowserMessageFilter {
                    int callbacks_id,
                    const GURL& push_endpoint,
                    const std::string& push_registration_id,
-                   bool success);
+                   PushMessagingStatus status);
 
   PushMessagingService* service();
 

content/common/push_messaging_messages.h

@@ -5,11 +5,15 @@
 // IPC messages for push messaging.
 // Multiply-included message file, hence no include guard.
 
+#include "content/public/common/push_messaging_status.h"
 #include "ipc/ipc_message_macros.h"
 #include "url/gurl.h"
 
 #define IPC_MESSAGE_START PushMessagingMsgStart
 
+IPC_ENUM_TRAITS_MAX_VALUE(content::PushMessagingStatus,
+                          content::PUSH_MESSAGING_STATUS_LAST)
+
 // Messages sent from the browser to the renderer.
 
 IPC_MESSAGE_ROUTED3(PushMessagingMsg_RegisterSuccess,
@@ -17,8 +21,9 @@ IPC_MESSAGE_ROUTED3(PushMessagingMsg_RegisterSuccess,
                     GURL /* push_endpoint */,
                     std::string /* push_registration_id */)
 
-IPC_MESSAGE_ROUTED1(PushMessagingMsg_RegisterError,
-                    int32 /* callbacks_id */)
+IPC_MESSAGE_ROUTED2(PushMessagingMsg_RegisterError,
+                    int32 /* callbacks_id */,
+                    content::PushMessagingStatus /* status */)
 
 // Messages sent from the renderer to the browser.
 

content/content_common.gypi

@@ -87,6 +87,8 @@
       'public/common/pepper_plugin_info.cc',
       'public/common/pepper_plugin_info.h',
       'public/common/process_type.h',
+      'public/common/push_messaging_status.cc',
+      'public/common/push_messaging_status.h',
       'public/common/referrer.h',
       'public/common/renderer_preferences.cc',
       'public/common/renderer_preferences.h',

content/public/browser/push_messaging_service.h

@@ -9,6 +9,7 @@
 
 #include "base/callback.h"
 #include "content/common/content_export.h"
+#include "content/public/common/push_messaging_status.h"
 #include "url/gurl.h"
 
 namespace content {
@@ -19,7 +20,7 @@ class CONTENT_EXPORT PushMessagingService {
  public:
   typedef base::Callback<void(const GURL& /* endpoint */,
                               const std::string& /* registration_id */,
-                              bool /* success */)>
+                              PushMessagingStatus /* status */)>
       RegisterCallback;
 
   virtual ~PushMessagingService() {}

content/public/common/push_messaging_status.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 "content/public/common/push_messaging_status.h"
+
+#include "base/logging.h"
+
+namespace content {
+
+const char* PushMessagingStatusToString(PushMessagingStatus status) {
+  switch (status) {
+    case PUSH_MESSAGING_STATUS_OK:
+      return "Operation has succeeded";
+
+    case PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_NO_SERVICE_WORKER:
+      return "Registration failed - no Service Worker";
+
+    case PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_SERVICE_NOT_AVAILABLE:
+      return "Registration failed - push service not available";
+
+    case PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_LIMIT_REACHED:
+      return "Registration failed - registration limit has been reached";
+
+    case PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_PERMISSION_DENIED:
+      return "Registration failed - permission denied";
+
+    case PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_SERVICE_ERROR:
+      return "Registration failed - push service error";
+
+    case PUSH_MESSAGING_STATUS_ERROR:
+      return "Operation has failed (unspecified reason)";
+  }
+  NOTREACHED();
+  return "";
+}
+
+}  // namespace content

content/public/common/push_messaging_status.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_COMMON_PUSH_MESSAGING_STATUS_STATUS_H_
+#define CONTENT_PUBLIC_COMMON_PUSH_MESSAGING_STATUS_STATUS_H_
+
+namespace content {
+
+enum PushMessagingStatus {
+  // Everything is ok.
+  PUSH_MESSAGING_STATUS_OK,
+
+  // Registration failed because there is no Service Worker.
+  PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_NO_SERVICE_WORKER,
+
+  // Registration failed because the push service is not available.
+  PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_SERVICE_NOT_AVAILABLE,
+
+  // Registration failed because the maximum number of registratons has been
+  // reached.
+  PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_LIMIT_REACHED,
+
+  // Registration failed because permission was denied.
+  PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_PERMISSION_DENIED,
+
+  // Registration failed in the push service implemented by the embedder.
+  PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_SERVICE_ERROR,
+
+  // Generic error (a more specific error should be used whenever possible).
+  PUSH_MESSAGING_STATUS_ERROR,
+
+  // Used for IPC message range checks.
+  PUSH_MESSAGING_STATUS_LAST = PUSH_MESSAGING_STATUS_ERROR
+};
+
+const char* PushMessagingStatusToString(PushMessagingStatus status);
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_COMMON_PUSH_MESSAGING_STATUS_STATUS_H_

content/renderer/push_messaging_dispatcher.cc

@@ -16,10 +16,6 @@
 
 using blink::WebString;
 
-namespace {
-const char kAbortErrorReason[] = "Registration failed.";
-}
-
 namespace content {
 
 PushMessagingDispatcher::PushMessagingDispatcher(RenderFrame* render_frame)
@@ -42,9 +38,10 @@ void PushMessagingDispatcher::registerPushMessaging(
     const WebString& sender_id,
     blink::WebPushRegistrationCallbacks* callbacks) {
   DCHECK(callbacks);
-  scoped_ptr<blink::WebPushError> error(
-      new blink::WebPushError(blink::WebPushError::ErrorTypeAbort,
-                              WebString::fromUTF8(kAbortErrorReason)));
+  scoped_ptr<blink::WebPushError> error(new blink::WebPushError(
+      blink::WebPushError::ErrorTypeAbort,
+      WebString::fromUTF8(PushMessagingStatusToString(
+          PUSH_MESSAGING_STATUS_REGISTRATION_FAILED_NO_SERVICE_WORKER))));
   callbacks->onError(error.release());
   delete callbacks;
 }
@@ -81,15 +78,15 @@ void PushMessagingDispatcher::OnRegisterSuccess(
   registration_callbacks_.Remove(callbacks_id);
 }
 
-void PushMessagingDispatcher::OnRegisterError(int32 callbacks_id) {
+void PushMessagingDispatcher::OnRegisterError(int32 callbacks_id,
+                                              PushMessagingStatus status) {
   blink::WebPushRegistrationCallbacks* callbacks =
       registration_callbacks_.Lookup(callbacks_id);
   CHECK(callbacks);
 
-  scoped_ptr<blink::WebPushError> error(
-      new blink::WebPushError(
-          blink::WebPushError::ErrorTypeAbort,
-          WebString::fromUTF8(kAbortErrorReason)));
+  scoped_ptr<blink::WebPushError> error(new blink::WebPushError(
+      blink::WebPushError::ErrorTypeAbort,
+      WebString::fromUTF8(PushMessagingStatusToString(status))));
   callbacks->onError(error.release());
   registration_callbacks_.Remove(callbacks_id);
 }

content/renderer/push_messaging_dispatcher.h

@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/id_map.h"
+#include "content/public/common/push_messaging_status.h"
 #include "content/public/renderer/render_frame_observer.h"
 #include "third_party/WebKit/public/platform/WebPushClient.h"
 
@@ -48,7 +49,7 @@ class PushMessagingDispatcher : public RenderFrameObserver,
                          const GURL& endpoint,
                          const std::string& registration_id);
 
-  void OnRegisterError(int32 callbacks_id);
+  void OnRegisterError(int32 callbacks_id, PushMessagingStatus status);
 
   IDMap<blink::WebPushRegistrationCallbacks, IDMapOwnPointer>
       registration_callbacks_;

content/shell/renderer/test_runner/mock_web_push_client.cc

@@ -15,9 +15,8 @@ using blink::WebString;
 namespace content {
 
 MockWebPushClient::MockWebPushClient()
-  // The default state should be be an error with "Registration failed." message
-  // because LayoutTests are currently depending on that.
-  : error_message_("Registration failed.") {
+    : error_message_(
+          "Registration failed (default mock client error message)") {
 }
 
 MockWebPushClient::~MockWebPushClient() {}