Reading, writing of CDD and CJT JSON formats.

Reading, writing of CDD and CJT JSON formats.
Printer description and print job ticket formats.

BUG=317027
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=248644

[Vitaly Buka (NO REVIEWS), 2014-02-03 19:09:24]

ping

[Noam Samuel, 2014-02-03 20:30:33]

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
File components/cloud_devices/description_items.h (right):

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:23: // Represents CDD capability
that stored JSON list.
Nit: Rephrase as

"Represents a CDD capability that is stored as a JSON list"

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:24: // Ex: "capability": [
{<VALUE>},  {<VALUE>},  {<VALUE>} ]
Nit: Since the key is not literally the word capability, use <CAPABILITY>
instead of capability?

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:26: // Traits specifies how <VALUE>
is stored in JSON and semantical validation.
Nit: Semantical->Semantic

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:27: template <class Option, class
Traits>
Please write in a comment somewhere the interface Traits must expose for
completeness.

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:71: // Ex: "capability": {
"option": [{ "is_default": true, <VALUE>}, {<VALUE>} ]}
Ditto on "capability"

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:116: bool no_default =
(default_idx_ >= size());
Why not use a signed default_idx_ == -1?

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:140: // Ex: "capability": {
"default_value": true }
Ditto on "capability"

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:169: // Represents CDD capability
that only existence is important for us.
Nit: that->for which

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:170: // Ex: "capability": { }
Ditto on "capability"

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:186: // Ex: "capability": {<VALUE>}
Ditto on "capability"

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
File components/cloud_devices/printer_description_unittest.cc (right):

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/printer_description_unittest.cc:487:
TEST(PrinterDescriptionTest, GctSetAll) {
CjtSetAll?

[Vitaly Buka (NO REVIEWS), 2014-02-03 22:06:52]

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
File components/cloud_devices/description_items.h (right):

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:23: // Represents CDD capability
that stored JSON list.
On 2014/02/03 20:30:34, Noam Samuel wrote:
> Nit: Rephrase as
> 
> "Represents a CDD capability that is stored as a JSON list"

Done.

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:24: // Ex: "capability": [
{<VALUE>},  {<VALUE>},  {<VALUE>} ]
On 2014/02/03 20:30:34, Noam Samuel wrote:
> Nit: Since the key is not literally the word capability, use <CAPABILITY>
> instead of capability?

Done.

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:26: // Traits specifies how <VALUE>
is stored in JSON and semantical validation.
On 2014/02/03 20:30:34, Noam Samuel wrote:
> Nit: Semantical->Semantic

Done.

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:27: template <class Option, class
Traits>
On 2014/02/03 20:30:34, Noam Samuel wrote:
> Please write in a comment somewhere the interface Traits must expose for
> completeness.

Done.

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:71: // Ex: "capability": {
"option": [{ "is_default": true, <VALUE>}, {<VALUE>} ]}
On 2014/02/03 20:30:34, Noam Samuel wrote:
> Ditto on "capability"

Done.

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:116: bool no_default =
(default_idx_ >= size());
On 2014/02/03 20:30:34, Noam Samuel wrote:
> Why not use a signed default_idx_ == -1?

Done.

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:140: // Ex: "capability": {
"default_value": true }
On 2014/02/03 20:30:34, Noam Samuel wrote:
> Ditto on "capability"

Done.

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:169: // Represents CDD capability
that only existence is important for us.
On 2014/02/03 20:30:34, Noam Samuel wrote:
> Nit: that->for which

Done.

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:170: // Ex: "capability": { }
On 2014/02/03 20:30:34, Noam Samuel wrote:
> Ditto on "capability"

Done.

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/description_items.h:186: // Ex: "capability": {<VALUE>}
On 2014/02/03 20:30:34, Noam Samuel wrote:
> Ditto on "capability"

Done.

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
File components/cloud_devices/printer_description_unittest.cc (right):

https://codereview.chromium.org/150993002/diff/210001/components/cloud_device...
components/cloud_devices/printer_description_unittest.cc:487:
TEST(PrinterDescriptionTest, GctSetAll) {
On 2014/02/03 20:30:34, Noam Samuel wrote:
> CjtSetAll?

Done.

[Noam Samuel, 2014-02-03 22:12:33]

lgtm

[commit-bot: I haz the power, 2014-02-03 22:15:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vitalybuka@chromium.org/150993002/360001

[Vitaly Buka (NO REVIEWS), 2014-02-03 22:24:01]

The CQ bit was unchecked by vitalybuka@chromium.org

[commit-bot: I haz the power, 2014-02-03 22:24:03]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-02-03 22:24:07]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-02-03 22:24:34]

CQ bit was unchecked on CL. Ignoring.

[Vitaly Buka (NO REVIEWS), 2014-02-03 22:24:59]

The CQ bit was checked by vitalybuka@chromium.org

[commit-bot: I haz the power, 2014-02-03 22:29:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vitalybuka@chromium.org/150993002/500001

[commit-bot: I haz the power, 2014-02-04 01:02:03]

Change committed as 248644

[commit-bot: I haz the power, 2014-02-04 01:02:04]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-02-04 01:02:04]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-02-04 01:02:09]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-02-04 01:02:10]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-02-04 01:02:13]

CQ bit was unchecked on CL. Ignoring.