Send notification to users upon receiving sms messages

ash/common/system/chromeos/network/sms_observer_unittest.cc

+// Copyright 2016 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 "ash/common/shelf/shelf_widget.h"
+#include "ash/common/shelf/wm_shelf.h"
+#include "ash/common/system/chromeos/network/sms_observer.h"
+#include "ash/public/interfaces/vpn_list.mojom.h"
+#include "ash/test/ash_test_base.h"
+#include "base/macros.h"
+#include "base/strings/utf_string_conversions.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/message_center/message_center.h"
+#include "ui/message_center/notification.h"
+#include "ui/message_center/notification_list.h"
+
+using ash::mojom::ThirdPartyVpnProvider;
+using ash::mojom::ThirdPartyVpnProviderPtr;

[tdanderson, 2017/01/26 22:19:06]

Why are these needed?

[yiyix, 2017/02/02 20:43:56]

Removed. It is leftover of some random trials i have.

[tdanderson, 2017/02/07 00:15:45]

Acknowledged.

+
+namespace ash {
+namespace test {
+
+class SmsObserverTest : public AshTestBase {
+ public:
+  SmsObserverTest() {}
+  ~SmsObserverTest() override {}
+
+  const char* kNotification = "chrome://network/sms";
+  SmsObserver* GetSmsObserver(ShelfWidget* shelf_widget) {
+    return shelf_widget->sms_observer_;
+  }
+
+  // Get the last sms notification from notification center.
+  const message_center::Notification* GetDisplayNotification() const {
+    const message_center::NotificationList::Notifications notifications =
+        message_center::MessageCenter::Get()->GetVisibleNotifications();
+    for (message_center::NotificationList::Notifications::const_iterator iter =

[tdanderson, 2017/01/26 22:19:07]

consider using a c++11 range-based for loop instead of an iterator here

[yiyix, 2017/02/02 20:43:56]

Done.

[tdanderson, 2017/02/07 00:15:45]

I'm a bit confused... all of this code seems to have been removed in Patch Set
2. Was that intentional?

[yiyix, 2017/02/07 22:03:22]

Yes, it is intentional. This method only returns the first notification with the
given id, so it doesn't really help me with cases where they are multiple
notifications. 

Also I check if notification center is empty before sending the sms message, so
there should not be other notification in the notification center, i.e., a
filter like this should not be needed. I also believe it is okay to fail the
test if more notification is created upon receiving sms messages.

[tdanderson, 2017/02/08 16:32:51]

Acknowledged.

+             notifications.begin();
+         iter != notifications.end(); ++iter) {

[tdanderson, 2017/01/26 22:19:06]

can you run 'git cl format ash' on this CL? the formatting looks a bit odd in
places

[yiyix, 2017/02/02 20:43:56]

It's weird. I did it... didn't change this format.

[tdanderson, 2017/02/07 00:15:45]

Acknowledged.

+      if ((*iter)->id().find(kNotification) != std::string::npos)
+        return *iter;
+    }
+
+    return NULL;

[tdanderson, 2017/01/26 22:19:06]

nit: nullptr

[yiyix, 2017/02/02 20:43:56]

Done.

+  }
+
+  base::string16 GetDisplayNotificationText() const {
+    const message_center::Notification* notification = GetDisplayNotification();
+    return notification ? notification->title() : base::string16();
+  }
+
+  base::string16 GetDisplayNotificationAdditionalText() const {
+    const message_center::Notification* notification = GetDisplayNotification();
+    return notification ? notification->message() : base::string16();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SmsObserverTest);
+};
+
+// Verify if notification is received after receiving a sms message with
+// number and content.
+TEST_F(SmsObserverTest, SendTextMessage) {
+  SmsObserver* sms_observer = GetSmsObserver(GetPrimaryShelf()->shelf_widget());
+  EXPECT_FALSE(GetDisplayNotification());
+  base::DictionaryValue* sms = new base::DictionaryValue();
+  sms->SetString("number", "000-000-0000");
+  sms->SetString("text", "FakeSMSClient: Test Message.");
+  sms->SetString("timestamp", "Fri Jun  8 13:26:04 EDT 2016");
+
+  sms_observer->MessageReceived(*sms);
+  EXPECT_TRUE(GetDisplayNotification());
+  EXPECT_EQ(GetDisplayNotification()->title(),
+            base::ASCIIToUTF16("000-000-0000"));
+  EXPECT_EQ(GetDisplayNotification()->message(),
+            base::ASCIIToUTF16("FakeSMSClient: Test Message."));
+  message_center::MessageCenter::Get()->RemoveAllNotifications(
+      false /* by_user */, message_center::MessageCenter::RemoveType::ALL);
+  EXPECT_FALSE(GetDisplayNotification());
+}
+
+// Verify if no notification is received if phone number is missing in sms
+// message.
+TEST_F(SmsObserverTest, TextMessageMissingNumber) {
+  SmsObserver* sms_observer = GetSmsObserver(GetPrimaryShelf()->shelf_widget());
+  EXPECT_FALSE(GetDisplayNotification());
+  base::DictionaryValue* sms = new base::DictionaryValue();
+  sms->SetString("text", "FakeSMSClient: Test Message.");
+  sms->SetString("timestamp", "Fri Jun  8 13:26:04 EDT 2016");
+
+  sms_observer->MessageReceived(*sms);
+  EXPECT_FALSE(GetDisplayNotification());
+}
+
+// Verify if no notification is received if text body is empty in sms message.
+TEST_F(SmsObserverTest, TextMessageEmptyText) {
+  SmsObserver* sms_observer = GetSmsObserver(GetPrimaryShelf()->shelf_widget());
+  EXPECT_FALSE(GetDisplayNotification());
+  base::DictionaryValue* sms = new base::DictionaryValue();
+  sms->SetString("number", "000-000-0000");
+  sms->SetString("text", "");
+  sms->SetString("timestamp", "Fri Jun  8 13:26:04 EDT 2016");
+
+  sms_observer->MessageReceived(*sms);
+  EXPECT_FALSE(GetDisplayNotification());
+}
+
+// Verify if no notification is received if the text is missing in sms message.
+TEST_F(SmsObserverTest, TextMessageMissingText) {

[tdanderson, 2017/01/26 22:19:06]

What's the difference between 'missing' and 'empty' text? The last two tests
seem to be doing the same thing

[yiyix, 2017/02/02 20:43:56]

In MessageReceived method in sms_observer, it checks for 2 different cases: - if
the key "text" is missing; and - if the value of "text" key is empty. 

+  SmsObserver* sms_observer = GetSmsObserver(GetPrimaryShelf()->shelf_widget());
+  EXPECT_FALSE(GetDisplayNotification());
+  base::DictionaryValue* sms = new base::DictionaryValue();
+  sms->SetString("number", "000-000-0000");
+  sms->SetString("timestamp", "Fri Jun  8 13:26:04 EDT 2016");
+
+  sms_observer->MessageReceived(*sms);
+  EXPECT_FALSE(GetDisplayNotification());
+}

[tdanderson, 2017/01/26 22:19:06]

Also consider adding a test which sends two notifications, each with the same
number, and verifying both have been received.

[yiyix, 2017/02/02 20:43:56]

That will be a good test. Added.

+
+}  // namespace test
+}  // namespace ash