Add a browser test for functions in notification provider API

chrome/browser/extensions/api/notification_provider/notification_provider_apitest.cc

Index: chrome/browser/extensions/api/notification_provider/notification_provider_apitest.cc
diff --git a/chrome/browser/extensions/api/notification_provider/notification_provider_apitest.cc b/chrome/browser/extensions/api/notification_provider/notification_provider_apitest.cc
index 3ba54097478d91eb4020895d0ecd6c7706a42339..7fcd2975b504a834c8feb440967bbc018465eea9 100644
--- a/chrome/browser/extensions/api/notification_provider/notification_provider_apitest.cc
+++ b/chrome/browser/extensions/api/notification_provider/notification_provider_apitest.cc
@@ -2,7 +2,6 @@
 // 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"
@@ -11,16 +10,27 @@
 
 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());
@@ -30,15 +40,41 @@ IN_PROC_BROWSER_TEST_F(NotificationProviderApiTest, Events) {
       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();
+}