New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 839175 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Improve DownloadUIAdapter

Project Member Reported by romax@chromium.org, May 3 2018

Issue description

DownloadUIAdapter should merge with OfflinePageNotificationBridge since they're duplicating notifications.

Also we've been planning to remove the cache in DownloadUIAdapter for a while, this may be achieved as well as part of the refactoring.
 

Comment 1 by romax@chromium.org, May 3 2018

Cc: romax@chromium.org petewil@chromium.org
 Issue 836989  has been merged into this issue.
Labels: offline-pages-p2p

Comment 3 by romax@chromium.org, May 21 2018

Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c53e06b34d1a731a9d0ff90d352176f237020b52

commit c53e06b34d1a731a9d0ff90d352176f237020b52
Author: Yafei Duan <romax@chromium.org>
Date: Wed Jun 06 00:03:27 2018

[Offline Pages] Refactor DownloadUIAdapter and OfflinePageDownloadBridge

This CL removed OfflinePageDownloadBridge and related code, integrated
them into DownloadUIAdapter/OfflinePageDownloadBridge, in order to unify
the notification/UI pipeline.

This CL removed the cache and its loading state in DownloadUIAdapter,
replacing the APIs with asynchronous calls down to lower level instead
of querying preloaded cache.

With the CL, DownloadUIAdapter will be registered as observer to both
OfflinePageModel and RequestCoordinator when OCANBUi is created. And
sending notification of page/request added/deleted whenever it receives
the event. Since successful request completion turns into successful
page save, OfflinePageAdded and OnAdded (with successful status) will
be merged if necessary, then notify OfflineContentProvider.

Minor details:
- Removed OfflinePageNotificationBridge.
- Removed OfflinePageDownloadNotifier and DownloadNotifyingObserver.
- Integrated some of the functions into DownloadUIAdapter and
  OfflinePageDownloadBridge.
- Changed some UI treatment based on newly used OfflineItem attributes.
- Changed related tests.

Bug:  839175 
Change-Id: Icbe763bf70b3f49d1db6717a3b7c4ba57c25fb71
Reviewed-on: https://chromium-review.googlesource.com/1068063
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Yafei Duan <romax@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564709}
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService2.java
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/chrome/android/java/src/org/chromium/chrome/browser/download/SystemDownloadNotifier2.java
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUi.java
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/chrome/android/java/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUiFactory.java
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadBridge.java
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/chrome/android/java_sources.gni
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/chrome/android/junit/src/org/chromium/chrome/browser/download/items/OfflineContentAggregatorNotificationBridgeUiTest.java
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/chrome/browser/BUILD.gn
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/chrome/browser/offline_pages/android/downloads/offline_page_download_bridge.cc
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/chrome/browser/offline_pages/android/downloads/offline_page_download_bridge.h
[delete] https://crrev.com/22387fe1750c9716f7fa48846263d7c9eff3f2c9/chrome/browser/offline_pages/android/downloads/offline_page_notification_bridge.cc
[delete] https://crrev.com/22387fe1750c9716f7fa48846263d7c9eff3f2c9/chrome/browser/offline_pages/android/downloads/offline_page_notification_bridge.h
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/chrome/browser/offline_pages/android/evaluation/offline_page_evaluation_bridge.cc
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/chrome/browser/offline_pages/android/offline_page_utils_android.cc
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/chrome/browser/offline_pages/android/request_coordinator_factory.cc
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/components/offline_pages/core/downloads/BUILD.gn
[delete] https://crrev.com/22387fe1750c9716f7fa48846263d7c9eff3f2c9/components/offline_pages/core/downloads/download_notifying_observer.cc
[delete] https://crrev.com/22387fe1750c9716f7fa48846263d7c9eff3f2c9/components/offline_pages/core/downloads/download_notifying_observer.h
[delete] https://crrev.com/22387fe1750c9716f7fa48846263d7c9eff3f2c9/components/offline_pages/core/downloads/download_notifying_observer_unittest.cc
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/components/offline_pages/core/downloads/download_ui_adapter.cc
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/components/offline_pages/core/downloads/download_ui_adapter.h
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/components/offline_pages/core/downloads/offline_item_conversions.cc
[modify] https://crrev.com/c53e06b34d1a731a9d0ff90d352176f237020b52/components/offline_pages/core/downloads/offline_item_conversions_unittest.cc
[delete] https://crrev.com/22387fe1750c9716f7fa48846263d7c9eff3f2c9/components/offline_pages/core/downloads/offline_page_download_notifier.h

Comment 5 by romax@chromium.org, Jun 6 2018

Status: Fixed (was: Started)

Sign in to add a comment