Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1742)

Unified Diff: chrome/browser/media_gallery/mtp_device_delegate_impl_linux.h

Issue 11358243: Redesigned and refactored ScopedMTPDeviceMapEntry, MTPDeviceMapService & MTPDeviceDelegate classes. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Fix unit test Created 8 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/media_gallery/mtp_device_delegate_impl_linux.h
diff --git a/chrome/browser/media_gallery/mtp_device_delegate_impl_linux.h b/chrome/browser/media_gallery/mtp_device_delegate_impl_linux.h
index 5f2802211642f6c4b3351c5804972e7e5bdf7064..cffb826c15d377eb213a8d2e2725c90bb2344f7c 100644
--- a/chrome/browser/media_gallery/mtp_device_delegate_impl_linux.h
+++ b/chrome/browser/media_gallery/mtp_device_delegate_impl_linux.h
@@ -7,10 +7,9 @@
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
#include "base/platform_file.h"
#include "base/synchronization/waitable_event.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
#include "webkit/fileapi/file_system_file_util.h"
#include "webkit/fileapi/media/mtp_device_delegate.h"
@@ -22,19 +21,23 @@ class SequencedTaskRunner;
namespace chrome {
-// Helper class to communicate with MTP storage to complete isolated file system
-// operations. This class contains platform specific code to communicate with
-// the attached MTP storage. Instantiate this class per MTP storage.
-// This class is ref-counted, because MTPDeviceDelegate is ref-counted.
-class MTPDeviceDelegateImplLinux : public fileapi::MTPDeviceDelegate,
- public content::NotificationObserver {
+// This class communicates with the MTP storage to complete the isolated file
+// system operations. This class contains platform specific code to communicate
+// with the attached MTP storage. Instantiate this class per MTP storage. This
+// object is constructed on the UI thread. This object is operated and
+// destructed on the sequenced task runner thread. ScopedMTPDeviceMapEntry class
+// manages the lifetime of this object via MTPDeviceMapService class. This class
+// supports weak pointers because fileapi::MTPDeviceDelegate supports weak
kinuko 2012/11/20 06:25:33 nit: fileapi::MTPDeviceDelegate -> 'the base class
kmadhusu 2012/11/21 04:09:53 Done.
+// pointers.
+class MTPDeviceDelegateImplLinux : public fileapi::MTPDeviceDelegate {
public:
- // Constructed on UI thread. Defer the device initializations until the first
- // file operation request. Do all the initializations in LazyInit() function.
+ // Should only be called by ScopedMTPDeviceMapEntry. Use
+ // GetAsWeakPtrOnIOThread() to get a weak pointer instance of this class.
+ // Defer the device initializations until the first file operation request.
+ // Do all the initializations in LazyInit() function.
explicit MTPDeviceDelegateImplLinux(const std::string& device_location);
- // Overridden from MTPDeviceDelegate. All the functions are called on
- // |media_task_runner_|.
+ // MTPDeviceDelegate:
kinuko 2012/11/20 06:25:33 nit: MTPDeviceDelegate overrides ?
kmadhusu 2012/11/21 04:09:53 During MTPDeviceDelegateWin CL review, pkasting@ m
virtual base::PlatformFileError GetFileInfo(
const FilePath& file_path,
base::PlatformFileInfo* file_info) OVERRIDE;
@@ -46,25 +49,27 @@ class MTPDeviceDelegateImplLinux : public fileapi::MTPDeviceDelegate,
const FilePath& local_path,
base::PlatformFileInfo* file_info) OVERRIDE;
virtual base::SequencedTaskRunner* GetMediaTaskRunner() OVERRIDE;
- virtual void DeleteOnCorrectThread() const OVERRIDE;
+ virtual void CancelPendingTasksAndDeleteDelegate() OVERRIDE;
+ virtual base::WeakPtr<fileapi::MTPDeviceDelegate> GetAsWeakPtrOnIOThread()
+ OVERRIDE;
private:
- friend struct fileapi::MTPDeviceDelegateDeleter;
- friend class base::DeleteHelper<MTPDeviceDelegateImplLinux>;
-
- // Private because this class is ref-counted.
+ // Destructed via DeleteDelegateOnTaskRunner(). Do all the clean up in
+ // DeleteDelegateOnTaskRunner().
virtual ~MTPDeviceDelegateImplLinux();
- // content::NotificationObserver implementation.
- virtual void Observe(int type,
- const content::NotificationSource& source,
- const content::NotificationDetails& details) OVERRIDE;
-
// Opens the device for communication. This function is called on
// |media_task_runner_|. Returns true if the device is ready for
// communication, else false.
bool LazyInit();
+ // Deletes the delegate on the task runner thread. This is called when the
Lei Zhang 2012/11/21 01:33:30 The part that starts with "This is called" is a re
kmadhusu 2012/11/21 04:09:53 Done.
+ // browser application is in shutdown mode or when the last extension using
+ // this MTP device delegate is destroyed or when the MTP device is removed
+ // from the system or when an extension revokes the MTP device media gallery
+ // permissions.
+ void DeleteDelegateOnTaskRunner();
+
// Stores the registered file system device path value. This path does not
// correspond to a real device path (E.g.: "/usb:2,2:81282").
const std::string device_path_;
@@ -78,17 +83,13 @@ class MTPDeviceDelegateImplLinux : public fileapi::MTPDeviceDelegate,
scoped_refptr<base::SequencedTaskRunner> media_task_runner_;
// |media_task_runner_| can wait on this event until the requested task is
- // complete.
+ // complete. Signaled on a different thread.
Lei Zhang 2012/11/21 01:33:30 I think the comment is a given. If a thread is wai
kmadhusu 2012/11/21 04:09:53 Removed the comment.
base::WaitableEvent on_task_completed_event_;
// Used to notify |media_task_runner_| pending tasks about the shutdown
- // sequence.
+ // sequence. Signaled on the IO thread.
base::WaitableEvent on_shutdown_event_;
- // Handles registering notifications with the NotificationService.
- // Used to listen for application termination message.
- content::NotificationRegistrar registrar_;
-
DISALLOW_COPY_AND_ASSIGN(MTPDeviceDelegateImplLinux);
};

Powered by Google App Engine
This is Rietveld 408576698