Plumbs touch calibration API to the display manager

extensions/browser/api/system_display/system_display_api.cc

Index: extensions/browser/api/system_display/system_display_api.cc
diff --git a/extensions/browser/api/system_display/system_display_api.cc b/extensions/browser/api/system_display/system_display_api.cc
index 41ed192169f49346d0a5dc96f2fa2fac96b49499..b810754d7119d4f773013a3bae34c0915f317adc 100644
--- a/extensions/browser/api/system_display/system_display_api.cc
+++ b/extensions/browser/api/system_display/system_display_api.cc
@@ -270,10 +270,6 @@ ExtensionFunction::ResponseAction
 SystemDisplayTouchCalibrationStartFunction::Run() {
   std::unique_ptr<display::TouchCalibrationStart::Params> params(
       display::TouchCalibrationStart::Params::Create(*args_));
-  if (!DisplayInfoProvider::Get()->IsTouchCalibrationActive(params->id)) {
-    return RespondNow(Error(
-        "Another touch calibration is already active."));
-  }

[stevenjb, 2016/12/27 19:37:27]

This seems like a useful error? (Although the logic appears to be backwards).

[malaykeshav, 2016/12/27 20:07:01]

This check has been moved to ValidateParamsForTouchCalibration() in
https://codereview.chromium.org/2603563002/diff/20001/chrome/browser/extensio...

Will add it to TouchCalibrationStart() when this API is plumbed through after
the UX is complete.

[stevenjb, 2016/12/27 21:58:12]

Looking at that code, that seems like a strange place to do this test. The error
really should be detected here. I am fine with moving the test to
DisplayInfoProvider::TouchCalibrationStart and passing &error to that like in
the other methods, but just moving that test, even temporarily, seems odd.

[malaykeshav, 2016/12/28 12:34:28]

Done

   if (!DisplayInfoProvider::Get()->TouchCalibrationStart(params->id))
     return RespondNow(Error("Invalid display ID: " + params->id));
   return RespondNow(NoArguments());
@@ -283,14 +279,10 @@ ExtensionFunction::ResponseAction
 SystemDisplayTouchCalibrationSetFunction::Run() {
   std::unique_ptr<display::TouchCalibrationSet::Params> params(
       display::TouchCalibrationSet::Params::Create(*args_));
-  if (!DisplayInfoProvider::Get()->IsTouchCalibrationActive(params->id)) {
-    return RespondNow(Error(
-        "Another touch calibration is already active."));
-  }
-  if (!DisplayInfoProvider::Get()->TouchCalibrationSet(params->id,
-                                                       params->pairs,
-                                                       params->bounds)) {
-    return RespondNow(Error("Invalid display ID: " + params->id));
+  std::string error;
+  if (!DisplayInfoProvider::Get()->TouchCalibrationSet(
+          params->id, params->pairs, params->bounds, &error)) {
+    return RespondNow(Error(error));
   }
   return RespondNow(NoArguments());
 }
@@ -299,12 +291,9 @@ ExtensionFunction::ResponseAction
 SystemDisplayTouchCalibrationResetFunction::Run() {
   std::unique_ptr<display::TouchCalibrationReset::Params> params(
       display::TouchCalibrationReset::Params::Create(*args_));
-  if (!DisplayInfoProvider::Get()->IsTouchCalibrationActive(params->id)) {
-    return RespondNow(Error(
-        "Another touch calibration is already active."));
-  }
-  if (!DisplayInfoProvider::Get()->TouchCalibrationReset(params->id))
-    return RespondNow(Error("Invalid display ID: " + params->id));
+  std::string error;
+  if (!DisplayInfoProvider::Get()->TouchCalibrationReset(params->id, &error))
+    return RespondNow(Error(error));
   return RespondNow(NoArguments());
 }