Add unit tests for screen capture and share notification UI.

ash/system/chromeos/screen_security/screen_tray_item_unittest.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 "ash/system/chromeos/screen_security/screen_capture_tray_item.h"
+#include "ash/system/chromeos/screen_security/screen_share_tray_item.h"
+#include "ash/system/chromeos/screen_security/screen_tray_item.h"
+

[James Cook, 2013/06/07 14:16:37]

Either put the primary test class header include on it's own line at the top:

#include "a/s/c/s/screen_tray_item.h"

#include "foo.h"
#include "bar.h"

Or just list all the headers alphabetically in one big block.  (I prefer the
former style myself.)

[Tim Song, 2013/06/07 18:14:35]

Done.

+#include "ash/test/ash_test_base.h"
+#include "base/callback.h"
+#include "base/strings/utf_string_conversions.h"
+#include "ui/base/events/event.h"
+#include "ui/gfx/point.h"
+#include "ui/views/view.h"
+
+namespace ash {
+namespace internal {
+
+const char kTestScreenCaptureAppName[] = "Ash Test";
+const char kTestScreenShareHelperName[] = "Ash Test";

[James Cook, 2013/06/07 14:16:37]

nit: Generally we use different strings, integers, etc. in test data to make it
easier to tell failures apart, for example in the debugger.

[Tim Song, 2013/06/07 18:14:35]

Done. I just realized that it would be a good idea to use some unicode strings
here to potentially test for localization issues.

+
+SystemTray* GetSystemTray() {
+  return Shell::GetInstance()->GetPrimarySystemTray();
+}
+
+SystemTrayNotifier* GetSystemTrayNotifier() {
+  return Shell::GetInstance()->system_tray_notifier();
+}
+
+void ClickViewCenter(views::View* view) {
+  gfx::Point click_location = gfx::Point(view->width()/2, view->height()/2);

[James Cook, 2013/06/07 14:16:37]

Spaces around /

Also, click_location should be click_location_in_local (a suffix to indicate the
coordinate system of the point, as opposed to _in_screen or _in_window)

[Tim Song, 2013/06/07 18:14:35]

Done.

+  view->OnMousePressed(ui::MouseEvent(ui::ET_MOUSE_PRESSED,
+                                      click_location,
+                                      click_location,
+                                      ui::EF_NONE));
+}
+
+class ScreenTrayItemTest : public ash::test::AshTestBase {
+ public:
+  explicit ScreenTrayItemTest()

[James Cook, 2013/06/07 14:16:37]

no explicit

[Tim Song, 2013/06/07 18:14:35]

Done. I should really keep an eye out when copy pasting code...

+      : tray_item_(NULL), stop_callback_hit_count_(0) {}
+  virtual ~ScreenTrayItemTest() {}
+
+  ScreenTrayItem* tray_item() { return tray_item_; }
+  uint stop_callback_hit_count() { return stop_callback_hit_count_; }

[James Cook, 2013/06/07 14:16:37]

int would be fine here.  Also, const here (but not above)

[Tim Song, 2013/06/07 18:14:35]

Done.

+
+  void StartSession() {
+    tray_item_->Start(
+        base::Bind(&ScreenTrayItemTest::StopCallback, base::Unretained(this)));
+  }
+
+  void StopSession() {
+    tray_item_->Stop();
+  }
+
+  void StopCallback() {
+    stop_callback_hit_count_++;
+  }
+
+ protected:
+  ScreenTrayItem* tray_item_;

[James Cook, 2013/06/07 14:16:37]

Make this private and use a setter.  We generally don't use protected member
variables.  This makes it a little easier to find access to the data when we
have to change the code later.

Also, comment on memory ownership (I believe it's owned by the parent view in
this case).

[Tim Song, 2013/06/07 18:14:35]

Done. I added the comments about memory management in the subclasses, as we
don't actually initialize tray_item_ here.

+  uint stop_callback_hit_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScreenTrayItemTest);
+};
+
+class ScreenCaptureTest : public ScreenTrayItemTest {
+ public:
+  explicit ScreenCaptureTest() {}
+  virtual ~ScreenCaptureTest() {}
+
+  virtual void SetUp() OVERRIDE {
+    test::AshTestBase::SetUp();
+    tray_item_ = new ScreenCaptureTrayItem(GetSystemTray());
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(ScreenCaptureTest);
+};
+
+class ScreenShareTest : public ScreenTrayItemTest {
+ public:
+  explicit ScreenShareTest() {}
+  virtual ~ScreenShareTest() {}
+
+  virtual void SetUp() OVERRIDE {
+    test::AshTestBase::SetUp();
+    tray_item_ = new ScreenShareTrayItem(GetSystemTray());
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(ScreenShareTest);
+};
+
+

[James Cook, 2013/06/07 14:16:37]

nit: one blank line

[Tim Song, 2013/06/07 18:14:35]

Done.

+void TestStartAndStop(ScreenTrayItemTest* test) {
+  ScreenTrayItem* tray_item = test->tray_item();
+
+  EXPECT_FALSE(tray_item->is_started());
+  EXPECT_EQ(test->stop_callback_hit_count(), 0u);

[James Cook, 2013/06/07 14:16:37]

expected value goes first:

EXPECT_EQ(0, stop_callback_hit_count())

[Tim Song, 2013/06/07 18:14:35]

Done.

+
+  test->StartSession();
+  EXPECT_TRUE(tray_item->is_started());
+
+  test->StopSession();
+  EXPECT_FALSE(tray_item->is_started());
+  EXPECT_EQ(test->stop_callback_hit_count(), 1u);
+}
+
+TEST_F(ScreenCaptureTest, StartAndStop) { TestStartAndStop(this); }
+TEST_F(ScreenShareTest, StartAndStop) { TestStartAndStop(this); }
+
+void TestNotificationStartAndStop(ScreenTrayItemTest* test,

[James Cook, 2013/06/07 14:16:37]

Good: nice descriptive function name

[Tim Song, 2013/06/07 18:14:35]

Thanks!

+                                  const base::Closure& start_func,

[James Cook, 2013/06/07 14:16:37]

no abbrevations: start_function, stop_function

[Tim Song, 2013/06/07 18:14:35]

Done.

+                                  const base::Closure& stop_func) {
+  ScreenTrayItem* tray_item = test->tray_item();
+  EXPECT_FALSE(tray_item->is_started());
+
+  start_func.Run();
+  EXPECT_TRUE(tray_item->is_started());
+
+  // The stop callback shouldn't be called because we stopped
+  // through the notification system.
+  stop_func.Run();
+  EXPECT_FALSE(tray_item->is_started());
+  EXPECT_EQ(test->stop_callback_hit_count(), 0u);
+}
+
+TEST_F(ScreenCaptureTest, NotificationStartAndStop) {
+  base::Closure start_func =
+      base::Bind(&SystemTrayNotifier::NotifyScreenCaptureStart,
+          base::Unretained(GetSystemTrayNotifier()),
+          base::Bind(&ScreenTrayItemTest::StopCallback,
+                     base::Unretained(this)),
+                     base::UTF8ToUTF16(kTestScreenCaptureAppName));
+
+  base::Closure stop_func =
+      base::Bind(&SystemTrayNotifier::NotifyScreenCaptureStop,
+          base::Unretained(GetSystemTrayNotifier()));
+
+  TestNotificationStartAndStop(this, start_func, stop_func);
+}
+
+TEST_F(ScreenShareTest, NotificationStartAndStop) {
+  base::Closure start_func =
+      base::Bind(&SystemTrayNotifier::NotifyScreenShareStart,
+          base::Unretained(GetSystemTrayNotifier()),
+          base::Bind(&ScreenTrayItemTest::StopCallback,
+                     base::Unretained(this)),
+                     base::UTF8ToUTF16(kTestScreenShareHelperName));
+
+  base::Closure stop_func =
+      base::Bind(&SystemTrayNotifier::NotifyScreenShareStop,
+          base::Unretained(GetSystemTrayNotifier()));
+
+  TestNotificationStartAndStop(this, start_func, stop_func);
+}
+
+
+void TestNotificationView(ScreenTrayItemTest* test) {
+  ScreenTrayItem* tray_item = test->tray_item();
+
+  test->StartSession();
+  EXPECT_TRUE(tray_item->notification_view()->visible());
+
+  // Clicking on the notification view should dismiss the view
+  ClickViewCenter(tray_item->notification_view());
+  EXPECT_FALSE(tray_item->notification_view());
+
+  test->StopSession();
+}
+
+TEST_F(ScreenCaptureTest, NotificationView) { TestNotificationView(this); }
+TEST_F(ScreenShareTest, NotificationView) { TestNotificationView(this); }
+
+} // namespace internal
+} // namespace ash