PicasaAlbumTableReader for Media Galleries API

webkit/fileapi/media/picasa/picasa_albumdata_reader.cc

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "webkit/fileapi/media/picasa/picasa_albumdata_reader.h"
+
+#include <vector>
+
+#include "base/utf_string_conversions.h"
+#include "webkit/fileapi/media/picasa/pmp_column_reader.h"
+#include "webkit/fileapi/media/picasa/pmp_constants.h"
+#include "webkit/fileapi/media/picasa/pmp_table_reader.h"
+
+namespace picasaimport {
+
+namespace {
+
+const int kColCategory = 0;
+const int kColDate     = 1;
+const int kColFilename = 2;
+const int kColName     = 3;
+const int kColToken    = 4;
+const int kColUid      = 5;
+
+const uint16 albumdata_field_types[] = {
+    PMP_UINT32_TYPE,
+    PMP_DOUBLE64_TYPE,
+    PMP_STRING_TYPE,
+    PMP_STRING_TYPE,
+    PMP_STRING_TYPE,
+    PMP_STRING_TYPE
+};
+
+int albumdata_columns_len = 6;
+
+const uint16 kAlbumCategoryUserAlbum = 0;
+const uint16 kAlbumCategoryFolder    = 2;
+const uint16 kAlbumCategoryInvalid   = 0xffff;

[vandebo (ex-Chrome), 2013/04/08 19:02:30]

Is this actually defined in PMP, or is this your sentinel value?

[tommycli, 2013/04/08 23:27:10]

Just a sentinel value.

[vandebo (ex-Chrome), 2013/04/08 23:57:24]

Maybe a quick comment to that effect then.

[tommycli, 2013/04/09 00:45:33]

Done.

+
+const int kPmpVariantTimeEpochYear = 1899;
+const int kPmpVariantTimeEpochMonth = 12;
+const int kPmpVariantTimeEpochDay = 30;
+
+// |variant_time| is specified as the number of days from Dec 30, 1899.
+base::Time TimeFromMicrosoftVariantTime(double variant_time) {
+  base::Time::Exploded variant_epoch;
+
+  // First set to Dec 30, 1899, (variant time epoch).
+  variant_epoch.year = kPmpVariantTimeEpochYear;
+  variant_epoch.month = kPmpVariantTimeEpochMonth;
+  variant_epoch.day_of_month = kPmpVariantTimeEpochDay;
+
+  base::TimeDelta variant_delta = base::TimeDelta::FromMicroseconds(
+      static_cast<int64>(variant_time * base::Time::kMicrosecondsPerDay));
+
+  return base::Time::FromLocalExploded(variant_epoch) + variant_delta;
+}
+
+}  // namespace
+
+PicasaAlbumdataReader::PicasaAlbumdataReader() { }
+
+PicasaAlbumdataReader::~PicasaAlbumdataReader() { }
+
+bool PicasaAlbumdataReader::Init(const base::FilePath& directory_path) {
+  const std::string kAlbumTokenPrefix = "]album:";
+
+  std::vector<std::string> column_names;

[vandebo (ex-Chrome), 2013/04/08 19:02:30]

As we discussed, to simplify this code, we can change the TableReader interface
to be Init(tablename, path) and then have an addColumn method.

[tommycli, 2013/04/08 23:27:10]

Done.

+  column_names.push_back("category");
+  column_names.push_back("date");
+  column_names.push_back("filename");
+  column_names.push_back("name");
+  column_names.push_back("token");
+  column_names.push_back("uid");
+
+  PmpTableReader pmp_reader;
+
+  if(!pmp_reader.Init("albumdata", directory_path, column_names))
+    return false;
+
+  std::vector<const PmpColumnReader*> columns = pmp_reader.GetColumns();
+
+  // Abort if column types aren't as expected.
+  for(int i = 0; i < albumdata_columns_len; i++) {
+    if(columns[i]->field_type() != albumdata_field_types[i])
+      return false;
+  }
+
+  for(uint32 i = 0; i < pmp_reader.RowCount(); i++) {
+    uint32 category = kAlbumCategoryInvalid;
+    double date;
+    std::string name, filename, token, uid;

[vandebo (ex-Chrome), 2013/04/08 19:02:30]

Move token, filename, and uid into the scope that they are used.  It's easier
for the reader if variables are in the most specific scope possible and also if
they are defined closer to where they are used.

[tommycli, 2013/04/08 23:27:10]

Done.

+
+    if(!columns[kColCategory]->ReadUInt32(i, &category) ||
+       !columns[kColDate]->ReadDouble64(i, &date) ||
+       !columns[kColName]->ReadString(i, &name) || name.empty()) {
+      continue;
+    }
+
+    switch(category) {

[vandebo (ex-Chrome), 2013/04/08 19:02:30]

nit: space

[tommycli, 2013/04/08 23:27:10]

Done.

+      case kAlbumCategoryUserAlbum: {
+        if(!columns[kColName]->ReadString(i, &name) || name.empty() ||

[vandebo (ex-Chrome), 2013/04/08 19:02:30]

You already read name, right?

[tommycli, 2013/04/08 23:27:10]

Done.

+           !columns[kColToken]->ReadString(i, &token) || token.empty() ||
+           !columns[kColUid]->ReadString(i, &uid) || uid.empty() ||
+           token.substr(0, kAlbumTokenPrefix.size()) != kAlbumTokenPrefix) {
+          continue;
+        }
+
+        AlbumInfo album;

[vandebo (ex-Chrome), 2013/04/08 19:02:30]

A constructor for AlbumInfo would make this code simpler:

user_albums_.push_back(AlbumInfo(name, TimeFrom..., uid));

[tommycli, 2013/04/08 23:27:10]

Done.

+        album.name = name;
+        album.timestamp = TimeFromMicrosoftVariantTime(date);
+        album.uid = uid;
+
+        user_albums_.push_back(album);
+        break;
+      }
+      case kAlbumCategoryFolder: {
+        if(!columns[kColFilename]->ReadString(i, &filename) || filename.empty())
+          continue;
+
+        FolderInfo folder;
+        folder.name = name;
+        folder.timestamp = TimeFromMicrosoftVariantTime(date);

[vandebo (ex-Chrome), 2013/04/08 19:02:30]

Pull the conversion to base::Time to before the switch?

[tommycli, 2013/04/08 23:27:10]

Done.

+#if defined(OS_WIN)
+        folder.path = base::FilePath(UTF8ToUTF16(filename));
+#else
+        folder.path = base::FilePath(filename);
+#endif
+
+        folders_.push_back(folder);
+        break;
+      }
+      default: {
+        break;
+      }
+    }
+  }
+
+  return true;
+}
+
+}  // namespace picasaimport