Index: chrome/browser/ui/webui/downloads_dom_handler.cc |
diff --git a/chrome/browser/ui/webui/downloads_dom_handler.cc b/chrome/browser/ui/webui/downloads_dom_handler.cc |
index 1a1dbc90aafa9cb331b7f04107dcc4364e0b0920..48784f3027559c3284e29b8e69bc8094cec1ada5 100644 |
--- a/chrome/browser/ui/webui/downloads_dom_handler.cc |
+++ b/chrome/browser/ui/webui/downloads_dom_handler.cc |
@@ -12,6 +12,7 @@ |
#include "base/bind_helpers.h" |
#include "base/i18n/rtl.h" |
#include "base/i18n/time_formatting.h" |
+#include "base/logging.h" |
#include "base/memory/singleton.h" |
#include "base/metrics/field_trial.h" |
#include "base/metrics/histogram.h" |
@@ -199,7 +200,7 @@ base::DictionaryValue* CreateDownloadItemValue( |
download_model.GetTabProgressStatusText()); |
file_value->SetInteger("percent", |
- static_cast<int>(download_item->PercentComplete())); |
+ std::max(0, static_cast<int>(download_item->PercentComplete()))); |
file_value->SetInteger("received", |
static_cast<int>(download_item->GetReceivedBytes())); |
break; |
@@ -322,35 +323,49 @@ void DownloadsDOMHandler::RegisterMessages() { |
void DownloadsDOMHandler::OnDownloadCreated( |
content::DownloadManager* manager, content::DownloadItem* download_item) { |
- if (IsDownloadDisplayable(*download_item)) |
- ScheduleSendCurrentDownloads(); |
+ if (!IsDownloadDisplayable(*download_item)) |
+ new_downloads_.insert(download_item->GetId()); |
asanka
2015/03/10 04:15:51
If OnDownloadCreated() is called for a download re
Dan Beam
2015/03/10 05:41:02
added back the original code
|
} |
void DownloadsDOMHandler::OnDownloadUpdated( |
content::DownloadManager* manager, |
content::DownloadItem* download_item) { |
- if (IsDownloadDisplayable(*download_item)) { |
- if (search_terms_ && !search_terms_->empty()) { |
- // Don't CallDownloadUpdated() if download_item doesn't match |
- // search_terms_. |
- // TODO(benjhayden): Consider splitting MatchesQuery() out to a function. |
- content::DownloadManager::DownloadVector all_items, filtered_items; |
- all_items.push_back(download_item); |
- DownloadQuery query; |
- query.AddFilter(DownloadQuery::FILTER_QUERY, *search_terms_.get()); |
- query.Search(all_items.begin(), all_items.end(), &filtered_items); |
- if (filtered_items.empty()) |
- return; |
+ const uint32 download_id = download_item->GetId(); |
+ if (new_downloads_.count(download_id)) { |
+ // New downloads must be sent before they can be updated. |
+ if (!IsDownloadDisplayable(*download_item)) { |
+ // Item isn't ready to be displayed yet. Wait until it is. |
+ return; |
} |
- base::ListValue results_value; |
- results_value.Append(CreateDownloadItemValue( |
- download_item, |
- (original_notifier_.get() && |
- (manager == main_notifier_.GetManager())))); |
- CallDownloadUpdated(results_value); |
- } else { |
+ // Send rather than schedule as this must happen before |CallUpdateItem|. |
+ SendCurrentDownloads(); |
asanka
2015/03/10 04:15:51
If we go this route, then a subsequent CallUpdateI
Dan Beam
2015/03/10 05:41:03
Done.
|
+ new_downloads_.erase(download_id); |
+ } |
+ |
+ if (DownloadItemModel(download_item).IsBeingRevived() || |
+ !IsDownloadDisplayable(*download_item)) { |
asanka
2015/03/10 04:15:51
IsDownloadDisplayable() could be returning false f
Dan Beam
2015/03/10 05:41:03
this matches what the current code does
asanka
2015/03/11 19:57:49
True, but it's still going to be quite expensive t
|
+ // A download will be shown or hidden by this update. Resend the list. |
ScheduleSendCurrentDownloads(); |
+ return; |
} |
+ |
+ if (search_terms_ && !search_terms_->empty()) { |
+ // Don't CallUpdateItem() if download_item doesn't match |
+ // search_terms_. |
+ // TODO(benjhayden): Consider splitting MatchesQuery() out to a function. |
+ content::DownloadManager::DownloadVector all_items, filtered_items; |
+ all_items.push_back(download_item); |
+ DownloadQuery query; |
+ query.AddFilter(DownloadQuery::FILTER_QUERY, *search_terms_); |
+ query.Search(all_items.begin(), all_items.end(), &filtered_items); |
+ if (filtered_items.empty()) |
+ return; |
+ } |
+ |
+ scoped_ptr<base::DictionaryValue> item(CreateDownloadItemValue( |
+ download_item, |
+ original_notifier_ && manager == main_notifier_.GetManager())); |
+ CallUpdateItem(*item); |
} |
void DownloadsDOMHandler::OnDownloadRemoved( |
@@ -374,7 +389,7 @@ void DownloadsDOMHandler::OnDownloadRemoved( |
void DownloadsDOMHandler::HandleGetDownloads(const base::ListValue* args) { |
CountDownloadsDOMEvents(DOWNLOADS_DOM_EVENT_GET_DOWNLOADS); |
- search_terms_.reset((args && !args->empty()) ? args->DeepCopy() : NULL); |
+ search_terms_.reset(args && !args->empty() ? args->DeepCopy() : NULL); |
SendCurrentDownloads(); |
} |
@@ -471,8 +486,14 @@ void DownloadsDOMHandler::HandleUndo(const base::ListValue* args) { |
content::DownloadItem* download = GetDownloadById(id); |
if (!download) |
continue; |
- DownloadItemModel(download).SetShouldShowInShelf(true); |
+ |
+ DownloadItemModel model(download); |
+ model.SetShouldShowInShelf(true); |
asanka
2015/03/10 04:15:51
Why was this necessary?
Dan Beam
2015/03/10 05:41:02
DownloadsDOMHandler::Handle{Remove,ClearAll}() run
|
+ model.SetIsBeingRevived(true); |
asanka
2015/03/10 04:15:51
It seems the only purpose of SetIsBeingRevived() i
Dan Beam
2015/03/10 05:41:02
that would work with only 1 handler.
ALL handlers
|
+ |
download->UpdateObservers(); |
+ |
+ model.SetIsBeingRevived(false); |
} |
} |
@@ -484,8 +505,10 @@ void DownloadsDOMHandler::HandleCancel(const base::ListValue* args) { |
} |
void DownloadsDOMHandler::HandleClearAll(const base::ListValue* args) { |
- if (!IsDeletingHistoryAllowed()) |
+ if (!IsDeletingHistoryAllowed()) { |
+ // This should only be reached during tests. |
return; |
+ } |
CountDownloadsDOMEvents(DOWNLOADS_DOM_EVENT_CLEAR_ALL); |
@@ -500,6 +523,7 @@ void DownloadsDOMHandler::HandleClearAll(const base::ListValue* args) { |
void DownloadsDOMHandler::RemoveDownloads( |
const std::vector<content::DownloadItem*>& to_remove) { |
std::set<uint32> ids; |
+ |
for (auto* download : to_remove) { |
DownloadItemModel item_model(download); |
if (!item_model.ShouldShowInShelf() || |
@@ -511,6 +535,7 @@ void DownloadsDOMHandler::RemoveDownloads( |
ids.insert(download->GetId()); |
download->UpdateObservers(); |
} |
+ |
if (!ids.empty()) |
removals_.push_back(ids); |
} |
@@ -534,7 +559,9 @@ void DownloadsDOMHandler::ScheduleSendCurrentDownloads() { |
// in a single UI message loop iteration when the user Clears All downloads. |
if (update_scheduled_) |
return; |
+ |
update_scheduled_ = true; |
+ |
BrowserThread::PostTask( |
BrowserThread::UI, FROM_HERE, |
base::Bind(&DownloadsDOMHandler::SendCurrentDownloads, |
@@ -560,34 +587,33 @@ void DownloadsDOMHandler::FinalizeRemovals() { |
void DownloadsDOMHandler::SendCurrentDownloads() { |
update_scheduled_ = false; |
+ |
content::DownloadManager::DownloadVector all_items, filtered_items; |
if (main_notifier_.GetManager()) { |
main_notifier_.GetManager()->GetAllDownloads(&all_items); |
main_notifier_.GetManager()->CheckForHistoryFilesRemoval(); |
} |
- if (original_notifier_.get() && original_notifier_->GetManager()) { |
+ if (original_notifier_ && original_notifier_->GetManager()) { |
original_notifier_->GetManager()->GetAllDownloads(&all_items); |
original_notifier_->GetManager()->CheckForHistoryFilesRemoval(); |
} |
+ |
DownloadQuery query; |
- if (search_terms_ && !search_terms_->empty()) { |
- query.AddFilter(DownloadQuery::FILTER_QUERY, *search_terms_.get()); |
- } |
+ if (search_terms_ && !search_terms_->empty()) |
+ query.AddFilter(DownloadQuery::FILTER_QUERY, *search_terms_); |
query.AddFilter(base::Bind(&IsDownloadDisplayable)); |
query.AddSorter(DownloadQuery::SORT_START_TIME, DownloadQuery::DESCENDING); |
query.Limit(kMaxDownloads); |
query.Search(all_items.begin(), all_items.end(), &filtered_items); |
+ |
base::ListValue results_value; |
- for (content::DownloadManager::DownloadVector::iterator |
- iter = filtered_items.begin(); iter != filtered_items.end(); ++iter) { |
+ for (auto it = filtered_items.begin(); it != filtered_items.end(); ++it) { |
asanka
2015/03/10 04:15:51
Use a range based for instead.
Dan Beam
2015/03/10 05:41:02
Done.
|
results_value.Append(CreateDownloadItemValue( |
- *iter, |
- (original_notifier_.get() && |
- main_notifier_.GetManager() && |
- (main_notifier_.GetManager()->GetDownload((*iter)->GetId()) == |
- *iter)))); |
+ *it, |
+ original_notifier_ && main_notifier_.GetManager() && |
+ main_notifier_.GetManager()->GetDownload((*it)->GetId()) == *it)); |
} |
- CallDownloadsList(results_value); |
+ CallUpdateAll(results_value); |
} |
void DownloadsDOMHandler::ShowDangerPrompt( |
@@ -619,9 +645,9 @@ void DownloadsDOMHandler::DangerPromptDone( |
bool DownloadsDOMHandler::IsDeletingHistoryAllowed() { |
content::DownloadManager* manager = main_notifier_.GetManager(); |
- return (manager && |
- Profile::FromBrowserContext(manager->GetBrowserContext())-> |
- GetPrefs()->GetBoolean(prefs::kAllowDeletingBrowserHistory)); |
+ return manager && |
+ Profile::FromBrowserContext(manager->GetBrowserContext())-> |
+ GetPrefs()->GetBoolean(prefs::kAllowDeletingBrowserHistory); |
} |
content::DownloadItem* DownloadsDOMHandler::GetDownloadByValue( |
@@ -645,7 +671,7 @@ content::DownloadItem* DownloadsDOMHandler::GetDownloadById(uint32 id) { |
content::DownloadItem* item = NULL; |
if (GetMainNotifierManager()) |
item = GetMainNotifierManager()->GetDownload(id); |
- if (!item && original_notifier_.get() && original_notifier_->GetManager()) |
+ if (!item && original_notifier_ && original_notifier_->GetManager()) |
item = original_notifier_->GetManager()->GetDownload(id); |
return item; |
} |
@@ -654,11 +680,10 @@ content::WebContents* DownloadsDOMHandler::GetWebUIWebContents() { |
return web_ui()->GetWebContents(); |
} |
-void DownloadsDOMHandler::CallDownloadsList(const base::ListValue& downloads) { |
- web_ui()->CallJavascriptFunction("downloadsList", downloads); |
+void DownloadsDOMHandler::CallUpdateAll(const base::ListValue& list) { |
+ web_ui()->CallJavascriptFunction("downloads.Manager.updateAll", list); |
} |
-void DownloadsDOMHandler::CallDownloadUpdated( |
- const base::ListValue& download_item) { |
- web_ui()->CallJavascriptFunction("downloadUpdated", download_item); |
+void DownloadsDOMHandler::CallUpdateItem(const base::DictionaryValue& item) { |
+ web_ui()->CallJavascriptFunction("downloads.Manager.updateItem", item); |
} |