From 62d4c69ec6bfaff365d9b240c1519802ddd245ab Mon Sep 17 00:00:00 2001 From: thestig Date: Fri, 7 Aug 2015 19:27:05 -0700 Subject: [PATCH] Default to 1 when --force-device-scale-factor has an invalid value. Currently, the scaling factor may end up as 0, causing a divide by 0. Also add warnings to base::StringToFoo(). BUG=517944 Review URL: https://codereview.chromium.org/1278173006 Cr-Commit-Position: refs/heads/master@{#342505} --- base/strings/string_number_conversions.h | 6 +++++- ui/gfx/display.cc | 4 +++- ui/gfx/display_unittest.cc | 36 ++++++++++++++++++++++++-------- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/base/strings/string_number_conversions.h b/base/strings/string_number_conversions.h index 050e627a1a13..cf1c3b467ddb 100644 --- a/base/strings/string_number_conversions.h +++ b/base/strings/string_number_conversions.h @@ -64,6 +64,8 @@ BASE_EXPORT std::string DoubleToString(double value); // - No characters parseable as a number at the beginning of the string. // |*output| will be set to 0. // - Empty string. |*output| will be set to 0. +// WARNING: Will write to |output| even when returning false. +// Read the comments above carefully. BASE_EXPORT bool StringToInt(const StringPiece& input, int* output); BASE_EXPORT bool StringToInt(const StringPiece16& input, int* output); @@ -81,10 +83,12 @@ BASE_EXPORT bool StringToSizeT(const StringPiece16& input, size_t* output); // For floating-point conversions, only conversions of input strings in decimal // form are defined to work. Behavior with strings representing floating-point -// numbers in hexadecimal, and strings representing non-fininte values (such as +// numbers in hexadecimal, and strings representing non-finite values (such as // NaN and inf) is undefined. Otherwise, these behave the same as the integral // variants. This expects the input string to NOT be specific to the locale. // If your input is locale specific, use ICU to read the number. +// WARNING: Will write to |output| even when returning false. +// Read the comments here and above StringToInt() carefully. BASE_EXPORT bool StringToDouble(const std::string& input, double* output); // Hex encoding ---------------------------------------------------------------- diff --git a/ui/gfx/display.cc b/ui/gfx/display.cc index 03949cb315c7..7b77ae40eac7 100644 --- a/ui/gfx/display.cc +++ b/ui/gfx/display.cc @@ -40,8 +40,10 @@ float GetForcedDeviceScaleFactorImpl() { std::string value = base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII( switches::kForceDeviceScaleFactor); - if (!base::StringToDouble(value, &scale_in_double)) + if (!base::StringToDouble(value, &scale_in_double)) { LOG(ERROR) << "Failed to parse the default device scale factor:" << value; + scale_in_double = 1.0; + } } return static_cast(scale_in_double); } diff --git a/ui/gfx/display_unittest.cc b/ui/gfx/display_unittest.cc index 7decdb68f474..f87b93e9546d 100644 --- a/ui/gfx/display_unittest.cc +++ b/ui/gfx/display_unittest.cc @@ -4,46 +4,64 @@ #include "ui/gfx/display.h" +#include "base/command_line.h" #include "testing/gtest/include/gtest/gtest.h" #include "ui/gfx/geometry/insets.h" +#include "ui/gfx/switches.h" + +namespace gfx { namespace { TEST(DisplayTest, WorkArea) { - gfx::Display display(0, gfx::Rect(0, 0, 100, 100)); + Display display(0, Rect(0, 0, 100, 100)); EXPECT_EQ("0,0 100x100", display.bounds().ToString()); EXPECT_EQ("0,0 100x100", display.work_area().ToString()); - display.set_work_area(gfx::Rect(3, 4, 90, 80)); + display.set_work_area(Rect(3, 4, 90, 80)); EXPECT_EQ("0,0 100x100", display.bounds().ToString()); EXPECT_EQ("3,4 90x80", display.work_area().ToString()); - display.SetScaleAndBounds(1.0f, gfx::Rect(10, 20, 50, 50)); + display.SetScaleAndBounds(1.0f, Rect(10, 20, 50, 50)); EXPECT_EQ("10,20 50x50", display.bounds().ToString()); EXPECT_EQ("13,24 40x30", display.work_area().ToString()); - display.SetSize(gfx::Size(200, 200)); + display.SetSize(Size(200, 200)); EXPECT_EQ("13,24 190x180", display.work_area().ToString()); - display.UpdateWorkAreaFromInsets(gfx::Insets(3, 4, 5, 6)); + display.UpdateWorkAreaFromInsets(Insets(3, 4, 5, 6)); EXPECT_EQ("14,23 190x192", display.work_area().ToString()); } TEST(DisplayTest, Scale) { - gfx::Display display(0, gfx::Rect(0, 0, 100, 100)); - display.set_work_area(gfx::Rect(10, 10, 80, 80)); + Display display(0, Rect(0, 0, 100, 100)); + display.set_work_area(Rect(10, 10, 80, 80)); EXPECT_EQ("0,0 100x100", display.bounds().ToString()); EXPECT_EQ("10,10 80x80", display.work_area().ToString()); // Scale it back to 2x - display.SetScaleAndBounds(2.0f, gfx::Rect(0, 0, 140, 140)); + display.SetScaleAndBounds(2.0f, Rect(0, 0, 140, 140)); EXPECT_EQ("0,0 70x70", display.bounds().ToString()); EXPECT_EQ("10,10 50x50", display.work_area().ToString()); // Scale it back to 1x - display.SetScaleAndBounds(1.0f, gfx::Rect(0, 0, 100, 100)); + display.SetScaleAndBounds(1.0f, Rect(0, 0, 100, 100)); EXPECT_EQ("0,0 100x100", display.bounds().ToString()); EXPECT_EQ("10,10 80x80", display.work_area().ToString()); } +// https://crbug.com/517944 +TEST(DisplayTest, ForcedDeviceScaleFactorByCommandLine) { + Display::ResetForceDeviceScaleFactorForTesting(); + + // Look ma, no value! + base::CommandLine::ForCurrentProcess()->AppendSwitch( + switches::kForceDeviceScaleFactor); + + EXPECT_EQ(1, Display::GetForcedDeviceScaleFactor()); + Display::ResetForceDeviceScaleFactorForTesting(); } + +} // namespace + +} // namespace gfx -- 2.11.4.GIT