Add a browser test for functions in notification provider API

chrome/browser/extensions/api/notification_provider/notification_provider_apitest.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 "base/memory/scoped_ptr.h"
 #include "chrome/browser/extensions/api/notification_provider/notification_provider_api.h"
 #include "chrome/browser/extensions/chrome_extension_function.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/api/notification_provider.h"
 
 typedef ExtensionApiTest NotificationProviderApiTest;
 
+namespace {
+
+void CreateNotificationOptionsForTest(

[Pete Williamson, 2014/08/04 20:30:45]

Great job refactoring this!  This is exactly the kind of thing we want to do as
we add more code to a project.

[liyanhou, 2014/08/04 23:24:16]

Thank you!

+    extensions::api::notifications::NotificationOptions* options) {
+  options->type = extensions::api::notifications::ParseTemplateType("basic");
+  options->icon_url = scoped_ptr<std::string>(new std::string("icon.png"));
+  options->title = scoped_ptr<std::string>(new std::string("Title"));
+  options->message =
+      scoped_ptr<std::string>(new std::string("Here goes the message"));
+  return;
+}
+
+}  // namespace
+
 IN_PROC_BROWSER_TEST_F(NotificationProviderApiTest, Events) {
   std::string sender_id1 = "SenderId1";
   std::string notification_id1 = "NotificationId1";
 
-  extensions::api::notifications::NotificationOptions options;
-  options.type = extensions::api::notifications::ParseTemplateType("basic");
-  options.icon_url = scoped_ptr<std::string>(new std::string("icon.png"));
-  options.title = scoped_ptr<std::string>(new std::string("Title"));
-  options.message =
-      scoped_ptr<std::string>(new std::string("Here goes the message"));
+  scoped_ptr<extensions::api::notifications::NotificationOptions> options(
+      new extensions::api::notifications::NotificationOptions());
+  CreateNotificationOptionsForTest(options.get());
 
   ResultCatcher catcher;
   catcher.RestrictToProfile(browser()->profile());
 
   // Test notification provider extension
   const extensions::Extension* extension =
       LoadExtension(test_data_dir_.AppendASCII("notification_provider/events"));
   ASSERT_TRUE(extension);
 
-  scoped_ptr<extensions::NotificationProviderEventRouter> event_router(
-      new extensions::NotificationProviderEventRouter(browser()->profile()));
+  extensions::NotificationProviderEventRouter* event_router =

[Pete Williamson, 2014/08/04 20:30:45]

Why remove the use of scoped_ptr here?  If we did mean to remove it, we should
have a delete, but probably better still to put it back.

[liyanhou, 2014/08/04 23:24:16]

Done. Sorry that was a mistake.

+      new extensions::NotificationProviderEventRouter(browser()->profile());
 
   event_router->CreateNotification(
-      extension->id(), sender_id1, notification_id1, options);
+      extension->id(), sender_id1, notification_id1, *options.get());
   event_router->UpdateNotification(
-      extension->id(), sender_id1, notification_id1, options);
+      extension->id(), sender_id1, notification_id1, *options.release());
   event_router->ClearNotification(
       extension->id(), sender_id1, notification_id1);
 
   EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
 }
+
+IN_PROC_BROWSER_TEST_F(NotificationProviderApiTest, TestBasicUsage) {
+  // set up content of a notification
+  std::string sender_id1 = "SenderId";
+  std::string notification_id1 = "NotificationId";
+
+  scoped_ptr<extensions::api::notifications::NotificationOptions> options(
+      new extensions::api::notifications::NotificationOptions());
+  CreateNotificationOptionsForTest(options.get());
+
+  ResultCatcher catcher;
+  catcher.RestrictToProfile(browser()->profile());
+
+  // Test notification provider extension
+  const extensions::Extension* extension = LoadExtension(

[Pete Williamson, 2014/08/04 20:30:46]

Do we ever free this memory?  Might be good to use a smart pointer.  While the
bar is lower for tests, we should still prevent memory leaks when running the
test.  If nothing else, it will help us know when a leak check of a test fails
that the problem is likely to be in the code under test instead of the test
itself.

[liyanhou, 2014/08/04 23:24:16]

Do we need to free this memory if it's not new'd?

+      test_data_dir_.AppendASCII("notification_provider/basic_usage"));
+  ASSERT_TRUE(extension);
+
+  extensions::NotificationProviderEventRouter* event_router =

[Pete Williamson, 2014/08/04 20:30:45]

ccoped_ptr here too.

[liyanhou, 2014/08/04 23:24:16]

Done.

+      new extensions::NotificationProviderEventRouter(browser()->profile());
+
+  event_router->CreateNotification(
+      extension->id(), sender_id1, notification_id1, *options.release());
+
+  EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
+}