Get rid of DriveEntryKind.

Get rid of DriveEntryKind.

Basically DriveEntryKind is used for clasifying files into one of {Folder, Hosted document, Normal file}.
We can classify them without DriveEntryKind by using FileResource::IsDirectory() and their mime type.
Utility functions for clasification are now indepenent with ResourceEntry, and placed in drive_api_util.h.
ResourceEntryKind is defined to provide users of ResourceEntry (e.g. sync file system) with similar accessor to file kind.

BUG=357038
TBR=rogerta@chromium.org for one line removal in .gyp file and .gn file.  
TEST=run google_apis_unittests and unit_tests

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

[fukino, 2014-07-10 06:45:28]

hashimoto san, could you take a look?

As moving functions have strong relationship with DriveEntryKind, I placed them
in drive_entry_kinds.cc (created).
Please let me know if another place is better (e.g. drive_api_util.cc?).

https://codereview.chromium.org/384543004/diff/1/google_apis/drive/gdata_wapi...
File google_apis/drive/gdata_wapi_parser.cc (left):

https://codereview.chromium.org/384543004/diff/1/google_apis/drive/gdata_wapi...
google_apis/drive/gdata_wapi_parser.cc:77: const char* entry;
Only ResourceEntry has dependency on the relationship between EntryKindMap::kind
and EntryKindMap::entry, so I keep this information inside ResourceEntry (and
information about relationship between kind and extension is moved to
drive_entry_kinds.cc).

[hashimoto, 2014-07-10 08:33:12]

Thank you for taking this task!
While you are working on this part, could you get rid of DriveEntryKind at all?
(if there is one place where DriveEntryKind cannot be removed, it should be UMA
related code)

Currently, DriveEntryKind is used as an intermediate type for doing the
following mappings:

 File extension -> Is this a Google Drive hosted document?
 MIME type -> Is this a Google Drive hosted document?
              Is this a directory?
              Is this a normal file? (i.e. not a directory nor a hosted
document)
              Special extensions like .gdoc, .gsheet.

These mapping can be processed without DriveEntryKind.
We are using DriveEntryKind only because it has been there since the age of
WAPI.

[fukino, 2014-07-14 08:00:46]

On 2014/07/10 08:33:12, hashimoto wrote:
> Thank you for taking this task!
> While you are working on this part, could you get rid of DriveEntryKind at
all?
> (if there is one place where DriveEntryKind cannot be removed, it should be
UMA
> related code)
> 
> Currently, DriveEntryKind is used as an intermediate type for doing the
> following mappings:
> 
>  File extension -> Is this a Google Drive hosted document?
>  MIME type -> Is this a Google Drive hosted document?
>               Is this a directory?
>               Is this a normal file? (i.e. not a directory nor a hosted
> document)
>               Special extensions like .gdoc, .gsheet.
> 
> These mapping can be processed without DriveEntryKind.
> We are using DriveEntryKind only because it has been there since the age of
> WAPI.

Thank you for suggesting that DriveEntryKind can be completely removed!
I made a CL to do it (and modified CL's description).
Could you take a look?

[hashimoto, 2014-07-14 08:23:26]

Thank you for doing this.
Nice to see 370+ lines being removed!

lgtm with nits.
Please get reviewed by one of sync_file_system owners.

https://codereview.chromium.org/384543004/diff/60001/chrome/browser/chromeos/...
File chrome/browser/chromeos/drive/resource_entry_conversion.cc (right):

https://codereview.chromium.org/384543004/diff/60001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/resource_entry_conversion.cc:82: const bool
is_file = !is_folder && !is_hosted_document;
nit: No need to have this bool now?

https://codereview.chromium.org/384543004/diff/60001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/resource_entry_conversion.cc:126: // The entry is
something that doesn't map into files (i.e. sites).
This branch is never reached.

https://codereview.chromium.org/384543004/diff/60001/chrome/browser/drive/dri...
File chrome/browser/drive/drive_api_util.cc (right):

https://codereview.chromium.org/384543004/diff/60001/chrome/browser/drive/dri...
chrome/browser/drive/drive_api_util.cc:379: bool
IsHostedDocumentByFileExtension(const base::FilePath& file_path) {
Could you remove one of IsHostedDocumentByExtension or
IsHostedDocumentByFileExtension?

https://codereview.chromium.org/384543004/diff/60001/chrome/browser/drive/dri...
chrome/browser/drive/drive_api_util.cc:380: #if defined(OS_WIN)
You can get rid of this #if with:
 extension = base::FilePath(file_path.Extension()).AsUTF8Unsafe().

[fukino, 2014-07-14 09:10:54]

Thank you for comments!

https://codereview.chromium.org/384543004/diff/60001/chrome/browser/chromeos/...
File chrome/browser/chromeos/drive/resource_entry_conversion.cc (right):

https://codereview.chromium.org/384543004/diff/60001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/resource_entry_conversion.cc:82: const bool
is_file = !is_folder && !is_hosted_document;
On 2014/07/14 08:23:26, hashimoto wrote:
> nit: No need to have this bool now?

Done. Removed these temporary bool variables.

https://codereview.chromium.org/384543004/diff/60001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/resource_entry_conversion.cc:126: // The entry is
something that doesn't map into files (i.e. sites).
On 2014/07/14 08:23:26, hashimoto wrote:
> This branch is never reached.

Done.

https://codereview.chromium.org/384543004/diff/60001/chrome/browser/drive/dri...
File chrome/browser/drive/drive_api_util.cc (right):

https://codereview.chromium.org/384543004/diff/60001/chrome/browser/drive/dri...
chrome/browser/drive/drive_api_util.cc:379: bool
IsHostedDocumentByFileExtension(const base::FilePath& file_path) {
On 2014/07/14 08:23:26, hashimoto wrote:
> Could you remove one of IsHostedDocumentByExtension or
> IsHostedDocumentByFileExtension?

Done. IsHostedDocumentByExtension survived.

https://codereview.chromium.org/384543004/diff/60001/chrome/browser/drive/dri...
chrome/browser/drive/drive_api_util.cc:380: #if defined(OS_WIN)
On 2014/07/14 08:23:26, hashimoto wrote:
> You can get rid of this #if with:
>  extension = base::FilePath(file_path.Extension()).AsUTF8Unsafe().

Done. I didn't know, thanks!

[fukino, 2014-07-14 09:15:47]

tzik san, Could you take a look at chrome/browser/sync_file_system/*?
DriveEntryKind is gone, but their counterparts are inside ResourceEntry, so
changed sync_file_system to use them.

[tzik, 2014-07-14 09:32:53]

lgtm

[fukino, 2014-07-14 10:05:01]

On 2014/07/14 09:32:53, tzik wrote:
> lgtm

Thank you!

[fukino, 2014-07-14 10:08:18]

Hi rogerta@, I TBRed you for google_apis/*.{gyp,gn} as this CL simply removes an
unused file and you're OOO now. If there's a problem, please let me know.
Thanks!

[fukino, 2014-07-14 10:09:52]

The CQ bit was checked by fukino@chromium.org

[commit-bot: I haz the power, 2014-07-14 10:10:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fukino@chromium.org/384543004/80001

[commit-bot: I haz the power, 2014-07-14 12:06:49]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-07-14 13:58:28]

Change committed as 282941