Created helper class for parsing Apple style XML dictionaries.

Created helper class for parsing Apple style XML dictionaries.
Converted iTunes and iPhotos parser to use it.
BUG=302745
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287940

[Kevin Bailey, 2014-08-05 15:19:12]

Well... it's a net loss in LOC, but I think it's a lot cleaner.
In fact, it sort of fixed an issue at the end of a parse.

[Lei Zhang, 2014-08-05 19:38:24]

I think it's still a win in terms of reduced complexity.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
File chrome/utility/media_galleries/iapps_xml_utils.cc (right):

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.cc:127: while (reader_->Depth()
>= dict_content_depth && reader_->Next())
while (...) {}, rather than the semi-colon on the next line.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
File chrome/utility/media_galleries/iapps_xml_utils.h (right):

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:38: class XmlDictReader {
This class can use some comments.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:40: XmlDictReader(XmlReader*
reader);
explicit

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:46: bool HandleKey(const
std::string& key) {
please put non-trivial impls in the .cc files.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:47: if (found_.find(key) !=
found_.end())
Just call Found()?

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:64: return found_.find(key) !=
found_.end();
You can use ContainsKey() from base/stl_util.h.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:67: virtual ~XmlDictReader();
Put the dtor next to the ctor?

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:74: };
DISALLOW_COPY_AND_ASSIGN(XmlDictReader);

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
File chrome/utility/media_galleries/iphoto_library_parser.cc (right):

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iphoto_library_parser.cc:207: 
nit: no empty lines at the beginning of a block.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iphoto_library_parser.cc:221: if
(!ContainsKey(library_->albums, name)) {
This block can be shortened to:

if (ContainsKey(library_->albums, name))
  name = name + "(" + base::Uint64ToString(album_info.id) + ")";
library_->albums[name] = album;

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iphoto_library_parser.cc:249: parser::Library*
library_;
nit: blank line after

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
File chrome/utility/media_galleries/itunes_library_parser.cc (right):

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/itunes_library_parser.cc:82: private:
nit: indentation

[Kevin Bailey, 2014-08-05 21:52:06]

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
File chrome/utility/media_galleries/iapps_xml_utils.cc (right):

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.cc:127: while (reader_->Depth()
>= dict_content_depth && reader_->Next())
On 2014/08/05 19:38:24, Lei Zhang wrote:
> while (...) {}, rather than the semi-colon on the next line.

Done.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
File chrome/utility/media_galleries/iapps_xml_utils.h (right):

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:38: class XmlDictReader {
On 2014/08/05 19:38:24, Lei Zhang wrote:
> This class can use some comments.

Done.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:40: XmlDictReader(XmlReader*
reader);
On 2014/08/05 19:38:24, Lei Zhang wrote:
> explicit

Done.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:46: bool HandleKey(const
std::string& key) {
On 2014/08/05 19:38:24, Lei Zhang wrote:
> please put non-trivial impls in the .cc files.

Done.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:47: if (found_.find(key) !=
found_.end())
On 2014/08/05 19:38:24, Lei Zhang wrote:
> Just call Found()?

Done.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:64: return found_.find(key) !=
found_.end();
On 2014/08/05 19:38:24, Lei Zhang wrote:
> You can use ContainsKey() from base/stl_util.h.

Done.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:67: virtual ~XmlDictReader();
On 2014/08/05 19:38:24, Lei Zhang wrote:
> Put the dtor next to the ctor?

Done.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:74: };
On 2014/08/05 19:38:24, Lei Zhang wrote:
> DISALLOW_COPY_AND_ASSIGN(XmlDictReader);

Done.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
File chrome/utility/media_galleries/iphoto_library_parser.cc (right):

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iphoto_library_parser.cc:207: 
On 2014/08/05 19:38:24, Lei Zhang wrote:
> nit: no empty lines at the beginning of a block.

Done.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iphoto_library_parser.cc:221: if
(!ContainsKey(library_->albums, name)) {
On 2014/08/05 19:38:24, Lei Zhang wrote:
> This block can be shortened to:
> 
> if (ContainsKey(library_->albums, name))
>   name = name + "(" + base::Uint64ToString(album_info.id) + ")";
> library_->albums[name] = album;

Done.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/iphoto_library_parser.cc:249: parser::Library*
library_;
On 2014/08/05 19:38:24, Lei Zhang wrote:
> nit: blank line after

Done.

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
File chrome/utility/media_galleries/itunes_library_parser.cc (right):

https://codereview.chromium.org/436293002/diff/40001/chrome/utility/media_gal...
chrome/utility/media_galleries/itunes_library_parser.cc:82: private:
On 2014/08/05 19:38:24, Lei Zhang wrote:
> nit: indentation

Done.

[Lei Zhang, 2014-08-05 21:58:00]

lgtm

https://codereview.chromium.org/436293002/diff/60001/chrome/utility/media_gal...
File chrome/utility/media_galleries/iapps_xml_utils.h (right):

https://codereview.chromium.org/436293002/diff/60001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:75: bool Found(const
std::string& key) {
const method?

https://codereview.chromium.org/436293002/diff/60001/chrome/utility/media_gal...
chrome/utility/media_galleries/iapps_xml_utils.h:76: return ContainsKey(found_,
key);
Move to .cc file.

[Lei Zhang, 2014-08-06 21:41:40]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2014-08-06 21:42:36]

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

[commit-bot: I haz the power, 2014-08-07 02:40:38]

Change committed as 287940