PushMessagingBrowserTest for registration success and failure

chrome/browser/services/gcm/fake_gcm_profile_service.cc

 // Copyright 2013 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/fake_gcm_profile_service.h"
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "chrome/browser/profiles/profile.h"
@@ skipping 94 matching lines @@
 
 // static
 KeyedService* FakeGCMProfileService::Build(content::BrowserContext* context) {
   Profile* profile = static_cast<Profile*>(context);
   FakeGCMProfileService* service = new FakeGCMProfileService(profile);
   service->SetDriverForTesting(new CustomFakeGCMDriver(service));
   return service;
 }
 
 FakeGCMProfileService::FakeGCMProfileService(Profile* profile)
-    : collect_(false) {}
+    : collect_(false) {
+  static_cast<PushMessagingServiceImpl*>(push_messaging_service())
+      ->SetProfileForTesting(profile);

[Peter Beverloo, 2014/10/17 14:04:07]

It looks like PushMessagingServiceImpl is only ever constructed with a NULL
profile from this code-path, which has now been changed with a late
initialization of the profile through the SetProfileForTesting method. That's a
bit silly.

I see that there are two constructors for the GCMProfileService:
  GCMProfileService(Profile)
  GCMProfileService(Profile, GCMClientFactory)

The former is used by Android, the latter by all other platforms. The
PushMessagingServiceImpl constructor has no side effects, so passing this in at
constructor time should be fine, aside from conflicting with the Android
constructor.

I'm not sure if there's a clean way to get this to work. Perhaps we could come
up with some distinct signature for the testing constructor to avoid the
duplication, allowing us to initialize the PushMessagingServiceImpl directly
with the Profile*?

[Michael van Ouwerkerk, 2014/10/17 15:28:01]

It would be nice to just inject the dependencies it needs using the constructor,
but it's a pain because of the platform differences. The late setting of the
dependency seems as reasonable as any other approach. The driver for example is
already set in this way as well.

+}
 
 FakeGCMProfileService::~FakeGCMProfileService() {}
 
 void FakeGCMProfileService::RegisterFinished(
     const std::string& app_id,
     const std::vector<std::string>& sender_ids) {
   if (collect_) {
     last_registered_app_id_ = app_id;
     last_registered_sender_ids_ = sender_ids;
   }
@@ skipping 30 matching lines @@
       static_cast<CustomFakeGCMDriver*>(driver());
   custom_driver->OnSendFinished(app_id, message.id, GCMClient::SUCCESS);
 }
 
 void FakeGCMProfileService::AddExpectedUnregisterResponse(
     GCMClient::Result result) {
   unregister_responses_.push_back(result);
 }
 
 }  // namespace gcm