Download shelf autohides on showing in shell, just same as regular open

chrome/browser/download/download_item_model.cc

Index: chrome/browser/download/download_item_model.cc
diff --git a/chrome/browser/download/download_item_model.cc b/chrome/browser/download/download_item_model.cc
index 171d3b00dd6d990ef0dc754e15ae5cfa66250539..0cd0e3e9aebfe2d6f7abc5abcf0361d97d84c535 100644
--- a/chrome/browser/download/download_item_model.cc
+++ b/chrome/browser/download/download_item_model.cc
@@ -38,7 +38,8 @@ namespace {
 // Per DownloadItem data used by DownloadItemModel. The model doesn't keep any
 // state since there could be multiple models associated with a single
 // DownloadItem, and the lifetime of the model is shorter than the DownloadItem.
-class DownloadItemModelData : public base::SupportsUserData::Data {
+class DownloadItemModelData : public base::SupportsUserData::Data,
+                              public DownloadItem::Observer {

[asanka, 2014/06/12 21:08:47]

Let's try to avoid having the DownloadItemModelData observe the DownloadItem.
Having the DownloadItemModel change state based on observing the DownloadItem is
a bit dangerous because the order in which observers are invoked is undefined.
So another observer that looks at DownloadItemModel during
OnDownloadOpened/Shown would see a different state based on whether it's invoked
before or after the DownloadItemModel's Downloaditem Observer.

Both Open and ShowInFolder actions are ultimately carried out by
ChromeDownloadManagerDelegate. The methods you are looking for are
OpenDownload() and ShowDownloadInShell(). You can have those methods call
DownloadItemModel(download_item).SetUserActed(true) as necessary. The
ShowDownloadInShell() method can also check if the show in folder was invoked
after the download was complete or not.

Pay special attention to the order of events when a download is opened or shown
in the shelf. Currently the obsevers are invoked before the delegate
(ChromeDownloadManagerDelegate). If necessary we can switch the order around as
long as we can verify that it won't break any assumptions.

  public:
   // Get the DownloadItemModelData object for |download|. Returns NULL if
   // there's no model data.
@@ -65,12 +66,27 @@ class DownloadItemModelData : public base::SupportsUserData::Data {
     should_prefer_opening_in_browser_ = preference;
   }
 
+  bool user_acted() const {
+    return user_acted_;
+  }
+  void set_user_acted(bool acted) {
+    user_acted_ = acted;
+  }
+
  private:
-  DownloadItemModelData();
-  virtual ~DownloadItemModelData() {}
+  virtual void OnDownloadOpened(DownloadItem* download) OVERRIDE;
+  virtual void OnDownloadShown(DownloadItem* download) OVERRIDE;
+  virtual void OnDownloadDestroyed(DownloadItem* download) OVERRIDE;
+
+ private:
+  explicit DownloadItemModelData(DownloadItem* download);
+  virtual ~DownloadItemModelData();
 
   static const char kKey[];
 
+  // Download item that holds this data.
+  DownloadItem* download_;
+
   // Whether the download should be displayed in the download shelf. True by
   // default.
   bool should_show_in_shelf_;
@@ -81,6 +97,9 @@ class DownloadItemModelData : public base::SupportsUserData::Data {
   // Whether the download should be opened in the browser vs. the system handler
   // for the file type.
   bool should_prefer_opening_in_browser_;
+
+  // Whether user acted on this item, so that download shelf can autohide.
+  bool user_acted_;
 };
 
 // static
@@ -98,16 +117,53 @@ DownloadItemModelData* DownloadItemModelData::GetOrCreate(
   DownloadItemModelData* data =
       static_cast<DownloadItemModelData*>(download->GetUserData(kKey));
   if (data == NULL) {
-    data = new DownloadItemModelData();
+    data = new DownloadItemModelData(download);
     download->SetUserData(kKey, data);
   }
   return data;
 }
 
-DownloadItemModelData::DownloadItemModelData()
-    : should_show_in_shelf_(true),
+DownloadItemModelData::DownloadItemModelData(DownloadItem* download)
+    : download_(download),
+      should_show_in_shelf_(true),
       was_ui_notified_(false),
-      should_prefer_opening_in_browser_(false) {
+      should_prefer_opening_in_browser_(false),
+      user_acted_(false) {
+  DCHECK(download);
+
+  // As |download| might be loaded from history, update our own state.
+  user_acted_ = download->GetOpened();
+  download_->AddObserver(this);
+}
+
+DownloadItemModelData::~DownloadItemModelData() {
+  if (download_)
+    download_->RemoveObserver(this);
+}
+
+void DownloadItemModelData::OnDownloadOpened(DownloadItem* download) {
+  DCHECK_EQ(download_, download);
+  user_acted_ = true;
+}
+
+void DownloadItemModelData::OnDownloadShown(DownloadItem* download) {
+  DCHECK_EQ(download_, download);
+
+  // The user can show download in shell even when it is not yet completed,
+  // as this flag is used to 'hide' the download item from user's sight, it is
+  // fair to do so only for completed downloads.
+  if (download->GetState() == DownloadItem::COMPLETE) {
+    user_acted_ = true;
+  }
+}
+
+void DownloadItemModelData::OnDownloadDestroyed(DownloadItem* download) {
+  DCHECK_EQ(download_, download);
+
+  // This is called from DownloadItem's destructor - so that
+  // base::SupportsUserData would destroy this model.
+  download_->RemoveObserver(this);
+  download_ = NULL;
 }
 
 base::string16 InterruptReasonStatusMessage(int reason) {
@@ -690,3 +746,13 @@ void DownloadItemModel::OpenUsingPlatformHandler() {
   delegate->OpenDownloadUsingPlatformHandler(download_);
   RecordDownloadOpenMethod(DOWNLOAD_OPEN_METHOD_USER_PLATFORM);
 }
+
+void DownloadItemModel::SetUserActed(bool acted) {
+  DownloadItemModelData* data = DownloadItemModelData::GetOrCreate(download_);
+  data->set_user_acted(acted);
+}
+
+bool DownloadItemModel::GetUserActed() const {
+  DownloadItemModelData* data = DownloadItemModelData::GetOrCreate(download_);
+  return data->user_acted();
+}