Updates display manager and display preferences to handle touch calibration data.

Updates display manager and display preferences to handle touch calibration data.

 - Adds a parameter to display manager's RegisterDisplayProperty
   method to take touch calibration data as argument.
 - Adds methods to display manager that allows the setting
   and clearing of touch calibration data that is stored on the
   device.
 - Allows the storage of touch calibration associated data via
   DisplayPreferences.
 - '==' Operator overload for TouchCalibrationData struct to
   allow single line comparisons.
 - Updates required unit tests.

Design Doc=go/cros-touch-calibration

BUG=667921
COMPONENT=DisplayManager, DisplayPreferences, ChromeOS

Committed: https://crrev.com/4d3a49e6e0c54704c4655374aac0530adfed836d
Cr-Commit-Position: refs/heads/master@{#436463}

[malaykeshav, 2016-12-01 01:32:55]

Description was changed from

==========
Updates display manager and display preferences to handle touch calibration
data.

 - Adds a parameter to display manager's RegisterDisplayProperty
   method to take touch calibration data.
 - Adds methods to display manager that allows the setting
   and clearing of touch calibration data that is stored on the
   device.
 - Allows the storage of touch calibration associated data via
   DisplayPreferences.
 - Updates required unit tests.

Design Doc=go/cros-touch-calibration

BUG=667921
COMPONENT=DisplayManager, DisplayPreferences, ChromeOS
==========

to

==========
Updates display manager and display preferences to handle touch calibration
data.

 - Adds a parameter to display manager's RegisterDisplayProperty
   method to take touch calibration data as argument.
 - Adds methods to display manager that allows the setting
   and clearing of touch calibration data that is stored on the
   device.
 - Allows the storage of touch calibration associated data via
   DisplayPreferences.
 - Updates required unit tests.

Design Doc=go/cros-touch-calibration

BUG=667921
COMPONENT=DisplayManager, DisplayPreferences, ChromeOS
==========

[malaykeshav, 2016-12-01 01:35:45]

The CQ bit was checked by malaykeshav@chromium.org to run a CQ dry run

[malaykeshav, 2016-12-01 01:35:51]

The CQ bit was unchecked by malaykeshav@chromium.org

[malaykeshav, 2016-12-01 01:37:05]

Patchset #1 (id:1) has been deleted

[malaykeshav, 2016-12-01 01:37:16]

The CQ bit was checked by malaykeshav@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-01 01:37:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-01 02:25:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-01 02:25:23]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[malaykeshav, 2016-12-02 02:29:04]

Patchset #2 (id:40001) has been deleted

[malaykeshav, 2016-12-02 02:29:11]

Patchset #1 (id:20001) has been deleted

[malaykeshav, 2016-12-02 02:30:05]

Description was changed from

==========
Updates display manager and display preferences to handle touch calibration
data.

 - Adds a parameter to display manager's RegisterDisplayProperty
   method to take touch calibration data as argument.
 - Adds methods to display manager that allows the setting
   and clearing of touch calibration data that is stored on the
   device.
 - Allows the storage of touch calibration associated data via
   DisplayPreferences.
 - Updates required unit tests.

Design Doc=go/cros-touch-calibration

BUG=667921
COMPONENT=DisplayManager, DisplayPreferences, ChromeOS
==========

to

==========
Updates display manager and display preferences to handle touch calibration
data.

 - Adds a parameter to display manager's RegisterDisplayProperty
   method to take touch calibration data as argument.
 - Adds methods to display manager that allows the setting
   and clearing of touch calibration data that is stored on the
   device.
 - Allows the storage of touch calibration associated data via
   DisplayPreferences.
 - '==' Operator overload for TouchCalibrationData struct to
   allow single line comparisons.
 - Updates required unit tests.

Design Doc=go/cros-touch-calibration

BUG=667921
COMPONENT=DisplayManager, DisplayPreferences, ChromeOS
==========

[malaykeshav, 2016-12-02 02:40:38]

The CQ bit was checked by malaykeshav@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-02 02:41:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[malaykeshav, 2016-12-02 02:41:04]

malaykeshav@chromium.org changed reviewers:
+ oshima@chromium.org

[malaykeshav, 2016-12-02 02:41:04]

PTAL.

[malaykeshav, 2016-12-02 02:43:45]

malaykeshav@chromium.org changed reviewers:
+ stevenjb@chromium.org

[malaykeshav, 2016-12-02 02:43:45]

PTAL.
Updates display manager and display preferences to handle touch calibration
data.
Also updates TouchCalibrationData struct to have easier comparison by
overloading 
the '==' operator.

[commit-bot: I haz the power, 2016-12-02 03:22:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-02 03:22:39]

Dry run: This issue passed the CQ dry run.

[stevenjb, 2016-12-02 20:02:48]

https://codereview.chromium.org/2540383002/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/display/display_preferences.cc (right):

https://codereview.chromium.org/2540383002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/display/display_preferences.cc:78: // Retrieves touch
calibration associated data from the dictionary and stroes
stores

https://codereview.chromium.org/2540383002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/display/display_preferences.cc:83: x = y = 0;
nit: int x = 0, y = 0;

https://codereview.chromium.org/2540383002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/display/display_preferences.cc:96: return false;
nit: Instead of using |got_value|, these could all just be:
if (!value.GetInteger(...) ||
    !value.GetInteger(...)) {
  return false;
}

https://codereview.chromium.org/2540383002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/display/display_preferences.cc:122: const
display::TouchCalibrationData& touch_calibration_data) {
|value| (output) should follow input.

[malaykeshav, 2016-12-02 20:12:33]

Resolved comments.
PTAL

https://codereview.chromium.org/2540383002/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/display/display_preferences.cc (right):

https://codereview.chromium.org/2540383002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/display/display_preferences.cc:78: // Retrieves touch
calibration associated data from the dictionary and stroes
On 2016/12/02 at 20:02:48, stevenjb wrote:
> stores

Done

https://codereview.chromium.org/2540383002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/display/display_preferences.cc:83: x = y = 0;
On 2016/12/02 at 20:02:48, stevenjb wrote:
> nit: int x = 0, y = 0;

Done

https://codereview.chromium.org/2540383002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/display/display_preferences.cc:96: return false;
On 2016/12/02 at 20:02:48, stevenjb wrote:
> nit: Instead of using |got_value|, these could all just be:
> if (!value.GetInteger(...) ||
>     !value.GetInteger(...)) {
>   return false;
> }

Done

https://codereview.chromium.org/2540383002/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/display/display_preferences.cc:122: const
display::TouchCalibrationData& touch_calibration_data) {
On 2016/12/02 at 20:02:48, stevenjb wrote:
> |value| (output) should follow input.

Done

[malaykeshav, 2016-12-02 20:12:41]

The CQ bit was checked by malaykeshav@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-02 20:13:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[stevenjb, 2016-12-02 20:16:48]

lgtm w/ nit, but please make sure oshima@ reviews this also, I'm not super
familiar with this code.

https://codereview.chromium.org/2540383002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/display/display_preferences.cc (right):

https://codereview.chromium.org/2540383002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/display/display_preferences.cc:93:
value.GetInteger(kTouchCalibrationDataKeys[row * 4 + 1], &y))) {
nit: I find if (!a || !b) easier to read, especially with early exits, and I
think it's a more common pattern.

https://codereview.chromium.org/2540383002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/display/display_preferences.cc:111: return false;
Elim got_value here also (and remove the unnecessary local).

[malaykeshav, 2016-12-02 20:23:08]

Updated with missing nits.

@oshima Can you have a final look before I commit the patch.

[malaykeshav, 2016-12-02 22:12:15]

The CQ bit was checked by malaykeshav@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-02 22:13:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[oshima, 2016-12-02 22:33:41]

https://codereview.chromium.org/2540383002/diff/120001/chrome/browser/chromeo...
File chrome/browser/chromeos/display/display_preferences.cc (right):

https://codereview.chromium.org/2540383002/diff/120001/chrome/browser/chromeo...
chrome/browser/chromeos/display/display_preferences.cc:48: "display_4_x",
"display_4_y", "touch_4_x", "touch_4_y"   // Pair 4
alternatively, you can just save them as a string and decode it.
I think that's simpler. (we could have done that way for insets, but it's too
late now)

https://codereview.chromium.org/2540383002/diff/120001/ui/display/manager/dis...
File ui/display/manager/display_manager.cc (right):

https://codereview.chromium.org/2540383002/diff/120001/ui/display/manager/dis...
ui/display/manager/display_manager.cc:1003: }
can should be able to do somethign like

Display* display = DisplayManager::FindDisplayForId(display_id);
if (dispay) {
  display::ManagedDisplayinfo info = GetDisplayInfo();
  DCHECK(info must be valid)
} else {
   just updat the table.
}

https://codereview.chromium.org/2540383002/diff/120001/ui/display/manager/man...
File ui/display/manager/managed_display_info.h (right):

https://codereview.chromium.org/2540383002/diff/120001/ui/display/manager/man...
ui/display/manager/managed_display_info.h:44: for (size_t o_row=0; o_row <
other.point_pairs.size(); o_row++) {
shouldn't this always be paint_pairs.size() == other.point_pairs.size() == 4?

can you always short the data by 1st x point so that you can just compare using
== ?

[commit-bot: I haz the power, 2016-12-02 23:55:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-02 23:55:15]

Dry run: This issue passed the CQ dry run.

[malaykeshav, 2016-12-03 02:01:28]

Resolved comments.
PTAL

https://codereview.chromium.org/2540383002/diff/120001/chrome/browser/chromeo...
File chrome/browser/chromeos/display/display_preferences.cc (right):

https://codereview.chromium.org/2540383002/diff/120001/chrome/browser/chromeo...
chrome/browser/chromeos/display/display_preferences.cc:48: "display_4_x",
"display_4_y", "touch_4_x", "touch_4_y"   // Pair 4
On 2016/12/02 at 22:33:41, oshima wrote:
> alternatively, you can just save them as a string and decode it.
> I think that's simpler. (we could have done that way for insets, but it's too
late now)

Done

https://codereview.chromium.org/2540383002/diff/120001/ui/display/manager/dis...
File ui/display/manager/display_manager.cc (right):

https://codereview.chromium.org/2540383002/diff/120001/ui/display/manager/dis...
ui/display/manager/display_manager.cc:1003: }
On 2016/12/02 at 22:33:41, oshima wrote:
> can should be able to do somethign like
> 
> Display* display = DisplayManager::FindDisplayForId(display_id);
> if (dispay) {
>   display::ManagedDisplayinfo info = GetDisplayInfo();
>   DCHECK(info must be valid)
> } else {
>    just updat the table.
> }

We would still need to iterate through |active_display_list_| to create
display_info_list and call UpdateDisplaysWith().
Keeping this as.

https://codereview.chromium.org/2540383002/diff/120001/ui/display/manager/man...
File ui/display/manager/managed_display_info.h (right):

https://codereview.chromium.org/2540383002/diff/120001/ui/display/manager/man...
ui/display/manager/managed_display_info.h:44: for (size_t o_row=0; o_row <
other.point_pairs.size(); o_row++) {
On 2016/12/02 at 22:33:41, oshima wrote:
> shouldn't this always be paint_pairs.size() == other.point_pairs.size() == 4?
> 
> can you always short the data by 1st x point so that you can just compare
using == ?

Done

[malaykeshav, 2016-12-03 02:04:45]

Patchset #5 (id:140001) has been deleted

[malaykeshav, 2016-12-05 19:05:07]

ping

[oshima, 2016-12-05 20:43:21]

https://codereview.chromium.org/2540383002/diff/120001/ui/display/manager/dis...
File ui/display/manager/display_manager.cc (right):

https://codereview.chromium.org/2540383002/diff/120001/ui/display/manager/dis...
ui/display/manager/display_manager.cc:1003: }
On 2016/12/03 02:01:28, malaykeshav wrote:
> On 2016/12/02 at 22:33:41, oshima wrote:
> > can should be able to do somethign like
> > 
> > Display* display = DisplayManager::FindDisplayForId(display_id);
> > if (dispay) {
> >   display::ManagedDisplayinfo info = GetDisplayInfo();
> >   DCHECK(info must be valid)
> > } else {
> >    just updat the table.
> > }
> 
> We would still need to iterate through |active_display_list_| to create
> display_info_list and call UpdateDisplaysWith().
> Keeping this as.

ah right. please disregard this.

https://codereview.chromium.org/2540383002/diff/160001/ui/display/manager/man...
File ui/display/manager/managed_display_info.h (right):

https://codereview.chromium.org/2540383002/diff/160001/ui/display/manager/man...
ui/display/manager/managed_display_info.h:53: 
return quad_1 == quad_2;

https://codereview.chromium.org/2540383002/diff/160001/ui/display/manager/man...
ui/display/manager/managed_display_info.h:59: }
please move to .cc.

[malaykeshav, 2016-12-05 21:53:49]

The CQ bit was checked by malaykeshav@chromium.org to run a CQ dry run

[malaykeshav, 2016-12-05 21:53:58]

Resolved comments. PTAL

https://codereview.chromium.org/2540383002/diff/160001/ui/display/manager/man...
File ui/display/manager/managed_display_info.h (right):

https://codereview.chromium.org/2540383002/diff/160001/ui/display/manager/man...
ui/display/manager/managed_display_info.h:53: 
On 2016/12/05 at 20:43:21, oshima wrote:
> return quad_1 == quad_2;

Done

https://codereview.chromium.org/2540383002/diff/160001/ui/display/manager/man...
ui/display/manager/managed_display_info.h:53: 
On 2016/12/05 at 20:43:21, oshima wrote:
> return quad_1 == quad_2;

Done

https://codereview.chromium.org/2540383002/diff/160001/ui/display/manager/man...
ui/display/manager/managed_display_info.h:59: }
On 2016/12/05 at 20:43:21, oshima wrote:
> please move to .cc.

done

[commit-bot: I haz the power, 2016-12-05 21:54:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-05 22:53:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-05 22:53:27]

Dry run: This issue passed the CQ dry run.

[oshima, 2016-12-05 22:58:40]

lgtm

[malaykeshav, 2016-12-05 22:58:54]

The CQ bit was checked by malaykeshav@chromium.org

[malaykeshav, 2016-12-05 22:58:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org
Link to the patchset: https://codereview.chromium.org/2540383002/#ps180001
(title: "Updates display manager and display preferences to handle touch
calibration data.")

[commit-bot: I haz the power, 2016-12-05 22:59:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-05 23:05:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-05 23:05:43]

Failed to apply patch for chrome/browser/chromeos/display/display_preferences.h:
While running git apply --index -p1;
  error: patch failed: chrome/browser/chromeos/display/display_preferences.h:6
  error: chrome/browser/chromeos/display/display_preferences.h: patch does not
apply

Patch:       chrome/browser/chromeos/display/display_preferences.h
Index: chrome/browser/chromeos/display/display_preferences.h
diff --git a/chrome/browser/chromeos/display/display_preferences.h
b/chrome/browser/chromeos/display/display_preferences.h
index
7580677878276b7f8af530225f0045727707d1fd..d48d51e54bda15a84b59f79afbe1aea7479c53ce
100644
--- a/chrome/browser/chromeos/display/display_preferences.h
+++ b/chrome/browser/chromeos/display/display_preferences.h
@@ -6,12 +6,17 @@
 #define CHROME_BROWSER_CHROMEOS_DISPLAY_DISPLAY_PREFERENCES_H_
 
 #include <stdint.h>
+#include <array>
 
 #include "third_party/cros_system_api/dbus/service_constants.h"
 #include "ui/display/manager/display_layout.h"
 
 class PrefRegistrySimple;
 
+namespace gfx {
+class Point;
+}
+
 namespace chromeos {
 
 // Registers the prefs associated with display settings and stored
@@ -38,6 +43,13 @@ void StoreDisplayLayoutPrefForTest(const
display::DisplayIdList& list,
 // Stores the given |power_state| for tests.
 void StoreDisplayPowerStateForTest(DisplayPowerState power_state);
 
+// Parses the marshalled string data stored in local preferences for
calibration
+// points and populates |point_pair_quad| using the unmarshalled data.
+// See TouchCalibrationData in Managed display info.
+bool ParseTouchCalibrationStringForTest(
+    const std::string& str,
+    std::array<std::pair<gfx::Point, gfx::Point>, 4>* point_pair_quad);
+
 }  // namespace chromeos
 
 #endif  // CHROME_BROWSER_CHROMEOS_DISPLAY_DISPLAY_PREFERENCES_H_

[malaykeshav, 2016-12-05 23:19:22]

The CQ bit was checked by malaykeshav@chromium.org

[malaykeshav, 2016-12-05 23:19:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/2540383002/#ps200001
(title: "Merge with ToT")

[commit-bot: I haz the power, 2016-12-05 23:19:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-06 00:03:15]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1480979962674710,
"parent_rev": "c475031b5713a5a1e798a399fd6f7f06645299ad", "commit_rev":
"10859d4a6fc4e51b0062ccf4fb6d7aeb181bb02a"}

[commit-bot: I haz the power, 2016-12-06 00:03:40]

Committed patchset #7 (id:200001)

[commit-bot: I haz the power, 2016-12-06 00:05:44]

Description was changed from

==========
Updates display manager and display preferences to handle touch calibration
data.

 - Adds a parameter to display manager's RegisterDisplayProperty
   method to take touch calibration data as argument.
 - Adds methods to display manager that allows the setting
   and clearing of touch calibration data that is stored on the
   device.
 - Allows the storage of touch calibration associated data via
   DisplayPreferences.
 - '==' Operator overload for TouchCalibrationData struct to
   allow single line comparisons.
 - Updates required unit tests.

Design Doc=go/cros-touch-calibration

BUG=667921
COMPONENT=DisplayManager, DisplayPreferences, ChromeOS
==========

to

==========
Updates display manager and display preferences to handle touch calibration
data.

 - Adds a parameter to display manager's RegisterDisplayProperty
   method to take touch calibration data as argument.
 - Adds methods to display manager that allows the setting
   and clearing of touch calibration data that is stored on the
   device.
 - Allows the storage of touch calibration associated data via
   DisplayPreferences.
 - '==' Operator overload for TouchCalibrationData struct to
   allow single line comparisons.
 - Updates required unit tests.

Design Doc=go/cros-touch-calibration

BUG=667921
COMPONENT=DisplayManager, DisplayPreferences, ChromeOS

Committed: https://crrev.com/4d3a49e6e0c54704c4655374aac0530adfed836d
Cr-Commit-Position: refs/heads/master@{#436463}
==========

[commit-bot: I haz the power, 2016-12-06 00:05:45]

Patchset 7 (id:??) landed as
https://crrev.com/4d3a49e6e0c54704c4655374aac0530adfed836d
Cr-Commit-Position: refs/heads/master@{#436463}