Media Galleries API Picasa: Make PicasaDataProvider handle async PMP and INI parsing robustly.

chrome/browser/media_galleries/fileapi/safe_picasa_album_table_reader.h

 // 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.
 
 #ifndef CHROME_BROWSER_MEDIA_GALLERIES_FILEAPI_SAFE_PICASA_ALBUM_TABLE_READER_H_
 #define CHROME_BROWSER_MEDIA_GALLERIES_FILEAPI_SAFE_PICASA_ALBUM_TABLE_READER_H_
 
 #include <string>
 
 #include "base/callback.h"
@@ skipping 13 matching lines @@
 
 namespace picasa {
 
 // SafePicasaAlbumTableReader parses the given Picasa PMP Album Table safely
 // via a utility process. The SafePicasaAlbumTableReader object is ref-counted
 // and kept alive after Start() is called until the ParserCallback is called.
 // The ParserCallback is guaranteed to be called eventually either when the
 // utility process replies or when it dies.
 class SafePicasaAlbumTableReader : public content::UtilityProcessHostClient {
  public:
-  typedef base::Callback<void(bool,
+  // Callback includes a refptr to itself to guarantee that this object
+  // survives past the execution of its last posted callback.
+  typedef base::Callback<void(scoped_refptr<SafePicasaAlbumTableReader>,

[vandebo (ex-Chrome), 2013/08/14 23:48:11]

I don't think there's a good reason to include this in this type definition.
Just bind it into the callback before passing it down.... But this is passed
into the constructor... move the callback to the Start method?

[tommycli, 2013/08/15 22:52:36]

PicasaDataProvider also needs to keep a reference so it knows to ignore 'stale'
instances. I'm not super happy about the lifetime of this object currently
either.

It can't be unilaterally destroyed by PicasaDataProvider because it might be
doing something on a different thread.

I could make the object 'Owned' by the callback and make PicasaDataProvider keep
a raw pointer, but is that actually better?

[vandebo (ex-Chrome), 2013/08/22 17:48:02]

Per offline discussion, Tommy looking into this again.

[tommycli, 2013/08/22 22:32:50]

Done.

+                              bool,

[vandebo (ex-Chrome), 2013/08/14 23:48:11]

nit: name this since it's unclear without a name.

[tommycli, 2013/08/15 22:52:36]

Done.

                               const std::vector<AlbumInfo>&,
                               const std::vector<AlbumInfo>&)> ParserCallback;
 
   SafePicasaAlbumTableReader(const AlbumTableFiles& album_table_files,
                              const ParserCallback& callback);
 
   void Start();
 
  private:
   enum ParserState {
@@ skipping 37 matching lines @@
   // Verifies the messages from the utility process came at the right time.
   // Initialized on the Media Task Runner, but only accessed on the IO thread.
   ParserState parser_state_;
 
   DISALLOW_COPY_AND_ASSIGN(SafePicasaAlbumTableReader);
 };
 
 }  // namespace picasa
 
 #endif  // CHROME_BROWSER_MEDIA_GALLERIES_FILEAPI_SAFE_PICASA_ALBUM_TABLE_READER_H_