Revert "More OfflineItemModel implementation"

This reverts commit 16355cba.

Reason for revert:
This seems to cause an ASAN failure on the CI. The error logs are:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/29069
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8934902171469785040/+/steps/interactive_ui_tests/0/logs/DownloadNotificationTest.IncognitoDownloadFile/0

Original change's description:
> More OfflineItemModel implementation
> 
> This CL makes OfflineItemModel inherit from DownloadUIModel
> 
> Bug: 881499
> Change-Id: I63270a79917bae87fdb4ae3531f9088f7900509e
> Reviewed-on: https://chromium-review.googlesource.com/1225888
> Commit-Queue: Min Qin <qinmin@chromium.org>
> Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#592645}

TBR=qinmin@chromium.org,shaktisahu@chromium.org

Change-Id: Ifdae8934bae3cf137fd2e4729f076f8df577241d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 881499
Reviewed-on: https://chromium-review.googlesource.com/1234099Reviewed-by: Taiju Tsuiki <tzik@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592691}

chrome/browser/download/download_item_model.cc

@@ -303,14 +303,9 @@ base::string16 InterruptReasonMessage(
 // DownloadItemModel
 
 DownloadItemModel::DownloadItemModel(DownloadItem* download)
-    : download_(download) {
-  download_->AddObserver(this);
-}
+    : download_(download) {}
 
-DownloadItemModel::~DownloadItemModel() {
-  if (download_)
-    download_->RemoveObserver(this);
-}
+DownloadItemModel::~DownloadItemModel() {}
 
 ContentId DownloadItemModel::GetContentId() const {
   bool off_the_record = content::DownloadItemUtils::GetBrowserContext(download_)
@@ -319,6 +314,16 @@ ContentId DownloadItemModel::GetContentId() const {
                    download_->GetGuid());
 }
 
+void DownloadItemModel::AddObserver(DownloadUIModel::Observer* observer) {
+  DownloadUIModel::AddObserver(observer);
+  download_->AddObserver(this);
+}
+
+void DownloadItemModel::RemoveObserver(DownloadUIModel::Observer* observer) {
+  DownloadUIModel::RemoveObserver(observer);
+  download_->RemoveObserver(this);
+}
+
 base::string16 DownloadItemModel::GetInterruptReasonText() const {
   if (download_->GetState() != DownloadItem::INTERRUPTED ||
       download_->GetLastReason() ==
@@ -827,7 +832,6 @@ void DownloadItemModel::OnDownloadOpened(DownloadItem* download) {
 void DownloadItemModel::OnDownloadDestroyed(DownloadItem* download) {
   for (auto& obs : observers_)
     obs.OnDownloadDestroyed();
-  download_ = nullptr;
 }
 
 base::string16 DownloadItemModel::GetInProgressStatusString() const {

chrome/browser/download/download_item_model.h

@@ -31,6 +31,8 @@ class DownloadItemModel : public DownloadUIModel,
   ~DownloadItemModel() override;
 
   // DownloadUIModel implementation.
+  void AddObserver(DownloadUIModel::Observer* observer) override;
+  void RemoveObserver(DownloadUIModel::Observer* observer) override;
   ContentId GetContentId() const override;
   base::string16 GetInterruptReasonText() const override;
   base::string16 GetStatusText() const override;

chrome/browser/download/download_ui_model.h

@@ -40,8 +40,8 @@ class DownloadUIModel {
     virtual ~Observer() {}
   };
 
-  void AddObserver(Observer* observer);
-  void RemoveObserver(Observer* observer);
+  virtual void AddObserver(Observer* observer);
+  virtual void RemoveObserver(Observer* observer);
 
   // Returns the content id associated with this download.
   virtual ContentId GetContentId() const;

chrome/browser/download/offline_item_model.cc

@@ -5,38 +5,21 @@
 #include "chrome/browser/download/offline_item_model.h"
 
 #include "chrome/browser/download/offline_item_model_manager.h"
-#include "chrome/browser/offline_items_collection/offline_content_aggregator_factory.h"
-#include "components/offline_items_collection/core/offline_content_aggregator.h"
 
-using offline_items_collection::ContentId;
-using offline_items_collection::OfflineItem;
+OfflineItemModel::OfflineItemModel(
+    OfflineItemModelManager* manager,
+    const offline_items_collection::OfflineItem& offline_item)
+    : manager_(manager), offline_item_(offline_item) {}
 
-OfflineItemModel::OfflineItemModel(OfflineItemModelManager* manager,
-                                   const OfflineItem& offline_item)
-    : manager_(manager),
-      offline_item_(std::make_unique<OfflineItem>(offline_item)) {
-  offline_items_collection::OfflineContentAggregator* aggregator =
-      OfflineContentAggregatorFactory::GetForBrowserContext(
-          manager_->browser_context());
-  offline_item_observer_ =
-      std::make_unique<FilteredOfflineItemObserver>(aggregator);
-  offline_item_observer_->AddObserver(offline_item_->id, this);
-}
-
-OfflineItemModel::~OfflineItemModel() {
-  if (offline_item_)
-    offline_item_observer_->RemoveObserver(offline_item_->id, this);
-}
+OfflineItemModel::~OfflineItemModel() = default;
 
 int64_t OfflineItemModel::GetCompletedBytes() const {
-  return offline_item_ ? offline_item_->received_bytes : 0;
+  return offline_item_.received_bytes;
 }
 
 int64_t OfflineItemModel::GetTotalBytes() const {
-  if (!offline_item_)
-    return 0;
-  return offline_item_->total_size_bytes > 0 ? offline_item_->total_size_bytes
-                                             : 0;
+  return offline_item_.total_size_bytes > 0 ? offline_item_.total_size_bytes
+                                            : 0;
 }
 
 int OfflineItemModel::PercentComplete() const {
@@ -48,24 +31,12 @@ int OfflineItemModel::PercentComplete() const {
 
 bool OfflineItemModel::WasUINotified() const {
   const OfflineItemModelData* data =
-      manager_->GetOrCreateOfflineItemModelData(offline_item_->id);
+      manager_->GetOrCreateOfflineItemModelData(offline_item_.id);
   return data->was_ui_notified_;
 }
 
 void OfflineItemModel::SetWasUINotified(bool was_ui_notified) {
   OfflineItemModelData* data =
-      manager_->GetOrCreateOfflineItemModelData(offline_item_->id);
+      manager_->GetOrCreateOfflineItemModelData(offline_item_.id);
   data->was_ui_notified_ = was_ui_notified;
 }
-
-void OfflineItemModel::OnItemRemoved(const ContentId& id) {
-  for (auto& obs : observers_)
-    obs.OnDownloadDestroyed();
-  offline_item_.reset();
-}
-
-void OfflineItemModel::OnItemUpdated(const OfflineItem& item) {
-  offline_item_ = std::make_unique<OfflineItem>(item);
-  for (auto& obs : observers_)
-    obs.OnDownloadUpdated();
-}

chrome/browser/download/offline_item_model.h

@@ -5,19 +5,13 @@
 #ifndef CHROME_BROWSER_DOWNLOAD_OFFLINE_ITEM_MODEL_H_
 #define CHROME_BROWSER_DOWNLOAD_OFFLINE_ITEM_MODEL_H_
 
-#include <memory>
-
 #include "chrome/browser/download/download_ui_model.h"
-#include "components/offline_items_collection/core/filtered_offline_item_observer.h"
 #include "components/offline_items_collection/core/offline_item.h"
 
 class OfflineItemModelManager;
 
-using offline_items_collection::FilteredOfflineItemObserver;
-
 // Implementation of DownloadUIModel that wrappers around a |OfflineItem|.
-class OfflineItemModel : public DownloadUIModel,
-                         public FilteredOfflineItemObserver::Observer {
+class OfflineItemModel : public DownloadUIModel {
  public:
   // Constructs a OfflineItemModel.
   OfflineItemModel(OfflineItemModelManager* manager,
@@ -32,15 +26,9 @@ class OfflineItemModel : public DownloadUIModel,
   void SetWasUINotified(bool should_notify) override;
 
  private:
-  // FilteredOfflineItemObserver::Observer overrides.
-  void OnItemRemoved(const offline_items_collection::ContentId& id) override;
-  void OnItemUpdated(
-      const offline_items_collection::OfflineItem& item) override;
-
   OfflineItemModelManager* manager_;
 
-  std::unique_ptr<FilteredOfflineItemObserver> offline_item_observer_;
-  std::unique_ptr<offline_items_collection::OfflineItem> offline_item_;
+  offline_items_collection::OfflineItem offline_item_;
 
   DISALLOW_COPY_AND_ASSIGN(OfflineItemModel);
 };

chrome/browser/download/offline_item_model_manager.cc

@@ -4,9 +4,7 @@
 
 #include "chrome/browser/download/offline_item_model_manager.h"
 
-OfflineItemModelManager::OfflineItemModelManager(
-    content::BrowserContext* browser_context)
-    : browser_context_(browser_context) {}
+OfflineItemModelManager::OfflineItemModelManager() = default;
 
 OfflineItemModelManager::~OfflineItemModelManager() = default;
 

chrome/browser/download/offline_item_model_manager.h

@@ -11,17 +11,13 @@
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/offline_items_collection/core/offline_item.h"
 
-namespace content {
-class BrowserContext;
-}  // namespace content
-
 using ContentId = offline_items_collection::ContentId;
 
 // Class for managing all the OfflineModels for a profile.
 class OfflineItemModelManager : public KeyedService {
  public:
   // Constructs a OfflineItemModelManager.
-  explicit OfflineItemModelManager(content::BrowserContext* browser_context);
+  OfflineItemModelManager();
   ~OfflineItemModelManager() override;
 
   // Returns the OfflineItemModel for the ContentId, if not found, an empty
@@ -30,10 +26,7 @@ class OfflineItemModelManager : public KeyedService {
 
   void RemoveOfflineItemModelData(const ContentId& id);
 
-  content::BrowserContext* browser_context() { return browser_context_; }
-
  private:
-  content::BrowserContext* browser_context_;
   std::map<ContentId, std::unique_ptr<OfflineItemModelData>>
       offline_item_model_data_;
 

chrome/browser/download/offline_item_model_manager_factory.cc

@@ -30,5 +30,5 @@ OfflineItemModelManagerFactory::~OfflineItemModelManagerFactory() = default;
 
 KeyedService* OfflineItemModelManagerFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
-  return new OfflineItemModelManager(context);
+  return new OfflineItemModelManager();
 }

chrome/browser/ui/views/download/download_item_view.cc

@@ -179,7 +179,6 @@ DownloadItemView::DownloadItemView(std::unique_ptr<DownloadUIModel> download,
 DownloadItemView::~DownloadItemView() {
   StopDownloadProgress();
   model_->RemoveObserver(this);
-  base::ThreadTaskRunnerHandle ::Get()->DeleteSoon(FROM_HERE, model_.release());
 }
 
 // Progress animation handlers.