Check for CGRectZero instead of CGRectNull.

Some definitions:
- null rectangle: CGRectNull = { {inf, inf}, {0, 0} }
- empty rectangle: any rectangle that has a 0 area, ie has a width or a height of 0. That means that CGRectZero = { {0, 0}, {0, 0} }) or CGRectNull are examples of empty rectangles.

Keyboard-change notifications include a begin and an end frame, describing where the keyboard was and where it is going.
When the software keyboard is undocked, some frames can be CGRectZero.

In order to handle this case, the code prior to this CL was checking incorrectly for CGRectNull, when it should have checked CGRectZero. This resulted in the keyboard being incorrectly reported as in hardware mode.

With this CL, the undocking case is properly handled, and the frame checked to be CGRectZero.

BUG=none
R=sdefresne@chromium.org

Review URL: https://codereview.chromium.org/1256553002

Cr-Commit-Position: refs/heads/master@{#340229}

3 files changed, 125 insertions, 4 deletions

diff --git a/ios/chrome/browser/ui/keyboard/hardware_keyboard_watcher.mm b/ios/chrome/browser/ui/keyboard/hardware_keyboard_watcher.mm
index 56ee339..c390c22 100644
--- a/ios/chrome/browser/ui/keyboard/hardware_keyboard_watcher.mm
+++ b/ios/chrome/browser/ui/keyboard/hardware_keyboard_watcher.mm
@@ -76,16 +76,18 @@ bool IntersectsButDoesNotInclude(CGRect firstRect, CGRect secondRect) {
       [userInfo[UIKeyboardFrameEndUserInfoKey] CGRectValue];
   CGRect screenBounds = [UIScreen mainScreen].bounds;
 
-  // Null frames are seen when moving a split dock. Split keyboard means
+  // CGRectZero frames are seen when moving a split dock. Split keyboard means
   // software keyboard.
-  bool hasNullFrames =
-      CGRectIsNull(beginKeyboardFrame) || CGRectIsNull(endKeyboardFrame);
+  bool hasCGRectZeroFrames =
+      CGRectEqualToRect(CGRectZero, beginKeyboardFrame) ||
+      CGRectEqualToRect(CGRectZero, endKeyboardFrame);
 
   bool keyboardIsPartiallyOnScreen =
       IntersectsButDoesNotInclude(screenBounds, beginKeyboardFrame) ||
       IntersectsButDoesNotInclude(screenBounds, endKeyboardFrame);
 
-  bool isInHarwareKeyboardMode = !hasNullFrames && keyboardIsPartiallyOnScreen;
+  bool isInHarwareKeyboardMode =
+      !hasCGRectZeroFrames && keyboardIsPartiallyOnScreen;
 
   UMA_HISTOGRAM_BOOLEAN("Omnibox.HardwareKeyboardModeEnabled",
                         isInHarwareKeyboardMode);
diff --git a/ios/chrome/browser/ui/keyboard/hardware_keyboard_watcher_unittest.mm b/ios/chrome/browser/ui/keyboard/hardware_keyboard_watcher_unittest.mm
new file mode 100644
index 0000000..6200e15
--- /dev/null
+++ b/ios/chrome/browser/ui/keyboard/hardware_keyboard_watcher_unittest.mm
@@ -0,0 +1,118 @@
+// Copyright 2015 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.
+
+#import "ios/chrome/browser/ui/keyboard/hardware_keyboard_watcher.h"
+
+#include "base/mac/scoped_nsobject.h"
+#include "base/test/histogram_tester.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"
+#import "third_party/ocmock/OCMock/OCMock.h"
+
+namespace {
+
+void PostKeyboardWillChangeNotification(CGRect beginFrame, CGRect endFrame) {
+  [[NSNotificationCenter defaultCenter]
+      postNotificationName:UIKeyboardWillChangeFrameNotification
+                    object:nil
+                  userInfo:@{
+                    UIKeyboardFrameBeginUserInfoKey :
+                        [NSValue valueWithCGRect:beginFrame],
+                    UIKeyboardFrameEndUserInfoKey :
+                        [NSValue valueWithCGRect:endFrame],
+                  }];
+}
+
+typedef PlatformTest HardwareKeyboardWatcherTest;
+
+TEST_F(HardwareKeyboardWatcherTest, AccessoryViewNotInHierarchy_NoHistogram) {
+  base::HistogramTester histogram_tester;
+  id mockView = [OCMockObject niceMockForClass:[UIView class]];
+  [[mockView stub] andReturn:nil];
+  base::scoped_nsobject<HardwareKeyboardWatcher> watcher(
+      [[HardwareKeyboardWatcher alloc] initWithAccessoryView:mockView]);
+
+  PostKeyboardWillChangeNotification(CGRectZero, CGRectZero);
+  PostKeyboardWillChangeNotification(CGRectInfinite, CGRectInfinite);
+
+  histogram_tester.ExpectTotalCount("Omnibox.HardwareKeyboardModeEnabled", 0);
+}
+
+TEST_F(HardwareKeyboardWatcherTest, EmptyBeginFrame_SoftwareKeyboardHistogram) {
+  base::HistogramTester histogram_tester;
+  id mockView = [OCMockObject niceMockForClass:[UIView class]];
+  [[[mockView stub] andReturn:[[UIApplication sharedApplication] keyWindow]]
+      window];
+  base::scoped_nsobject<HardwareKeyboardWatcher> watcher(
+      [[HardwareKeyboardWatcher alloc] initWithAccessoryView:mockView]);
+
+  PostKeyboardWillChangeNotification(CGRectZero, CGRectInfinite);
+
+  histogram_tester.ExpectUniqueSample("Omnibox.HardwareKeyboardModeEnabled",
+                                      false, 1);
+}
+
+TEST_F(HardwareKeyboardWatcherTest, EmptyEndFrame_SoftwareKeyboardHistogram) {
+  base::HistogramTester histogram_tester;
+  id mockView = [OCMockObject niceMockForClass:[UIView class]];
+  [[[mockView stub] andReturn:[[UIApplication sharedApplication] keyWindow]]
+      window];
+  base::scoped_nsobject<HardwareKeyboardWatcher> watcher(
+      [[HardwareKeyboardWatcher alloc] initWithAccessoryView:mockView]);
+
+  PostKeyboardWillChangeNotification(CGRectInfinite, CGRectZero);
+
+  histogram_tester.ExpectUniqueSample("Omnibox.HardwareKeyboardModeEnabled",
+                                      false, 1);
+}
+
+TEST_F(HardwareKeyboardWatcherTest,
+       KeyboardFullyOnScreen_SoftwareKeyboardHistogram) {
+  base::HistogramTester histogram_tester;
+  id mockView = [OCMockObject niceMockForClass:[UIView class]];
+  [[[mockView stub] andReturn:[[UIApplication sharedApplication] keyWindow]]
+      window];
+  base::scoped_nsobject<HardwareKeyboardWatcher> watcher(
+      [[HardwareKeyboardWatcher alloc] initWithAccessoryView:mockView]);
+
+  PostKeyboardWillChangeNotification(CGRectMake(0, 0, 100, 100),
+                                     CGRectMake(0, 100, 100, 100));
+
+  histogram_tester.ExpectUniqueSample("Omnibox.HardwareKeyboardModeEnabled",
+                                      false, 1);
+}
+
+TEST_F(HardwareKeyboardWatcherTest,
+       KeyboardFullyOffScreen_SoftwareKeyboardHistogram) {
+  base::HistogramTester histogram_tester;
+  id mockView = [OCMockObject niceMockForClass:[UIView class]];
+  [[[mockView stub] andReturn:[[UIApplication sharedApplication] keyWindow]]
+      window];
+  base::scoped_nsobject<HardwareKeyboardWatcher> watcher(
+      [[HardwareKeyboardWatcher alloc] initWithAccessoryView:mockView]);
+
+  PostKeyboardWillChangeNotification(CGRectMake(0, -100, 100, 100),
+                                     CGRectMake(0, 0, 100, 100));
+
+  histogram_tester.ExpectUniqueSample("Omnibox.HardwareKeyboardModeEnabled",
+                                      false, 1);
+}
+
+TEST_F(HardwareKeyboardWatcherTest,
+       KeyboardPartiallyOnScreen_SoftwareKeyboardHistogram) {
+  base::HistogramTester histogram_tester;
+  id mockView = [OCMockObject niceMockForClass:[UIView class]];
+  [[[mockView stub] andReturn:[[UIApplication sharedApplication] keyWindow]]
+      window];
+  base::scoped_nsobject<HardwareKeyboardWatcher> watcher(
+      [[HardwareKeyboardWatcher alloc] initWithAccessoryView:mockView]);
+
+  PostKeyboardWillChangeNotification(CGRectMake(0, -50, 100, 100),
+                                     CGRectMake(0, 0, 100, 100));
+
+  histogram_tester.ExpectUniqueSample("Omnibox.HardwareKeyboardModeEnabled",
+                                      true, 1);
+}
+
+}  // namespace
diff --git a/ios/chrome/ios_chrome_tests.gyp b/ios/chrome/ios_chrome_tests.gyp
index 5aa6bd6..04fd04c 100644
--- a/ios/chrome/ios_chrome_tests.gyp
+++ b/ios/chrome/ios_chrome_tests.gyp
@@ -54,6 +54,7 @@
         'browser/snapshots/snapshots_util_unittest.mm',
         'browser/translate/translate_service_ios_unittest.cc',
         'browser/ui/commands/set_up_for_testing_command_unittest.mm',
+        'browser/ui/keyboard/hardware_keyboard_watcher_unittest.mm',
         'browser/ui/native_content_controller_unittest.mm',
         'browser/ui/ui_util_unittest.mm',
         'browser/ui/uikit_ui_util_unittest.mm',