Files.app: Start to use DeviceEventRouter.

The new class manages device states and dispatch device events property instead
of EventRouter, MountedDiskMonitor, and JavaScript side's DeviceHandler.

BUG=360946,396258

Review URL: https://codereview.chromium.org/490643005

Cr-Commit-Position: refs/heads/master@{#291891}

chrome/browser/chromeos/extensions/file_manager/device_event_router.cc

@@ -77,9 +77,8 @@ void DeviceEventRouter::OnDeviceAddedDelayed(const std::string& device_path) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (GetDeviceState(device_path) == DEVICE_SCANNED) {
-    // TODO(hirono): Rename DEVICE_EVENT_TYPE_ADDED with
-    // DEVICE_EVENT_TYPE_SCAN_STARTED.
-    OnDeviceEvent(file_browser_private::DEVICE_EVENT_TYPE_ADDED, device_path);
+    OnDeviceEvent(file_browser_private::DEVICE_EVENT_TYPE_SCAN_STARTED,
+                  device_path);
     SetDeviceState(device_path, DEVICE_SCANNED_AND_REPORTED);
   }
 }
@@ -99,7 +98,7 @@ void DeviceEventRouter::OnDiskAdded(
     // If the disk is not being mounted, mark the device scan cancelled.
     const std::string& device_path = disk.system_path_prefix();
     if (GetDeviceState(device_path) == DEVICE_SCANNED_AND_REPORTED) {
-      OnDeviceEvent(file_browser_private::DEVICE_EVENT_TYPE_SCAN_CANCELED,
+      OnDeviceEvent(file_browser_private::DEVICE_EVENT_TYPE_SCAN_CANCELLED,
                     device_path);
     }
     SetDeviceState(device_path, DEVICE_STATE_USUAL);
@@ -159,9 +158,6 @@ void DeviceEventRouter::OnFormatCompleted(const std::string& device_path,
                 device_path);
 }
 
-void DeviceEventRouter::OnHardUnplugged(const std::string& device_path) {
-}
-
 void DeviceEventRouter::SuspendImminent() {
   DCHECK(thread_checker_.CalledOnValidThread());
   is_resuming_ = true;

chrome/browser/chromeos/extensions/file_manager/device_event_router.h

@@ -57,13 +57,13 @@ class DeviceEventRouter : public VolumeManagerObserver,
   virtual void OnFormatCompleted(const std::string& device_path,
                                  bool success) OVERRIDE;
 
-  // TODO(hirono): Remove the method from VolumeManagerObserver.
-  virtual void OnHardUnplugged(const std::string& device_path) OVERRIDE;
-
   // PowerManagerClient::Observer overrides.
   virtual void SuspendImminent() OVERRIDE;
   virtual void SuspendDone(const base::TimeDelta& sleep_duration) OVERRIDE;
 
+  bool is_resuming() const { return is_resuming_; }
+  bool is_starting_up() const { return is_starting_up_; }
+
  protected:
   // Handles a device event containing |type| and |device_path|.
   virtual void OnDeviceEvent(

chrome/browser/chromeos/extensions/file_manager/device_event_router_unittest.cc

@@ -140,7 +140,7 @@ TEST_F(DeviceEventRouterTest, DeviceScan) {
   device_event_router->OnDiskRemoved(disk_unmounted);
   device_event_router->OnDeviceRemoved("/device/test");
   ASSERT_EQ(2u, device_event_router->events.size());
-  EXPECT_EQ(file_browser_private::DEVICE_EVENT_TYPE_ADDED,
+  EXPECT_EQ(file_browser_private::DEVICE_EVENT_TYPE_SCAN_STARTED,
             device_event_router->events[0].type);
   EXPECT_EQ("/device/test", device_event_router->events[0].device_path);
   EXPECT_EQ(file_browser_private::DEVICE_EVENT_TYPE_REMOVED,
@@ -159,10 +159,10 @@ TEST_F(DeviceEventRouterTest, DeviceScanCancelled) {
   device_event_router->OnDiskRemoved(disk_unmounted);
   device_event_router->OnDeviceRemoved("/device/test");
   ASSERT_EQ(3u, device_event_router->events.size());
-  EXPECT_EQ(file_browser_private::DEVICE_EVENT_TYPE_ADDED,
+  EXPECT_EQ(file_browser_private::DEVICE_EVENT_TYPE_SCAN_STARTED,
             device_event_router->events[0].type);
   EXPECT_EQ("/device/test", device_event_router->events[0].device_path);
-  EXPECT_EQ(file_browser_private::DEVICE_EVENT_TYPE_SCAN_CANCELED,
+  EXPECT_EQ(file_browser_private::DEVICE_EVENT_TYPE_SCAN_CANCELLED,
             device_event_router->events[1].type);
   EXPECT_EQ("/device/test", device_event_router->events[1].device_path);
   EXPECT_EQ(file_browser_private::DEVICE_EVENT_TYPE_REMOVED,

chrome/browser/chromeos/extensions/file_manager/event_router.h

@@ -47,6 +47,7 @@ class FileChange;
 }
 
 namespace file_manager {
+class DeviceEventRouter;
 
 // Monitors changes in disk mounts, network connection state and preferences
 // affecting File Manager. Dispatches appropriate File Browser events.
@@ -130,7 +131,6 @@ class EventRouter
                                bool is_remounting) OVERRIDE;
   virtual void OnVolumeUnmounted(chromeos::MountError error_code,
                                  const VolumeInfo& volume_info) OVERRIDE;
-  virtual void OnHardUnplugged(const std::string& device_path) OVERRIDE;
   virtual void OnFormatStarted(
       const std::string& device_path, bool success) OVERRIDE;
   virtual void OnFormatCompleted(
@@ -174,8 +174,7 @@ class EventRouter
   void DispatchMountCompletedEvent(
       extensions::api::file_browser_private::MountCompletedEventType event_type,
       chromeos::MountError error,
-      const VolumeInfo& volume_info,
-      bool is_remounting);
+      const VolumeInfo& volume_info);
 
   // If needed, opens a file manager window for the removable device mounted at
   // |mount_path|. Disk.mount_path() is empty, since it is being filled out
@@ -183,12 +182,6 @@ class EventRouter
   void ShowRemovableDeviceInFileManager(VolumeType type,
                                         const base::FilePath& mount_path);
 
-  // Dispatches an onDeviceChanged event containing |type| and |path| to
-  // extensions.
-  void DispatchDeviceEvent(
-      extensions::api::file_browser_private::DeviceEventType type,
-      const std::string& path);
-
   // Sends onFileTranferUpdated to extensions if needed. If |always| is true,
   // it sends the event always. Otherwise, it sends the event if enough time has
   // passed from the previous event so as not to make extension busy.
@@ -213,6 +206,8 @@ class EventRouter
   content::NotificationRegistrar notification_registrar_;
   bool multi_user_window_manager_observer_registered_;
 
+  scoped_ptr<DeviceEventRouter> device_event_router_;
+
   // 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<EventRouter> weak_factory_;

chrome/browser/chromeos/file_manager/volume_manager.cc

@@ -509,8 +509,6 @@ void VolumeManager::OnDiskEvent(
       const std::string& device_path = disk->system_path_prefix();
       if (mounted_disk_monitor_->DeviceIsHardUnpluggedButNotReported(
               device_path)) {
-        FOR_EACH_OBSERVER(VolumeManagerObserver, observers_,
-                          OnHardUnplugged(device_path));
         mounted_disk_monitor_->MarkAsHardUnpluggedReported(device_path);
       }
       return;

chrome/browser/chromeos/file_manager/volume_manager_observer.h

@@ -42,9 +42,6 @@ class VolumeManagerObserver {
   virtual void OnVolumeUnmounted(chromeos::MountError error_code,
                                  const VolumeInfo& volume_info) = 0;
 
-  // Fired when a device is hard unplugged.
-  virtual void OnHardUnplugged(const std::string& device_path) = 0;
-
   // Fired when formatting a device is started (or failed to start).
   virtual void OnFormatStarted(
       const std::string& device_path, bool success) = 0;

chrome/browser/chromeos/file_manager/volume_manager_unittest.cc

@@ -38,7 +38,6 @@ class LoggingObserver : public VolumeManagerObserver {
       VOLUME_UNMOUNTED,
       FORMAT_STARTED,
       FORMAT_COMPLETED,
-      HARD_UNPLUGGED,
     } type;
 
     // Available on DEVICE_ADDED, DEVICE_REMOVED, VOLUME_MOUNTED,
@@ -115,13 +114,6 @@ class LoggingObserver : public VolumeManagerObserver {
     events_.push_back(event);
   }
 
-  virtual void OnHardUnplugged(const std::string& device_path) OVERRIDE {
-    Event event;
-    event.type = Event::HARD_UNPLUGGED;
-    event.device_path = device_path;
-    events_.push_back(event);
-  }
-
   virtual void OnFormatStarted(
       const std::string& device_path, bool success) OVERRIDE {
     Event event;
@@ -389,15 +381,11 @@ TEST_F(VolumeManagerTest, OnDiskEvent_Removed) {
   volume_manager()->OnDiskEvent(
       chromeos::disks::DiskMountManager::DISK_REMOVED, &kMountedDisk);
 
-  ASSERT_EQ(2U, observer.events().size());
+  ASSERT_EQ(1U, observer.events().size());
   const LoggingObserver::Event& event = observer.events()[0];
   EXPECT_EQ(LoggingObserver::Event::DISK_REMOVED, event.type);
   EXPECT_EQ("device1", event.device_path);
 
-  // Since the Disk has non-empty mount_path, it's regarded as hard unplugging.
-  EXPECT_EQ(LoggingObserver::Event::HARD_UNPLUGGED,
-            observer.events()[1].type);
-
   ASSERT_EQ(1U, disk_mount_manager_->unmount_requests().size());
   const FakeDiskMountManager::UnmountRequest& unmount_request =
       disk_mount_manager_->unmount_requests()[0];
@@ -838,74 +826,6 @@ TEST_F(VolumeManagerTest, ArchiveSourceFiltering) {
   EXPECT_EQ(3u, observer.events().size());
 }
 
-TEST_F(VolumeManagerTest, HardUnplugged) {
-  volume_manager()->Initialize();
-  LoggingObserver observer;
-  volume_manager()->AddObserver(&observer);
-
-  // Disk that has a mount path is removed.
-  chromeos::disks::DiskMountManager::Disk mounted_disk(
-      "device1",
-      "/mount/path",
-      "",
-      "",
-      "",
-      "",
-      "",
-      "",
-      "",
-      "",
-      "uuid1",
-      "device1",
-      chromeos::DEVICE_TYPE_UNKNOWN,
-      0,
-      false,
-      false,
-      false,
-      false,
-      false,
-      false);
-
-  chromeos::disks::DiskMountManager::Disk unmounted_disk(
-      "device2",
-      "",
-      "",
-      "",
-      "",
-      "",
-      "",
-      "",
-      "",
-      "",
-      "uuid2",
-      "device2",
-      chromeos::DEVICE_TYPE_UNKNOWN,
-      0,
-      false,
-      false,
-      false,
-      false,
-      false,
-      false);
-
-  // Do not publish the hard_unplugged event for a disk that is already
-  // unmounted.
-  disk_mount_manager_->InvokeDiskEventForTest(
-      chromeos::disks::DiskMountManager::DISK_REMOVED, &unmounted_disk);
-  // Publish the hard_unplugged event for a disk that is currently mounted.
-  disk_mount_manager_->InvokeDiskEventForTest(
-      chromeos::disks::DiskMountManager::DISK_REMOVED, &mounted_disk);
-  // Do not publish the hard_unplugged event twice for the same disk.
-  disk_mount_manager_->InvokeDiskEventForTest(
-      chromeos::disks::DiskMountManager::DISK_REMOVED, &mounted_disk);
-
-  EXPECT_EQ(4u, observer.events().size());
-  EXPECT_EQ(LoggingObserver::Event::DISK_REMOVED, observer.events()[0].type);
-  EXPECT_EQ(LoggingObserver::Event::DISK_REMOVED, observer.events()[1].type);
-  EXPECT_EQ(LoggingObserver::Event::HARD_UNPLUGGED, observer.events()[2].type);
-  EXPECT_EQ(LoggingObserver::Event::DISK_REMOVED, observer.events()[3].type);
-}
-
 TEST_F(VolumeManagerTest, MTPPlugAndUnplug) {
   LoggingObserver observer;
   volume_manager()->AddObserver(&observer);

chrome/common/extensions/api/file_browser_private.idl

@@ -98,14 +98,12 @@ enum InspectionType {
 
 // Device event type.
 enum DeviceEventType {
-  // Device is added.
-  added,
-  // If the device is disabled by preference, the disabled event is published
-  // instead of the added event.
+  // Device scan is started.
+  scan_started,
+  // Device scan is cancelled.
+  scan_cancelled,
+  // If the device is disabled by preference.
   disabled,
-  // Device is added, but scan for the device is canceled. The event is
-  // published after the added event.
-  scan_canceled,
   // Device is removed.
   removed,
   // Device is hard unplugged.
@@ -299,10 +297,7 @@ dictionary MountCompletedEvent {
   // Metadata of the mounted volume.
   VolumeMetadata volumeMetadata;
 
-  // Whether it is remount or not.
-  boolean isRemounting;
-
-  // Whether the volume should be opend by Files.app or not.
+  // Whether the volume event should be notified or not.
   boolean shouldNotify;
 };
 

ui/file_manager/file_manager/background/js/device_handler.js

@@ -27,22 +27,12 @@ function DeviceHandler() {
    */
   this.navigationVolumes_ = {};
 
-  /**
-   * Whether the device is just after starting up or not.
-   *
-   * @type {boolean}
-   * @private
-   */
-  this.isStartup_ = false;
-
   chrome.fileBrowserPrivate.onDeviceChanged.addListener(
       this.onDeviceChanged_.bind(this));
   chrome.fileBrowserPrivate.onMountCompleted.addListener(
       this.onMountCompleted_.bind(this));
   chrome.notifications.onButtonClicked.addListener(
       this.onNotificationButtonClicked_.bind(this));
-  chrome.runtime.onStartup.addListener(
-      this.onStartup_.bind(this));
 }
 
 DeviceHandler.prototype = {
@@ -97,13 +87,6 @@ DeviceHandler.Notification = function(prefix, title, message, opt_buttonLabel) {
    */
   this.queue_ = new AsyncUtil.Queue();
 
-  /**
-   * Timeout ID.
-   * @type {number}
-   * @private
-   */
-  this.pendingShowTimerId_ = 0;
-
   Object.seal(this);
 };
 
@@ -199,7 +182,6 @@ DeviceHandler.Notification.FORMAT_FAIL = new DeviceHandler.Notification(
  * @return {string} Notification ID.
  */
 DeviceHandler.Notification.prototype.show = function(devicePath, opt_message) {
-  this.clearTimeout_();
   var notificationId = this.makeId_(devicePath);
   this.queue_.run(function(callback) {
     var buttons =
@@ -218,21 +200,11 @@ DeviceHandler.Notification.prototype.show = function(devicePath, opt_message) {
   return notificationId;
 };
 
-/**
- * Shows the notification after 5 seconds.
- * @param {string} devicePath Device path.
- */
-DeviceHandler.Notification.prototype.showLater = function(devicePath) {
-  this.clearTimeout_();
-  this.pendingShowTimerId_ = setTimeout(this.show.bind(this, devicePath), 5000);
-};
-
 /**
  * Hides the notification for the device path.
  * @param {string} devicePath Device path.
  */
 DeviceHandler.Notification.prototype.hide = function(devicePath) {
-  this.clearTimeout_();
   this.queue_.run(function(callback) {
     chrome.notifications.clear(this.makeId_(devicePath), callback);
   }.bind(this));
@@ -248,17 +220,6 @@ DeviceHandler.Notification.prototype.makeId_ = function(devicePath) {
   return this.prefix + ':' + devicePath;
 };
 
-/**
- * Cancels the timeout request.
- * @private
- */
-DeviceHandler.Notification.prototype.clearTimeout_ = function() {
-  if (this.pendingShowTimerId_) {
-    clearTimeout(this.pendingShowTimerId_);
-    this.pendingShowTimerId_ = 0;
-  }
-};
-
 /**
  * Handles notifications from C++ sides.
  * @param {DeviceEvent} event Device event.
@@ -266,18 +227,16 @@ DeviceHandler.Notification.prototype.clearTimeout_ = function() {
  */
 DeviceHandler.prototype.onDeviceChanged_ = function(event) {
   switch (event.type) {
-    case 'added':
-      if (!this.isStartup_)
-        DeviceHandler.Notification.DEVICE.showLater(event.devicePath);
-      this.mountStatus_[event.devicePath] = DeviceHandler.MountStatus.NO_RESULT;
+    case 'scan_started':
+      DeviceHandler.Notification.DEVICE.show(event.devicePath);
+      break;
+    case 'scan_cancelled':
+      DeviceHandler.Notification.DEVICE.hide(event.devicePath);
       break;
     case 'disabled':
       DeviceHandler.Notification.DEVICE_EXTERNAL_STORAGE_DISABLED.show(
           event.devicePath);
       break;
-    case 'scan_canceled':
-      DeviceHandler.Notification.DEVICE.hide(event.devicePath);
-      break;
     case 'removed':
       DeviceHandler.Notification.DEVICE.hide(event.devicePath);
       DeviceHandler.Notification.DEVICE_FAIL.hide(event.devicePath);
@@ -286,10 +245,8 @@ DeviceHandler.prototype.onDeviceChanged_ = function(event) {
       delete this.mountStatus_[event.devicePath];
       break;
     case 'hard_unplugged':
-      if (!this.isStartup_) {
-        DeviceHandler.Notification.DEVICE_HARD_UNPLUGGED.show(
-            event.devicePath);
-      }
+      DeviceHandler.Notification.DEVICE_HARD_UNPLUGGED.show(
+          event.devicePath);
       break;
     case 'format_start':
       DeviceHandler.Notification.FORMAT_START.show(event.devicePath);
@@ -302,6 +259,9 @@ DeviceHandler.prototype.onDeviceChanged_ = function(event) {
       DeviceHandler.Notification.FORMAT_START.hide(event.devicePath);
       DeviceHandler.Notification.FORMAT_FAIL.show(event.devicePath);
       break;
+    default:
+      console.error('Unknown event tyep: ' + event.type);
+      break;
   }
 };
 
@@ -334,12 +294,13 @@ DeviceHandler.prototype.onMountCompleted_ = function(event) {
   // If this is remounting, which happens when resuming ChromeOS, the device has
   // already inserted to the computer. So we suppress the notification.
   var volume = event.volumeMetadata;
-  if (!volume.deviceType || event.isRemounting)
+  if (!volume.deviceType || !event.shouldNotify)
     return;
 
   // If the current volume status is succeed and it should be handled in
   // Files.app, show the notification to navigate the volume.
-  if (event.status === 'success' && event.shouldNotify) {
+  if (event.eventType === 'mount' &&
+      event.status === 'success') {
     if (this.navigationVolumes_[event.volumeMetadata.devicePath]) {
       // The notification has already shown for the device. It seems the device
       // has multiple volumes. The order of mount events of volumes are
@@ -352,12 +313,10 @@ DeviceHandler.prototype.onMountCompleted_ = function(event) {
             event.volumeMetadata.volumeId;
       }
     } else {
-      if (!this.isStartup_) {
-        this.navigationVolumes_[event.volumeMetadata.devicePath] =
-            event.volumeMetadata.volumeId;
-        DeviceHandler.Notification.DEVICE_NAVIGATION.show(
-            event.volumeMetadata.devicePath);
-      }
+      this.navigationVolumes_[event.volumeMetadata.devicePath] =
+          event.volumeMetadata.volumeId;
+      DeviceHandler.Notification.DEVICE_NAVIGATION.show(
+          event.volumeMetadata.devicePath);
     }
   } else if (event.status === 'error_unknown_filesystem') {
     // The volume id is necessary to navigate when users click start
@@ -381,10 +340,9 @@ DeviceHandler.prototype.onMountCompleted_ = function(event) {
   };
 
   // Update the current status.
+  if (!this.mountStatus_[volume.devicePath])
+    this.mountStatus_[volume.devicePath] = DeviceHandler.MountStatus.NO_RESULT;
   switch (this.mountStatus_[volume.devicePath]) {
-    // If there is no related device, do nothing.
-    case undefined:
-      return;
     // If the multipart error message has already shown, do nothing because the
     // message does not changed by the following mount results.
     case DeviceHandler.MountStatus.MULTIPART_ERROR:
@@ -469,10 +427,3 @@ DeviceHandler.prototype.onNotificationButtonClicked_ = function(id) {
     this.dispatchEvent(event);
   }
 };
-
-DeviceHandler.prototype.onStartup_ = function() {
-  this.isStartup_ = true;
-  setTimeout(function() {
-    this.isStartup_ = false;
-  }.bind(this), 5000);
-};