Don't return unsupported scales frrom GetScaleFactorFromScale()

ui/base/layout_unittest.cc

+// Copyright (c) 2012 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 "testing/gtest/include/gtest/gtest.h"
+
+#include "ui/base/layout.h"
+
+namespace ui {
+
+TEST(LayoutTest, GetScaleFactorScale) {
+  EXPECT_FLOAT_EQ(1, GetScaleFactorScale(SCALE_FACTOR_100P));
+  EXPECT_FLOAT_EQ(1.4, GetScaleFactorScale(SCALE_FACTOR_140P));
+  EXPECT_FLOAT_EQ(1.8, GetScaleFactorScale(SCALE_FACTOR_180P));
+  EXPECT_FLOAT_EQ(2, GetScaleFactorScale(SCALE_FACTOR_200P));
+}
+
+TEST(LayoutTest, GetScaleFactorFromScale) {
+  std::vector<ScaleFactor> original_supported_factors =
+      GetSupportedScaleFactors();
+
+  std::vector<ScaleFactor> supported_factors;
+  test::SetSupportedScaleFactors(supported_factors);
+  for (float scale = 0.1; scale < 3; scale += 0.1)

[pkotwicz, 2012/09/19 21:05:17]

Nit: set your bounds between 1 and 2.

[Xianzhu, 2012/09/19 21:38:23]

We do have scales smaller than 1 or bigger than 2 on Android.

+    EXPECT_EQ(SCALE_FACTOR_100P, GetScaleFactorFromScale(scale));
+
+  supported_factors.push_back(SCALE_FACTOR_100P);
+  supported_factors.push_back(SCALE_FACTOR_180P);
+  test::SetSupportedScaleFactors(supported_factors);
+  EXPECT_EQ(SCALE_FACTOR_100P, GetScaleFactorFromScale(0.1));
+  EXPECT_EQ(SCALE_FACTOR_100P, GetScaleFactorFromScale(0.9));
+  EXPECT_EQ(SCALE_FACTOR_100P, GetScaleFactorFromScale(1));
+  EXPECT_EQ(SCALE_FACTOR_100P, GetScaleFactorFromScale(1.39));
+  EXPECT_EQ(SCALE_FACTOR_180P, GetScaleFactorFromScale(1.41));
+  EXPECT_EQ(SCALE_FACTOR_180P, GetScaleFactorFromScale(1.8));
+  EXPECT_EQ(SCALE_FACTOR_180P, GetScaleFactorFromScale(2));
+  EXPECT_EQ(SCALE_FACTOR_180P, GetScaleFactorFromScale(999));
+
+  supported_factors.push_back(SCALE_FACTOR_140P);
+  supported_factors.push_back(SCALE_FACTOR_200P);

[pkotwicz, 2012/09/19 21:05:17]

Nit: Add either SCALE_FACTOR_140P or SCALE_FACTOR_200P.
I do not think that your test is much stronger if you add both. But its ok if
you leave it as is. (And I don't mind being proven wrong)

[Xianzhu, 2012/09/19 21:38:23]

Actually this is testing the behavior of the original GetScaleFactorFromScale(),
that is, all of the factors are supported. The case that part of the factors are
supported has been tested above.

Separated the test case into 3 trying to make it clearer.

+  test::SetSupportedScaleFactors(supported_factors);
+  EXPECT_EQ(SCALE_FACTOR_100P, GetScaleFactorFromScale(0.1));
+  EXPECT_EQ(SCALE_FACTOR_100P, GetScaleFactorFromScale(0.9));
+  EXPECT_EQ(SCALE_FACTOR_100P, GetScaleFactorFromScale(1));
+  EXPECT_EQ(SCALE_FACTOR_100P, GetScaleFactorFromScale(1.19));
+  EXPECT_EQ(SCALE_FACTOR_140P, GetScaleFactorFromScale(1.21));
+  EXPECT_EQ(SCALE_FACTOR_140P, GetScaleFactorFromScale(1.3));
+  EXPECT_EQ(SCALE_FACTOR_140P, GetScaleFactorFromScale(1.4));
+  EXPECT_EQ(SCALE_FACTOR_140P, GetScaleFactorFromScale(1.59));
+  EXPECT_EQ(SCALE_FACTOR_180P, GetScaleFactorFromScale(1.61));
+  EXPECT_EQ(SCALE_FACTOR_180P, GetScaleFactorFromScale(1.7));
+  EXPECT_EQ(SCALE_FACTOR_180P, GetScaleFactorFromScale(1.89));
+  EXPECT_EQ(SCALE_FACTOR_200P, GetScaleFactorFromScale(1.91));
+  EXPECT_EQ(SCALE_FACTOR_200P, GetScaleFactorFromScale(2));
+  EXPECT_EQ(SCALE_FACTOR_200P, GetScaleFactorFromScale(2.1));
+  EXPECT_EQ(SCALE_FACTOR_200P, GetScaleFactorFromScale(999));
+
+  test::SetSupportedScaleFactors(original_supported_factors);
+}
+
+}  // namespace ui