Add Drive API parser for About/Apps json

Add Drive API parser for About/Apps json

BUG=chromium:127728
TEST=unit_test --gtest_filter="DriveApiParser.*"

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

[satorux1, 2012-07-24 16:15:26]

http://codereview.chromium.org/10810070/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/drive_api_parser.h (right):

http://codereview.chromium.org/10810070/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/drive_api_parser.h:43: static
scoped_ptr<AboutResource> CreateFrom(const base::Value& value);
function comment is missing. CreateFrom() and Parse() are similar. which one
should we use?

http://codereview.chromium.org/10810070/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/drive_api_parser.h:49: const std::string&
root_folder_id() const { return root_folder_id_; }
please document this. with the old API, the root directory resource ID is always
set to "folder:root". is this the same with the new API?

http://codereview.chromium.org/10810070/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/drive_api_parser.h:67: class DriveAppIcon {
class comment is missing.

http://codereview.chromium.org/10810070/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/drive_api_parser.h:88: int size() const { return
size_; }
size -> side_length() ?

http://codereview.chromium.org/10810070/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/drive_api_parser.h:109: class AppResource {
class comment

http://codereview.chromium.org/10810070/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/drive_api_parser.h:126: const GURL& product_url()
const { return product_url_; }
Please document these fields.

http://codereview.chromium.org/10810070/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/drive_api_parser.h:182: class AppList {
class comment.

[kochi, 2012-07-25 06:24:04]

Thanks for the early review.  Added more comments to
read the code easier.

https://chromiumcodereview.appspot.com/10810070/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/gdata/drive_api_parser.h (right):

https://chromiumcodereview.appspot.com/10810070/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/gdata/drive_api_parser.h:43: static
scoped_ptr<AboutResource> CreateFrom(const base::Value& value);
On 2012/07/24 16:15:26, satorux1 wrote:
> function comment is missing. CreateFrom() and Parse() are similar. which one
> should we use?

Done.

https://chromiumcodereview.appspot.com/10810070/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/gdata/drive_api_parser.h:49: const std::string&
root_folder_id() const { return root_folder_id_; }
The short answer is no, thus this field exists.
I also added a link to original json description in
developers.google.com in the class comment.

https://chromiumcodereview.appspot.com/10810070/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/gdata/drive_api_parser.h:67: class DriveAppIcon {
On 2012/07/24 16:15:26, satorux1 wrote:
> class comment is missing.

Done.

https://chromiumcodereview.appspot.com/10810070/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/gdata/drive_api_parser.h:88: int size() const { return
size_; }
I tried to use the same field name as JSON, but I think this might be confusing
against other STL size() usages.
As this basically plays same role as AppIcon class for WAPI,
I copied the name (icon_side_length) from it.

https://chromiumcodereview.appspot.com/10810070/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/gdata/drive_api_parser.h:109: class AppResource {
On 2012/07/24 16:15:26, satorux1 wrote:
> class comment 

Done.

https://chromiumcodereview.appspot.com/10810070/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/gdata/drive_api_parser.h:126: const GURL& product_url()
const { return product_url_; }
On 2012/07/24 16:15:26, satorux1 wrote:
> Please document these fields.

Done.

https://chromiumcodereview.appspot.com/10810070/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/gdata/drive_api_parser.h:182: class AppList {
On 2012/07/24 16:15:26, satorux1 wrote:
> class comment.

Done.

[kochi, 2012-07-25 13:40:51]

https://chromiumcodereview.appspot.com/10810070/diff/8001/chrome/browser/chro...
File chrome/browser/chromeos/gdata/drive_api_parser.h (right):

https://chromiumcodereview.appspot.com/10810070/diff/8001/chrome/browser/chro...
chrome/browser/chromeos/gdata/drive_api_parser.h:63: bool Parse(const
base::Value& value) OVERRIDE;
no OVERRIDE.

[satorux1, 2012-07-25 19:39:57]

LGTM with nits

http://codereview.chromium.org/10810070/diff/8001/chrome/browser/chromeos/gda...
File chrome/browser/chromeos/gdata/drive_api_parser.cc (right):

http://codereview.chromium.org/10810070/diff/8001/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/drive_api_parser.cc:86: bool
CheckResourceKind(const base::Value& value,
maybe HasResourceKind() is a bit better?

http://codereview.chromium.org/10810070/diff/8001/chrome/browser/chromeos/gda...
File chrome/browser/chromeos/gdata/drive_api_parser.h (right):

http://codereview.chromium.org/10810070/diff/8001/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/drive_api_parser.h:34: //
https://developers.google.com/drive/v2/reference/about
Nice to have a link like this!

http://codereview.chromium.org/10810070/diff/8001/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/drive_api_parser.h:138: // Returns application ID.
how does it look like? could you add an example here?

http://codereview.chromium.org/10810070/diff/8001/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/drive_api_parser.h:144: // Returns the name of the
type of object this application creates.
ditto.

http://codereview.chromium.org/10810070/diff/8001/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/drive_api_parser.h:154: bool installed() const {
return installed_; }
maybe add is_

http://codereview.chromium.org/10810070/diff/8001/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/drive_api_parser.h:158: bool authorized() const {
return authorized_; }
ditto.

[kochi, 2012-07-26 02:16:10]

http://codereview.chromium.org/10810070/diff/8001/chrome/browser/chromeos/gda...
File chrome/browser/chromeos/gdata/drive_api_parser.cc (right):

http://codereview.chromium.org/10810070/diff/8001/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/drive_api_parser.cc:86: bool
CheckResourceKind(const base::Value& value,
As it checks against expected value, renamed to IsResourceKindExpected().

http://codereview.chromium.org/10810070/diff/8001/chrome/browser/chromeos/gda...
File chrome/browser/chromeos/gdata/drive_api_parser.h (right):

http://codereview.chromium.org/10810070/diff/8001/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/drive_api_parser.h:138: // Returns application ID.
On 2012/07/25 19:39:57, satorux1 wrote:
> how does it look like? could you add an example here?

Done.

http://codereview.chromium.org/10810070/diff/8001/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/drive_api_parser.h:144: // Returns the name of the
type of object this application creates.
I thought this is mime-type, but it seems not.
I'll figure out what is this for, added TODO.

http://codereview.chromium.org/10810070/diff/8001/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/drive_api_parser.h:154: bool installed() const {
return installed_; }
On 2012/07/25 19:39:57, satorux1 wrote:
> maybe add is_

Done.

http://codereview.chromium.org/10810070/diff/8001/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/drive_api_parser.h:158: bool authorized() const {
return authorized_; }
On 2012/07/25 19:39:57, satorux1 wrote:
> ditto.

Done.

[kochi, 2012-07-26 04:05:28]

Ben, could you check chrome/ for OWNERS?

Thanks,

[Ben Goodger (Google), 2012-07-26 15:50:46]

lgtm

[commit-bot: I haz the power, 2012-07-26 23:16:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kochi@chromium.org/10810070/7004