[MediaGalleries] Enable iPhoto gallery

chrome/browser/media_galleries/media_file_system_registry.cc

 // Copyright (c) 2012 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.
 
 // MediaFileSystemRegistry implementation.
 
 #include "chrome/browser/media_galleries/media_file_system_registry.h"
 
 #include <set>
 #include <vector>
@@ skipping 130 matching lines @@
       }
       default: {
         NOTREACHED();
         break;
       }
     }
   }
 
   void OnRendererProcessTerminated(const RenderProcessHost* rph) {
     RPHRefCount::iterator rph_info = refs_.find(rph);
+    // Will this crash if the RPH is navigated to a page on the same host

[vandebo (ex-Chrome), 2014/01/06 18:45:03]

This this a debug comment?

[Greg Billock, 2014/01/07 16:50:55]

I think this is a real hole. Filling.

+    // (triggering OWCDON()) and then the tab crashes?
     DCHECK(rph_info != refs_.end());
     delete rph_info->second;
     refs_.erase(rph_info);
     if (refs_.empty())
       no_references_callback_.Run();
   }
 
   void OnWebContentsDestroyedOrNavigated(const WebContents* contents) {
     RenderProcessHost* rph = contents->GetRenderProcessHost();
     RPHRefCount::iterator rph_info = refs_.find(rph);
@@ skipping 119 matching lines @@
 
  private:
   typedef std::map<MediaGalleryPrefId, MediaFileSystemInfo> PrefIdFsInfoMap;
 
   // Private destructor and friend declaration for ref counted implementation.
   friend class base::RefCountedThreadSafe<ExtensionGalleriesHost>;
 
   virtual ~ExtensionGalleriesHost() {
     DCHECK(rph_refs_.empty());
     DCHECK(pref_id_map_.empty());
-
   }
 
+  // It appears there's a shutdown race in some browser tests. If the RPHs shut
+  // down first, there may still be Filter... calls pending which will then
+  // invoke this method interleaved in the destruction callback sequence and
+  // re-populate pref_id_map_.

[vandebo (ex-Chrome), 2014/01/06 18:45:03]

So if rph_refs_ is empty, we know that we lost the race and shouldn't do
anything?

[Greg Billock, 2014/01/07 16:50:55]

I also looked, and I agree this is a fine check. Added it.

   void GetMediaFileSystemsForAttachedDevices(
       const MediaStorageUtil::DeviceIdSet* attached_devices,
       const MediaGalleryPrefIdSet& galleries,
       const MediaGalleriesPrefInfoMap& galleries_info,
       const MediaFileSystemsCallback& callback) {
     std::vector<MediaFileSystemInfo> result;
     MediaGalleryPrefIdSet new_galleries;
     for (std::set<MediaGalleryPrefId>::const_iterator pref_id_it =
              galleries.begin();
          pref_id_it != galleries.end();
@@ skipping 200 matching lines @@
   virtual std::string RegisterFileSystem(
       const std::string& device_id, const base::FilePath& path) OVERRIDE {
     if (StorageInfo::IsMassStorageDevice(device_id)) {
       return RegisterFileSystemForMassStorage(device_id, path);
     } else {
       return RegisterFileSystemForMTPDevice(device_id, path);
     }
   }
 
   virtual void RevokeFileSystem(const std::string& fsid) OVERRIDE {
+    // Does this need to do something about non-imported, non-MTP filesystems?
+    // It gets called during RPH shutdown, and may leave around local drive
+    // FS trash.

[vandebo (ex-Chrome), 2014/01/06 18:45:03]

What kind of trash do you mean?

[Greg Billock, 2014/01/07 16:50:55]

Not exactly sure. Is there some cleanup that needs done? I don't really remember
that there's anything to do. I'll remove the question, unless there's some TODO
here.

     ImportedMediaGalleryRegistry* imported_registry =
         ImportedMediaGalleryRegistry::GetInstance();
     if (imported_registry->RevokeImportedFilesystemOnUIThread(fsid))
       return;
 
     IsolatedContext::GetInstance()->RevokeFileSystem(fsid);
 
     content::BrowserThread::PostTask(
         content::BrowserThread::IO, FROM_HERE, base::Bind(
             &MTPDeviceMapService::RevokeMTPFileSystem,
@@ skipping 143 matching lines @@
   DCHECK_EQ(1U, erase_count);
   if (extension_hosts->second.empty()) {
     // When a profile has no ExtensionGalleriesHosts left, remove the
     // matching gallery-change-watcher since it is no longer needed. Leave the
     // |extension_hosts| entry alone, since it indicates the profile has been
     // previously used.
     MediaGalleriesPreferences* preferences = GetPreferences(profile);
     preferences->RemoveGalleryChangeObserver(this);
   }
 }