Only send test sms messages on a switch.

Use Shell singleton system/web_notification trays in unittests.

Ash notifications have two problems with unit tests:
1. SmsWatcher only expects to be initialized once. The unit tests were creating one instance of SystemTray in the Shell instance created in AshTestBase, and a second was getting created in SystemTrayTest. Both contain a TraySms that initializes SmsWatcher. I expect there could be other side effects of having two system trays, so using the one in Shell in the unit tests seemed best.
2. The Sms stub was always generating test notifications when the default view was shown, which made the unit test behavior difficult to predict. The solution is to put these test messages on a flag.

BUG=none
TEST=aura_shell_unittests pass with --ash-notify

Review URL: https://chromiumcodereview.appspot.com/10553008

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

6 files changed, 30 insertions, 21 deletions

diff --git a/ash/system/tray/system_tray_unittest.cc b/ash/system/tray/system_tray_unittest.cc
index 0c3aeb70..1fde2c4 100644
--- a/ash/system/tray/system_tray_unittest.cc
+++ b/ash/system/tray/system_tray_unittest.cc
@@ -20,11 +20,8 @@ namespace test {
 
 namespace {
 
-SystemTray* CreateSystemTray() {
-  internal::StatusAreaWidget* widget = new internal::StatusAreaWidget;
-  widget->CreateTrayViews(NULL);
-  widget->Show();
-  return widget->system_tray();
+SystemTray* GetSystemTray() {
+  return Shell::GetInstance()->status_area_widget()->system_tray();
 }
 
 // Trivial item implementation that tracks its views for testing.
@@ -95,7 +92,7 @@ class TestItem : public SystemTrayItem {
 typedef AshTestBase SystemTrayTest;
 
 TEST_F(SystemTrayTest, SystemTrayDefaultView) {
-  scoped_ptr<SystemTray> tray(CreateSystemTray());
+  SystemTray* tray = GetSystemTray();
   ASSERT_TRUE(tray->GetWidget());
 
   tray->ShowDefaultView(BUBBLE_CREATE_NEW);
@@ -107,7 +104,7 @@ TEST_F(SystemTrayTest, SystemTrayDefaultView) {
 }
 
 TEST_F(SystemTrayTest, SystemTrayTestItems) {
-  scoped_ptr<SystemTray> tray(CreateSystemTray());
+  SystemTray* tray = GetSystemTray();
   ASSERT_TRUE(tray->GetWidget());
 
   TestItem* test_item = new TestItem;
@@ -138,7 +135,7 @@ TEST_F(SystemTrayTest, SystemTrayTestItems) {
 }
 
 TEST_F(SystemTrayTest, TrayWidgetAutoResizes) {
-  scoped_ptr<SystemTray> tray(CreateSystemTray());
+  SystemTray* tray = GetSystemTray();
   ASSERT_TRUE(tray->GetWidget());
 
   gfx::Size widget_size = tray->GetWidget()->GetWindowScreenBounds().size();
@@ -164,7 +161,7 @@ TEST_F(SystemTrayTest, TrayWidgetAutoResizes) {
 
 // Disabled due to a use-after-free, see http://crbug.com/127539.
 TEST_F(SystemTrayTest, DISABLED_SystemTrayNotifications) {
-  scoped_ptr<SystemTray> tray(CreateSystemTray());
+  SystemTray* tray = GetSystemTray();
   ASSERT_TRUE(tray->GetWidget());
 
   TestItem* test_item = new TestItem;
@@ -199,7 +196,7 @@ TEST_F(SystemTrayTest, DISABLED_SystemTrayNotifications) {
 }
 
 TEST_F(SystemTrayTest, BubbleCreationTypesTest) {
-  scoped_ptr<SystemTray> tray(CreateSystemTray());
+  SystemTray* tray = GetSystemTray();
   ASSERT_TRUE(tray->GetWidget());
 
   TestItem* test_item = new TestItem;
diff --git a/ash/system/web_notification/web_notification_tray_unittest.cc b/ash/system/web_notification/web_notification_tray_unittest.cc
index a153371..0319032 100644
--- a/ash/system/web_notification/web_notification_tray_unittest.cc
+++ b/ash/system/web_notification/web_notification_tray_unittest.cc
@@ -19,11 +19,8 @@ namespace ash {
 
 namespace {
 
-WebNotificationTray* CreateWebNotificationTray() {
-  internal::StatusAreaWidget* widget = new internal::StatusAreaWidget;
-  widget->CreateTrayViews(NULL);
-  widget->Show();
-  return widget->web_notification_tray();
+WebNotificationTray* GetWebNotificationTray() {
+  return Shell::GetInstance()->status_area_widget()->web_notification_tray();
 }
 
 class TestDelegate : public WebNotificationTray::Delegate {
@@ -77,28 +74,28 @@ class TestDelegate : public WebNotificationTray::Delegate {
 typedef test::AshTestBase WebNotificationTrayTest;
 
 TEST_F(WebNotificationTrayTest, WebNotifications) {
-  scoped_ptr<WebNotificationTray> tray(CreateWebNotificationTray());
+  WebNotificationTray* tray = GetWebNotificationTray();
   scoped_ptr<TestDelegate> delegate(new TestDelegate);
   tray->SetDelegate(delegate.get());
 
   ASSERT_TRUE(tray->GetWidget());
 
   // Adding a notification should show the bubble.
-  delegate->AddNotification(tray.get(), "test_id1");
+  delegate->AddNotification(tray, "test_id1");
   EXPECT_TRUE(tray->bubble() != NULL);
   EXPECT_EQ(1, tray->GetNotificationCount());
-  delegate->AddNotification(tray.get(), "test_id2");
-  delegate->AddNotification(tray.get(), "test_id2");
+  delegate->AddNotification(tray, "test_id2");
+  delegate->AddNotification(tray, "test_id2");
   EXPECT_EQ(2, tray->GetNotificationCount());
   // Ensure that removing a notification removes it from the tray, and signals
   // the delegate.
   EXPECT_TRUE(delegate->HasNotificationId("test_id2"));
-  delegate->RemoveNotification(tray.get(), "test_id2");
+  delegate->RemoveNotification(tray, "test_id2");
   EXPECT_FALSE(delegate->HasNotificationId("test_id2"));
   EXPECT_EQ(1, tray->GetNotificationCount());
 
   // Removing the last notification should hide the bubble.
-  delegate->RemoveNotification(tray.get(), "test_id1");
+  delegate->RemoveNotification(tray, "test_id1");
   EXPECT_EQ(0, tray->GetNotificationCount());
   EXPECT_TRUE(tray->bubble() == NULL);
 }
diff --git a/chromeos/chromeos_switches.cc b/chromeos/chromeos_switches.cc
index 2c1e3ad..2ade84b 100644
--- a/chromeos/chromeos_switches.cc
+++ b/chromeos/chromeos_switches.cc
@@ -10,5 +10,8 @@ namespace switches {
 // Forces the stub implementation of dbus clients.
 const char kDbusStub[] = "dbus-stub";
 
+// Sends test messages on first call to RequestUpdate (stub only).
+const char kSmsTestMessages[] = "sms-test-messages";
+
 }  // namespace switches
 }  // namespace chromeos
diff --git a/chromeos/chromeos_switches.h b/chromeos/chromeos_switches.h
index 5b33dea..5eeb21b 100644
--- a/chromeos/chromeos_switches.h
+++ b/chromeos/chromeos_switches.h
@@ -22,6 +22,7 @@ namespace switches {
 
 // Please keep alphabetized.
 CHROMEOS_EXPORT extern const char kDbusStub[];
+CHROMEOS_EXPORT extern const char kSmsTestMessages[];
 
 }  // namespace switches
 }  // namespace chromeos
diff --git a/chromeos/dbus/gsm_sms_client.cc b/chromeos/dbus/gsm_sms_client.cc
index 2836605..149d743 100644
--- a/chromeos/dbus/gsm_sms_client.cc
+++ b/chromeos/dbus/gsm_sms_client.cc
@@ -8,12 +8,14 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop.h"
 #include "base/stringprintf.h"
 #include "base/stl_util.h"
 #include "base/values.h"
+#include "chromeos/chromeos_switches.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
 #include "dbus/object_proxy.h"
@@ -295,6 +297,9 @@ class GsmSMSClientStubImpl : public GsmSMSClient {
   // GsmSMSClient override.
   virtual void RequestUpdate(const std::string& service_name,
                              const dbus::ObjectPath& object_path) {
+    if (!CommandLine::ForCurrentProcess()->HasSwitch(
+            chromeos::switches::kSmsTestMessages))
+      return;
     if (test_index_ >= 0)
       return;
     test_index_ = 0;
diff --git a/chromeos/dbus/sms_client.cc b/chromeos/dbus/sms_client.cc
index 72df74d..ee9fe10 100644
--- a/chromeos/dbus/sms_client.cc
+++ b/chromeos/dbus/sms_client.cc
@@ -7,12 +7,14 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop.h"
 #include "base/stringprintf.h"
 #include "base/stl_util.h"
 #include "base/values.h"
+#include "chromeos/chromeos_switches.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
 #include "dbus/object_proxy.h"
@@ -82,6 +84,10 @@ class SMSClientStubImpl : public SMSClient {
   virtual void GetAll(const std::string& service_name,
                       const dbus::ObjectPath& object_path,
                       const GetAllCallback& callback) OVERRIDE {
+    if (!CommandLine::ForCurrentProcess()->HasSwitch(
+            chromeos::switches::kSmsTestMessages))
+      return;
+
     // Ownership passed to callback
     base::DictionaryValue *sms = new base::DictionaryValue();
     sms->SetString("Number", "000-000-0000");