Commit 955a6d52 authored by dewittj's avatar dewittj Committed by Commit bot

[Offline Pages] Check for the appropriate namespace when showing notifications.

Download notifications should only be shown for pages in the namespaces that
will be shown in the Download Home.

BUG=642019

Review-Url: https://codereview.chromium.org/2295593002
Cr-Commit-Position: refs/heads/master@{#415444}
parent e6124d0e
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "components/offline_pages/background/request_coordinator.h" #include "components/offline_pages/background/request_coordinator.h"
#include "components/offline_pages/background/save_page_request.h" #include "components/offline_pages/background/save_page_request.h"
#include "components/offline_pages/downloads/download_ui_adapter.h"
#include "components/offline_pages/downloads/offline_page_download_notifier.h" #include "components/offline_pages/downloads/offline_page_download_notifier.h"
namespace offline_pages { namespace offline_pages {
...@@ -42,11 +43,15 @@ void DownloadNotifyingObserver::CreateAndStartObserving( ...@@ -42,11 +43,15 @@ void DownloadNotifyingObserver::CreateAndStartObserving(
void DownloadNotifyingObserver::OnAdded(const SavePageRequest& request) { void DownloadNotifyingObserver::OnAdded(const SavePageRequest& request) {
DCHECK(notifier_.get()); DCHECK(notifier_.get());
if (!DownloadUIAdapter::IsVisibleInUI(request.client_id()))
return;
notifier_->NotifyDownloadProgress(DownloadUIItem(request)); notifier_->NotifyDownloadProgress(DownloadUIItem(request));
} }
void DownloadNotifyingObserver::OnChanged(const SavePageRequest& request) { void DownloadNotifyingObserver::OnChanged(const SavePageRequest& request) {
DCHECK(notifier_.get()); DCHECK(notifier_.get());
if (!DownloadUIAdapter::IsVisibleInUI(request.client_id()))
return;
if (request.request_state() == SavePageRequest::RequestState::PAUSED) if (request.request_state() == SavePageRequest::RequestState::PAUSED)
notifier_->NotifyDownloadPaused(DownloadUIItem(request)); notifier_->NotifyDownloadPaused(DownloadUIItem(request));
else else
...@@ -57,6 +62,8 @@ void DownloadNotifyingObserver::OnCompleted( ...@@ -57,6 +62,8 @@ void DownloadNotifyingObserver::OnCompleted(
const SavePageRequest& request, const SavePageRequest& request,
RequestCoordinator::SavePageStatus status) { RequestCoordinator::SavePageStatus status) {
DCHECK(notifier_.get()); DCHECK(notifier_.get());
if (!DownloadUIAdapter::IsVisibleInUI(request.client_id()))
return;
if (status == RequestCoordinator::SavePageStatus::SUCCESS) if (status == RequestCoordinator::SavePageStatus::SUCCESS)
notifier_->NotifyDownloadSuccessful(DownloadUIItem(request)); notifier_->NotifyDownloadSuccessful(DownloadUIItem(request));
else if (status == RequestCoordinator::SavePageStatus::REMOVED) else if (status == RequestCoordinator::SavePageStatus::REMOVED)
......
...@@ -228,4 +228,18 @@ TEST_F(DownloadNotifyingObserverTest, OnCompletedFailure) { ...@@ -228,4 +228,18 @@ TEST_F(DownloadNotifyingObserverTest, OnCompletedFailure) {
EXPECT_EQ(kTestCreationTime, notifier()->download_item()->start_time); EXPECT_EQ(kTestCreationTime, notifier()->download_item()->start_time);
} }
TEST_F(DownloadNotifyingObserverTest, NamespacesNotVisibleInUI) {
std::vector<std::string> name_spaces = {
kBookmarkNamespace, kLastNNamespace, kCCTNamespace,
kNTPSuggestionsNamespace, kDefaultNamespace};
for (auto name_space : name_spaces) {
ClientId invisible_client_id(name_space, kTestGuid);
SavePageRequest request(kTestOfflineId, GURL(kTestUrl), invisible_client_id,
kTestCreationTime, kTestUserRequested);
observer()->OnAdded(request);
EXPECT_EQ(LastNotificationType::NONE, notifier()->last_notification_type());
}
}
} // namespace offline_pages } // namespace offline_pages
...@@ -210,6 +210,7 @@ void DownloadUIAdapter::OnDeletePagesDone(DeletePageResult result) { ...@@ -210,6 +210,7 @@ void DownloadUIAdapter::OnDeletePagesDone(DeletePageResult result) {
// TODO(dimich): Consider adding UMA to record user actions. // TODO(dimich): Consider adding UMA to record user actions.
} }
// static
bool DownloadUIAdapter::IsVisibleInUI(const ClientId& client_id) { bool DownloadUIAdapter::IsVisibleInUI(const ClientId& client_id) {
const std::string& name_space = client_id.name_space; const std::string& name_space = client_id.name_space;
return (name_space == kAsyncNamespace || name_space == kDownloadNamespace) && return (name_space == kAsyncNamespace || name_space == kDownloadNamespace) &&
......
...@@ -56,6 +56,10 @@ class DownloadUIAdapter : public OfflinePageModel::Observer, ...@@ -56,6 +56,10 @@ class DownloadUIAdapter : public OfflinePageModel::Observer,
static DownloadUIAdapter* FromOfflinePageModel( static DownloadUIAdapter* FromOfflinePageModel(
OfflinePageModel* offline_page_model); OfflinePageModel* offline_page_model);
// Checks a client ID for proper namespace and ID format to be shown in the
// Downloads Home UI.
static bool IsVisibleInUI(const ClientId& page);
// This adapter is potentially shared by UI elements, each of which adds // This adapter is potentially shared by UI elements, each of which adds
// itself as an observer. // itself as an observer.
// When the last observer si removed, cached list of items is destroyed and // When the last observer si removed, cached list of items is destroyed and
...@@ -111,8 +115,6 @@ class DownloadUIAdapter : public OfflinePageModel::Observer, ...@@ -111,8 +115,6 @@ class DownloadUIAdapter : public OfflinePageModel::Observer,
void OnOfflinePagesChanged(const MultipleOfflinePageItemResult& pages); void OnOfflinePagesChanged(const MultipleOfflinePageItemResult& pages);
void OnDeletePagesDone(DeletePageResult result); void OnDeletePagesDone(DeletePageResult result);
bool IsVisibleInUI(const ClientId& page);
// Always valid, this class is a member of the model. // Always valid, this class is a member of the model.
OfflinePageModel* model_; OfflinePageModel* model_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment