drive: Fix lifetime model of FileBrowserEventRouter

chrome/browser/chromeos/extensions/file_browser_event_router.h

 // 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.
 
 #ifndef CHROME_BROWSER_CHROMEOS_EXTENSIONS_FILE_BROWSER_EVENT_ROUTER_H_
 #define CHROME_BROWSER_CHROMEOS_EXTENSIONS_FILE_BROWSER_EVENT_ROUTER_H_
 
 #include <map>
 #include <set>
 #include <string>
@@ skipping 16 matching lines @@
 class Profile;
 
 namespace drive {
 class DriveEntryProto;
 class DriveFileSystemInterface;
 }
 
 // Monitors changes in disk mounts, network connection state and preferences
 // affecting File Manager. Dispatches appropriate File Browser events.
 class FileBrowserEventRouter
-    : public base::RefCountedThreadSafe<FileBrowserEventRouter>,
-      public chromeos::disks::DiskMountManager::Observer,
+    : public chromeos::disks::DiskMountManager::Observer,
       public chromeos::ConnectivityStateHelperObserver,
       public chromeos::SystemKeyEventListener::ModifiersObserver,
       public drive::DriveFileSystemObserver,
       public google_apis::DriveServiceObserver {
  public:
+  virtual ~FileBrowserEventRouter();

[hashimoto, 2013/04/08 04:49:38]

nit: How about moving this dtor to after SuspendStateDelegate?

[satorux1, 2013/04/08 05:03:37]

Done.

+
   // Interface that should keep track of the system state in regards to system
   // suspend and resume events.
   // When the |IsResuming()| returns true, it should be able to check if a
   // removable device was present before the was system suspended.
   class SuspendStateDelegate {
    public:
     virtual ~SuspendStateDelegate() {}
 
     // Returns true if the system has recently woken up.
     virtual bool SystemIsResuming() const = 0;
@@ skipping 49 matching lines @@
       const base::FilePath& directory_path) OVERRIDE;
   virtual void OnResourceListFetched(int num_accumulated_entries) OVERRIDE;
   virtual void OnFileSystemMounted() OVERRIDE;
   virtual void OnFileSystemBeingUnmounted() OVERRIDE;
 
   // chromeos::SystemKeyEventListener::ModifiersObserver overrides.
   virtual void OnModifiersChange(int pressed_modifiers) OVERRIDE;
 
  private:
   friend class FileBrowserPrivateAPI;
-  friend class base::RefCountedThreadSafe<FileBrowserEventRouter>;
 
   typedef std::map<std::string, int> ExtensionUsageRegistry;
 
   class FileWatcherExtensions {
    public:
     FileWatcherExtensions(const base::FilePath& path,
         const std::string& extension_id,
         bool is_remote_file_system);
 
     ~FileWatcherExtensions();
@@ skipping 15 matching lines @@
     linked_ptr<base::FilePathWatcher> file_watcher_;
     base::FilePath local_path_;
     base::FilePath virtual_path_;
     ExtensionUsageRegistry extensions_;
     unsigned int ref_count_;
     bool is_remote_file_system_;
   };
 
   typedef std::map<base::FilePath, FileWatcherExtensions*> WatcherMap;
 
   explicit FileBrowserEventRouter(Profile* profile);

[hashimoto, 2013/04/08 04:49:38]

Can't we make this ctor public and remove "friend class FileBrowserPrivateAPI"?
If we are to keep "friend class FileBrowserPrivateAPI", we don't have to make
the dtor public.

[satorux1, 2013/04/08 05:03:37]

Done. Good catch!

-  virtual ~FileBrowserEventRouter();
 
   // USB mount event handlers.
   void OnDiskAdded(const chromeos::disks::DiskMountManager::Disk* disk);
   void OnDiskRemoved(const chromeos::disks::DiskMountManager::Disk* disk);
   void OnDiskMounted(const chromeos::disks::DiskMountManager::Disk* disk);
   void OnDiskUnmounted(const chromeos::disks::DiskMountManager::Disk* disk);
   void OnDeviceAdded(const std::string& device_path);
   void OnDeviceRemoved(const std::string& device_path);
   void OnDeviceScanned(const std::string& device_path);
   void OnFormatStarted(const std::string& device_path, bool success);
@@ skipping 28 matching lines @@
   // Returns the DriveFileSystem for the current profile.
   drive::DriveFileSystemInterface* GetRemoteFileSystem() const;
 
   // Handles requests to start and stop periodic updates on remote file system.
   // When |start| is set to true, this function starts periodic updates only if
   // it is not yet started; when |start| is set to false, this function stops
   // periodic updates only if the number of outstanding update requests reaches
   // zero.
   void HandleRemoteUpdateRequestOnUIThread(bool start);
 
-  base::WeakPtrFactory<FileBrowserEventRouter> weak_factory_;
   base::FilePathWatcher::Callback file_watcher_callback_;
   WatcherMap file_watchers_;
   scoped_ptr<FileBrowserNotifications> notifications_;
   scoped_ptr<PrefChangeRegistrar> pref_change_registrar_;
   scoped_ptr<SuspendStateDelegate> suspend_state_delegate_;
   Profile* profile_;
   base::Lock lock_;
 
   // Number of active update requests on the remote file system.
   int num_remote_update_requests_;
 
   // Event router behavior depends on shift modifier status. This is designed
   // for power users.
   bool shift_pressed_;
 
+  // Note: This should remain the last member so it'll be destroyed and
+  // invalidate the weak pointers before any other members are destroyed.
+  base::WeakPtrFactory<FileBrowserEventRouter> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(FileBrowserEventRouter);
 };
 
 #endif  // CHROME_BROWSER_CHROMEOS_EXTENSIONS_FILE_BROWSER_EVENT_ROUTER_H_