diff --git a/DEPS b/DEPS index bcd0b02..88720f9 100644 --- a/DEPS +++ b/DEPS
@@ -43,7 +43,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other. - 'v8_revision': '39ded14758a814db6bfe4d59ac3190431dd4ff64', + 'v8_revision': '56a72bc710c1ad6af85dfc753baaeb03da0c5445', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling swarming_client # and whatever else without interference from each other. @@ -190,7 +190,7 @@ Var('chromium_git') + '/chromium/third_party/ffmpeg.git' + '@' + 'f962bcb6a8aed397bb2d418527a4e4a23f5ab955', 'src/third_party/libjingle/source/talk': - Var('chromium_git') + '/external/webrtc/trunk/talk.git' + '@' + '27d493b66324061421c6a3626cf00f76f9bb60b9', # commit position 10819 + Var('chromium_git') + '/external/webrtc/trunk/talk.git' + '@' + '67e7a761483e48f720a388b886b0354225858700', # commit position 10833 'src/third_party/usrsctp/usrsctplib': Var('chromium_git') + '/external/usrsctplib.git' + '@' + '36444a999739e9e408f8f587cb4c3ffeef2e50ac', # from svn revision 9215 @@ -214,7 +214,7 @@ Var('chromium_git') + '/native_client/src/third_party/scons-2.0.1.git' + '@' + '1c1550e17fc26355d08627fbdec13d8291227067', 'src/third_party/webrtc': - Var('chromium_git') + '/external/webrtc/trunk/webrtc.git' + '@' + '415a73ab088f08b9762840703f5b84f7d82d2999', # commit position 10821 + Var('chromium_git') + '/external/webrtc/trunk/webrtc.git' + '@' + 'febd8d424cc5ae0ba65a0e1806ed80d76d9e9e5e', # commit position 10836 'src/third_party/openmax_dl': Var('chromium_git') + '/external/webrtc/deps/third_party/openmax.git' + '@' + Var('openmax_dl_revision'),
diff --git a/chrome/browser/download/download_query.cc b/chrome/browser/download/download_query.cc index 7e6ce7f6..415b622 100644 --- a/chrome/browser/download/download_query.cc +++ b/chrome/browser/download/download_query.cc
@@ -69,35 +69,6 @@ // The next several functions are helpers for making Callbacks that access // DownloadItem fields. -static bool MatchesQuery( - const std::vector<base::string16>& query_terms, - const DownloadItem& item) { - DCHECK(!query_terms.empty()); - base::string16 url_raw(base::UTF8ToUTF16(item.GetOriginalUrl().spec())); - base::string16 url_formatted = url_raw; - if (item.GetBrowserContext()) { - Profile* profile = Profile::FromBrowserContext(item.GetBrowserContext()); - url_formatted = url_formatter::FormatUrl( - item.GetOriginalUrl(), - profile->GetPrefs()->GetString(prefs::kAcceptLanguages)); - } - base::string16 path(item.GetTargetFilePath().LossyDisplayName()); - - for (std::vector<base::string16>::const_iterator it = query_terms.begin(); - it != query_terms.end(); ++it) { - base::string16 term = base::i18n::ToLower(*it); - if (!base::i18n::StringSearchIgnoringCaseAndAccents( - term, url_raw, NULL, NULL) && - !base::i18n::StringSearchIgnoringCaseAndAccents( - term, url_formatted, NULL, NULL) && - !base::i18n::StringSearchIgnoringCaseAndAccents( - term, path, NULL, NULL)) { - return false; - } - } - return true; -} - static int64 GetStartTimeMsEpoch(const DownloadItem& item) { return (item.GetStartTime() - base::Time::UnixEpoch()).InMilliseconds(); } @@ -237,6 +208,37 @@ } // anonymous namespace +// static +bool DownloadQuery::MatchesQuery(const std::vector<base::string16>& query_terms, + const DownloadItem& item) { + if (query_terms.empty()) + return true; + + base::string16 url_raw(base::UTF8ToUTF16(item.GetOriginalUrl().spec())); + base::string16 url_formatted = url_raw; + if (item.GetBrowserContext()) { + Profile* profile = Profile::FromBrowserContext(item.GetBrowserContext()); + url_formatted = url_formatter::FormatUrl( + item.GetOriginalUrl(), + profile->GetPrefs()->GetString(prefs::kAcceptLanguages)); + } + base::string16 path(item.GetTargetFilePath().LossyDisplayName()); + + for (std::vector<base::string16>::const_iterator it = query_terms.begin(); + it != query_terms.end(); ++it) { + base::string16 term = base::i18n::ToLower(*it); + if (!base::i18n::StringSearchIgnoringCaseAndAccents( + term, url_raw, NULL, NULL) && + !base::i18n::StringSearchIgnoringCaseAndAccents( + term, url_formatted, NULL, NULL) && + !base::i18n::StringSearchIgnoringCaseAndAccents( + term, path, NULL, NULL)) { + return false; + } + } + return true; +} + DownloadQuery::DownloadQuery() : limit_(kuint32max), skip_(0U) {} DownloadQuery::~DownloadQuery() {}
diff --git a/chrome/browser/download/download_query.h b/chrome/browser/download/download_query.h index baa5190c..22ea2bf 100644 --- a/chrome/browser/download/download_query.h +++ b/chrome/browser/download/download_query.h
@@ -10,6 +10,7 @@ #include <vector> #include "base/callback_forward.h" +#include "base/strings/string16.h" #include "content/public/browser/download_item.h" namespace base { @@ -90,6 +91,9 @@ DESCENDING, }; + static bool MatchesQuery(const std::vector<base::string16>& query_terms, + const content::DownloadItem& item); + DownloadQuery(); ~DownloadQuery();
diff --git a/chrome/browser/resources/md_downloads/action_service.js b/chrome/browser/resources/md_downloads/action_service.js index f984fe5..ee0a1b4 100644 --- a/chrome/browser/resources/md_downloads/action_service.js +++ b/chrome/browser/resources/md_downloads/action_service.js
@@ -14,6 +14,27 @@ /** @constructor */ function ActionService() {} + /** + * @param {string} s + * @return {string} |s| without whitespace at the beginning or end. + */ + function trim(s) { return s.trim(); } + + /** + * @param {string|undefined} value + * @return {boolean} Whether |value| is truthy. + */ + function truthy(value) { return !!value; } + + /** + * @param {string} searchText Input typed by the user into a search box. + * @return {Array<string>} A list of terms extracted from |searchText|. + */ + ActionService.splitTerms = function(searchText) { + // Split quoted terms (e.g., 'The "lazy" dog' => ['The', 'lazy', 'dog']). + return searchText.split(/"([^"]*)"/).map(trim).filter(truthy); + }; + ActionService.prototype = { /** @param {string} id ID of the download to cancel. */ cancel: chromeSendWithId('cancel'), @@ -40,12 +61,15 @@ /** @param {string} id ID of the download that the user started dragging. */ drag: chromeSendWithId('drag'), + /** @private {boolean} */ + isSearching_: false, + /** * @return {boolean} Whether the user is currently searching for downloads * (i.e. has a non-empty search term). */ isSearching: function() { - return this.searchText_.length > 0; + return this.isSearching_; }, /** Opens the current local destination for downloads. */ @@ -78,9 +102,10 @@ this.searchText_ = searchText; - // Split quoted terms (e.g., 'The "lazy" dog' => ['The', 'lazy', 'dog']). - function trim(s) { return s.trim(); } - chrome.send('getDownloads', searchText.split(/"([^"]*)"/).map(trim)); + var terms = ActionService.splitTerms(searchText); + this.isSearching_ = terms.length > 0; + + chrome.send('getDownloads', terms); }, /**
diff --git a/chrome/browser/resources/md_downloads/action_service_unittest.js b/chrome/browser/resources/md_downloads/action_service_unittest.js new file mode 100644 index 0000000..b188594 --- /dev/null +++ b/chrome/browser/resources/md_downloads/action_service_unittest.js
@@ -0,0 +1,37 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +/** + * @param {!Array<string>} list + * @return {string} + */ +function str(list) { + return JSON.stringify(list); +} + +/** + * @extends {testing.Test} + * @constructor + */ +function ActionServiceUnitTest() {} + +ActionServiceUnitTest.prototype = { + __proto__: testing.Test.prototype, + + /** @override */ + extraLibraries: [ + '../../../../ui/webui/resources/js/cr.js', + 'action_service.js', + ], +}; + +TEST_F('ActionServiceUnitTest', 'splitTerms', function() { + var ActionService = downloads.ActionService; + assertEquals(str([]), str(ActionService.splitTerms(''))); + assertEquals(str([]), str(ActionService.splitTerms(' '))); + assertEquals(str(['a']), str(ActionService.splitTerms('a'))); + assertEquals(str(['a b']), str(ActionService.splitTerms('a b'))); + assertEquals(str(['a', 'b']), str(ActionService.splitTerms('a "b"'))); + assertEquals(str(['a', 'b', 'c']), str(ActionService.splitTerms('a "b" c'))); +});
diff --git a/chrome/browser/resources/md_downloads/crisper.js b/chrome/browser/resources/md_downloads/crisper.js index da445238..3b32922d 100644 --- a/chrome/browser/resources/md_downloads/crisper.js +++ b/chrome/browser/resources/md_downloads/crisper.js
@@ -1531,6 +1531,27 @@ /** @constructor */ function ActionService() {} + /** + * @param {string} searchText Input typed by the user into a search box. + * @return {Array<string>} A list of terms extracted from |searchText|. + */ + ActionService.splitTerms = function(searchText) { + /** + * @param {string} s + * @return {string} |s| without whitespace at the beginning or end. + */ + function trim(s) { return s.trim(); } + + /** + * @param {string} s + * @return {boolean} Whether |s| is empty. + */ + function notEmpty(s) { return s.length > 0; } + + // Split quoted terms (e.g., 'The "lazy" dog' => ['The', 'lazy', 'dog']). + return searchText.split(/"([^"]*)"/).map(trim).filter(notEmpty); + }; + ActionService.prototype = { /** @param {string} id ID of the download to cancel. */ cancel: chromeSendWithId('cancel'), @@ -1562,7 +1583,7 @@ * (i.e. has a non-empty search term). */ isSearching: function() { - return this.searchText_.length > 0; + return !!this.searchText_; }, /** Opens the current local destination for downloads. */ @@ -1594,10 +1615,7 @@ return; this.searchText_ = searchText; - - // Split quoted terms (e.g., 'The "lazy" dog' => ['The', 'lazy', 'dog']). - function trim(s) { return s.trim(); } - chrome.send('getDownloads', searchText.split(/"([^"]*)"/).map(trim)); + chrome.send('getDownloads', ActionService.splitTerms(searchText)); }, /** @@ -16937,12 +16955,13 @@ properties: { hasDownloads_: { + observer: 'hasDownloadsChanged_', type: Boolean, - value: false, }, items_: { type: Array, + value: function() { return []; }, }, }, @@ -16950,6 +16969,46 @@ loading: true, }, + observers: [ + 'itemsChanged_(items_.*)', + ], + + /** @private */ + clearAll_: function() { + this.set('items_', []); + }, + + /** @private */ + hasDownloadsChanged_: function() { + if (loadTimeData.getBoolean('allowDeletingHistory')) + this.$.toolbar.downloadsShowing = this.hasDownloads_; + + if (this.hasDownloads_) { + this.$['downloads-list'].fire('iron-resize'); + } else { + var isSearching = downloads.ActionService.getInstance().isSearching(); + var messageToShow = isSearching ? 'noSearchResults' : 'noDownloads'; + this.$['no-downloads'].querySelector('span').textContent = + loadTimeData.getString(messageToShow); + } + }, + + /** + * @param {number} index + * @param {!Array<!downloads.Data>} list + * @private + */ + insertItems_: function(index, list) { + this.splice.apply(this, ['items_', index, 0].concat(list)); + this.updateHideDates_(index, index + list.length); + this.removeAttribute('loading'); + }, + + /** @private */ + itemsChanged_: function() { + this.hasDownloads_ = this.items_.length > 0; + }, + /** * @param {Event} e * @private @@ -16988,78 +17047,64 @@ }, /** - * @return {number} The number of downloads shown on the page. + * @param {number} index * @private */ - size_: function() { - return this.items_.length; + removeItem_: function(index) { + this.splice('items_', index, 1); + this.updateHideDates_(index, index); }, /** - * Called when all items need to be updated. - * @param {!Array<!downloads.Data>} list A list of new download data. + * @param {number} start + * @param {number} end * @private */ - updateAll_: function(list) { - /** @private {!Object<number>} */ - this.idToIndex_ = {}; - - for (var i = 0; i < list.length; ++i) { - var data = list[i]; - - this.idToIndex_[data.id] = data.index = i; - - var prev = list[i - 1]; - data.hideDate = !!prev && prev.date_string == data.date_string; + updateHideDates_: function(start, end) { + for (var i = start; i <= end; ++i) { + var current = this.items_[i]; + if (!current) + continue; + var prev = this.items_[i - 1]; + current.hideDate = !!prev && prev.date_string == current.date_string; } - - // TODO(dbeam): this resets the scroll position, which is a huge bummer. - // Removing something from the bottom of the list should not scroll you - // back to the top. The grand plan is to restructure how the C++ sends the - // JS data so that it only gets updates (rather than the most recent set - // of items). TL;DR - we can't ship with this bug. - this.items_ = list; - - var hasDownloads = this.size_() > 0; - if (!hasDownloads) { - var isSearching = downloads.ActionService.getInstance().isSearching(); - var messageToShow = isSearching ? 'noSearchResults' : 'noDownloads'; - this.$['no-downloads'].querySelector('span').textContent = - loadTimeData.getString(messageToShow); - } - this.hasDownloads_ = hasDownloads; - - if (loadTimeData.getBoolean('allowDeletingHistory')) - this.$.toolbar.downloadsShowing = this.hasDownloads_; - - this.removeAttribute('loading'); }, /** + * @param {number} index * @param {!downloads.Data} data * @private */ - updateItem_: function(data) { - var index = this.idToIndex_[data.id]; + updateItem_: function(index, data) { this.set('items_.' + index, data); + this.updateHideDates_(index, index); this.$['downloads-list'].updateSizeForItem(index); }, }); - Manager.size = function() { - return document.querySelector('downloads-manager').size_(); + Manager.clearAll = function() { + Manager.get().clearAll_(); }; - Manager.updateAll = function(list) { - document.querySelector('downloads-manager').updateAll_(list); + /** @return {!downloads.Manager} */ + Manager.get = function() { + return queryRequiredElement('downloads-manager'); }; - Manager.updateItem = function(item) { - document.querySelector('downloads-manager').updateItem_(item); + Manager.insertItems = function(index, list) { + Manager.get().insertItems_(index, list); }; Manager.onLoad = function() { - document.querySelector('downloads-manager').onLoad_(); + Manager.get().onLoad_(); + }; + + Manager.removeItem = function(index) { + Manager.get().removeItem_(index); + }; + + Manager.updateItem = function(index, data) { + Manager.get().updateItem_(index, data); }; return {Manager: Manager};
diff --git a/chrome/browser/resources/md_downloads/i18n.html b/chrome/browser/resources/md_downloads/i18n.html new file mode 100644 index 0000000..1db516c --- /dev/null +++ b/chrome/browser/resources/md_downloads/i18n.html
@@ -0,0 +1,2 @@ +<script src="chrome://resources/js/load_time_data.js"></script> +<script src="chrome://downloads/strings.js"></script>
diff --git a/chrome/browser/resources/md_downloads/manager.html b/chrome/browser/resources/md_downloads/manager.html index d1b2fa7..159a0d5 100644 --- a/chrome/browser/resources/md_downloads/manager.html +++ b/chrome/browser/resources/md_downloads/manager.html
@@ -23,7 +23,7 @@ <div id="no-downloads" hidden="[[hasDownloads_]]"> <div> <div class="illustration"></div> - <span><!-- Text populated in Manager#updateAll_(). --></span> + <span><!-- Text populated dynamically. --></span> </div> </div> </template>
diff --git a/chrome/browser/resources/md_downloads/manager.js b/chrome/browser/resources/md_downloads/manager.js index 76017ac..91faf6a1 100644 --- a/chrome/browser/resources/md_downloads/manager.js +++ b/chrome/browser/resources/md_downloads/manager.js
@@ -8,12 +8,13 @@ properties: { hasDownloads_: { + observer: 'hasDownloadsChanged_', type: Boolean, - value: false, }, items_: { type: Array, + value: function() { return []; }, }, }, @@ -21,6 +22,46 @@ loading: true, }, + observers: [ + 'itemsChanged_(items_.*)', + ], + + /** @private */ + clearAll_: function() { + this.set('items_', []); + }, + + /** @private */ + hasDownloadsChanged_: function() { + if (loadTimeData.getBoolean('allowDeletingHistory')) + this.$.toolbar.downloadsShowing = this.hasDownloads_; + + if (this.hasDownloads_) { + this.$['downloads-list'].fire('iron-resize'); + } else { + var isSearching = downloads.ActionService.getInstance().isSearching(); + var messageToShow = isSearching ? 'noSearchResults' : 'noDownloads'; + this.$['no-downloads'].querySelector('span').textContent = + loadTimeData.getString(messageToShow); + } + }, + + /** + * @param {number} index + * @param {!Array<!downloads.Data>} list + * @private + */ + insertItems_: function(index, list) { + this.splice.apply(this, ['items_', index, 0].concat(list)); + this.updateHideDates_(index, index + list.length); + this.removeAttribute('loading'); + }, + + /** @private */ + itemsChanged_: function() { + this.hasDownloads_ = this.items_.length > 0; + }, + /** * @param {Event} e * @private @@ -59,78 +100,65 @@ }, /** - * @return {number} The number of downloads shown on the page. + * @param {number} index * @private */ - size_: function() { - return this.items_.length; + removeItem_: function(index) { + this.splice('items_', index, 1); + this.updateHideDates_(index, index); }, /** - * Called when all items need to be updated. - * @param {!Array<!downloads.Data>} list A list of new download data. + * @param {number} start + * @param {number} end * @private */ - updateAll_: function(list) { - /** @private {!Object<number>} */ - this.idToIndex_ = {}; - - for (var i = 0; i < list.length; ++i) { - var data = list[i]; - - this.idToIndex_[data.id] = data.index = i; - - var prev = list[i - 1]; - data.hideDate = !!prev && prev.date_string == data.date_string; + updateHideDates_: function(start, end) { + for (var i = start; i <= end; ++i) { + var current = this.items_[i]; + if (!current) + continue; + var prev = this.items_[i - 1]; + current.hideDate = !!prev && prev.date_string == current.date_string; } - - // TODO(dbeam): this resets the scroll position, which is a huge bummer. - // Removing something from the bottom of the list should not scroll you - // back to the top. The grand plan is to restructure how the C++ sends the - // JS data so that it only gets updates (rather than the most recent set - // of items). TL;DR - we can't ship with this bug. - this.items_ = list; - - var hasDownloads = this.size_() > 0; - if (!hasDownloads) { - var isSearching = downloads.ActionService.getInstance().isSearching(); - var messageToShow = isSearching ? 'noSearchResults' : 'noDownloads'; - this.$['no-downloads'].querySelector('span').textContent = - loadTimeData.getString(messageToShow); - } - this.hasDownloads_ = hasDownloads; - - if (loadTimeData.getBoolean('allowDeletingHistory')) - this.$.toolbar.downloadsShowing = this.hasDownloads_; - - this.removeAttribute('loading'); }, /** + * @param {number} index * @param {!downloads.Data} data * @private */ - updateItem_: function(data) { - var index = this.idToIndex_[data.id]; + updateItem_: function(index, data) { this.set('items_.' + index, data); + this.updateHideDates_(index, index); this.$['downloads-list'].updateSizeForItem(index); }, }); - Manager.size = function() { - return document.querySelector('downloads-manager').size_(); + Manager.clearAll = function() { + Manager.get().clearAll_(); }; - Manager.updateAll = function(list) { - document.querySelector('downloads-manager').updateAll_(list); + /** @return {!downloads.Manager} */ + Manager.get = function() { + return /** @type {!downloads.Manager} */( + queryRequiredElement('downloads-manager')); }; - Manager.updateItem = function(item) { - document.querySelector('downloads-manager').updateItem_(item); + Manager.insertItems = function(index, list) { + Manager.get().insertItems_(index, list); }; Manager.onLoad = function() { - document.querySelector('downloads-manager').onLoad_(); + Manager.get().onLoad_(); + }; + + Manager.removeItem = function(index) { + Manager.get().removeItem_(index); + }; + + Manager.updateItem = function(index, data) { + Manager.get().updateItem_(index, data); }; return {Manager: Manager};
diff --git a/chrome/browser/ui/webui/md_downloads/downloads_list_tracker.cc b/chrome/browser/ui/webui/md_downloads/downloads_list_tracker.cc new file mode 100644 index 0000000..b163644 --- /dev/null +++ b/chrome/browser/ui/webui/md_downloads/downloads_list_tracker.cc
@@ -0,0 +1,394 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/ui/webui/md_downloads/downloads_list_tracker.h" + +#include <iterator> + +#include "base/bind.h" +#include "base/bind_helpers.h" +#include "base/i18n/rtl.h" +#include "base/strings/string16.h" +#include "base/strings/string_number_conversions.h" +#include "base/time/time.h" +#include "base/value_conversions.h" +#include "base/values.h" +#include "chrome/browser/download/all_download_item_notifier.h" +#include "chrome/browser/download/download_crx_util.h" +#include "chrome/browser/download/download_item_model.h" +#include "chrome/browser/download/download_query.h" +#include "chrome/browser/extensions/api/downloads/downloads_api.h" +#include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/profiles/profile.h" +#include "content/public/browser/browser_context.h" +#include "content/public/browser/download_item.h" +#include "content/public/browser/download_manager.h" +#include "content/public/browser/web_ui.h" +#include "extensions/browser/extension_system.h" +#include "net/base/filename_util.h" +#include "third_party/icu/source/i18n/unicode/datefmt.h" +#include "ui/base/l10n/time_format.h" + +using content::BrowserContext; +using content::DownloadItem; +using content::DownloadManager; + +using DownloadVector = DownloadManager::DownloadVector; + +namespace { + +// Returns a string constant to be used as the |danger_type| value in +// CreateDownloadItemValue(). Only return strings for DANGEROUS_FILE, +// DANGEROUS_URL, DANGEROUS_CONTENT, and UNCOMMON_CONTENT because the +// |danger_type| value is only defined if the value of |state| is |DANGEROUS|. +const char* GetDangerTypeString(content::DownloadDangerType danger_type) { + switch (danger_type) { + case content::DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE: + return "DANGEROUS_FILE"; + case content::DOWNLOAD_DANGER_TYPE_DANGEROUS_URL: + return "DANGEROUS_URL"; + case content::DOWNLOAD_DANGER_TYPE_DANGEROUS_CONTENT: + return "DANGEROUS_CONTENT"; + case content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT: + return "UNCOMMON_CONTENT"; + case content::DOWNLOAD_DANGER_TYPE_DANGEROUS_HOST: + return "DANGEROUS_HOST"; + case content::DOWNLOAD_DANGER_TYPE_POTENTIALLY_UNWANTED: + return "POTENTIALLY_UNWANTED"; + case content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS: + case content::DOWNLOAD_DANGER_TYPE_MAYBE_DANGEROUS_CONTENT: + case content::DOWNLOAD_DANGER_TYPE_USER_VALIDATED: + case content::DOWNLOAD_DANGER_TYPE_MAX: + break; + } + // Don't return a danger type string if it is NOT_DANGEROUS, + // MAYBE_DANGEROUS_CONTENT, or USER_VALIDATED. + NOTREACHED(); + return ""; +} + +// TODO(dbeam): if useful elsewhere, move to base/i18n/time_formatting.h? +base::string16 TimeFormatLongDate(const base::Time& time) { + scoped_ptr<icu::DateFormat> formatter( + icu::DateFormat::createDateInstance(icu::DateFormat::kLong)); + icu::UnicodeString date_string; + formatter->format(static_cast<UDate>(time.ToDoubleT() * 1000), date_string); + return base::string16(date_string.getBuffer(), + static_cast<size_t>(date_string.length())); +} + +} // namespace + +DownloadsListTracker::DownloadsListTracker( + DownloadManager* download_manager, + content::WebUI* web_ui) + : main_notifier_(download_manager, this), + web_ui_(web_ui), + should_show_(base::Bind(&DownloadsListTracker::ShouldShow, + base::Unretained(this))) { + Init(); +} + +DownloadsListTracker::~DownloadsListTracker() {} + +void DownloadsListTracker::CallClearAll() { + if (sending_updates_) + web_ui_->CallJavascriptFunction("downloads.Manager.clearAll"); +} + +bool DownloadsListTracker::SetSearchTerms(const base::ListValue& search_terms) { + std::vector<base::string16> new_terms; + new_terms.resize(search_terms.GetSize()); + + for (size_t i = 0; i < search_terms.GetSize(); ++i) + search_terms.GetString(i, &new_terms[i]); + + if (new_terms == search_terms_) + return false; + + search_terms_.swap(new_terms); + RebuildSortedSet(); + return true; +} + +void DownloadsListTracker::Start() { + sending_updates_ = true; + + // TODO(dbeam): paging and limiting logic. + + base::ListValue list; + for (auto* item : sorted_visible_items_) + list.Append(CreateDownloadItemValue(item).Pass()); + + web_ui_->CallJavascriptFunction("downloads.Manager.insertItems", + base::FundamentalValue(0), list); +} + +void DownloadsListTracker::Stop() { + sending_updates_ = false; +} + +DownloadManager* DownloadsListTracker::GetMainNotifierManager() const { + return main_notifier_.GetManager(); +} + +DownloadManager* DownloadsListTracker::GetOriginalNotifierManager() const { + return original_notifier_ ? original_notifier_->GetManager() : nullptr; +} + +void DownloadsListTracker::OnDownloadCreated(DownloadManager* manager, + DownloadItem* download_item) { + if (should_show_.Run(*download_item)) + CallInsertItem(sorted_visible_items_.insert(download_item).first); +} + +void DownloadsListTracker::OnDownloadUpdated(DownloadManager* manager, + DownloadItem* download_item) { + auto current_position = sorted_visible_items_.find(download_item); + bool is_showing = current_position != sorted_visible_items_.end(); + bool should_show = should_show_.Run(*download_item); + + if (!is_showing && should_show) + CallInsertItem(sorted_visible_items_.insert(download_item).first); + else if (is_showing && !should_show) + RemoveItem(current_position); + else if (is_showing) + CallUpdateItem(current_position); +} + +void DownloadsListTracker::OnDownloadRemoved(DownloadManager* manager, + DownloadItem* download_item) { + auto current_position = sorted_visible_items_.find(download_item); + if (current_position != sorted_visible_items_.end()) + RemoveItem(current_position); +} + +DownloadsListTracker::DownloadsListTracker( + DownloadManager* download_manager, + content::WebUI* web_ui, + base::Callback<bool(const DownloadItem&)> should_show) + : main_notifier_(download_manager, this), + web_ui_(web_ui), + should_show_(should_show) { + Init(); +} + +scoped_ptr<base::DictionaryValue> DownloadsListTracker::CreateDownloadItemValue( + content::DownloadItem* download_item) const { + // TODO(asanka): Move towards using download_model here for getting status and + // progress. The difference currently only matters to Drive downloads and + // those don't show up on the downloads page, but should. + DownloadItemModel download_model(download_item); + + // The items which are to be written into file_value are also described in + // chrome/browser/resources/downloads/downloads.js in @typedef for + // BackendDownloadObject. Please update it whenever you add or remove + // any keys in file_value. + scoped_ptr<base::DictionaryValue> file_value(new base::DictionaryValue); + + file_value->SetInteger( + "started", static_cast<int>(download_item->GetStartTime().ToTimeT())); + file_value->SetString( + "since_string", ui::TimeFormat::RelativeDate( + download_item->GetStartTime(), NULL)); + file_value->SetString( + "date_string", TimeFormatLongDate(download_item->GetStartTime())); + + file_value->SetString("id", base::Uint64ToString(download_item->GetId())); + + base::FilePath download_path(download_item->GetTargetFilePath()); + file_value->Set("file_path", base::CreateFilePathValue(download_path)); + file_value->SetString("file_url", + net::FilePathToFileURL(download_path).spec()); + + extensions::DownloadedByExtension* by_ext = + extensions::DownloadedByExtension::Get(download_item); + std::string by_ext_id; + std::string by_ext_name; + if (by_ext) { + by_ext_id = by_ext->id(); + // TODO(dbeam): why doesn't DownloadsByExtension::name() return a string16? + by_ext_name = by_ext->name(); + + // Lookup the extension's current name() in case the user changed their + // language. This won't work if the extension was uninstalled, so the name + // might be the wrong language. + bool include_disabled = true; + const extensions::Extension* extension = extensions::ExtensionSystem::Get( + Profile::FromBrowserContext(download_item->GetBrowserContext()))-> + extension_service()->GetExtensionById(by_ext->id(), include_disabled); + if (extension) + by_ext_name = extension->name(); + } + file_value->SetString("by_ext_id", by_ext_id); + file_value->SetString("by_ext_name", by_ext_name); + + // Keep file names as LTR. TODO(dbeam): why? + base::string16 file_name = + download_item->GetFileNameToReportUser().LossyDisplayName(); + file_name = base::i18n::GetDisplayStringInLTRDirectionality(file_name); + file_value->SetString("file_name", file_name); + file_value->SetString("url", download_item->GetURL().spec()); + file_value->SetInteger("total", static_cast<int>( + download_item->GetTotalBytes())); + file_value->SetBoolean("file_externally_removed", + download_item->GetFileExternallyRemoved()); + file_value->SetBoolean("resume", download_item->CanResume()); + + bool incognito = false; + auto* original_manager = GetOriginalNotifierManager(); + if (original_manager) { + incognito = + original_manager->GetDownload(download_item->GetId()) == download_item; + } + file_value->SetBoolean("otr", incognito); + + const char* danger_type = ""; + base::string16 last_reason_text; + // -2 is invalid, -1 means indeterminate, and 0-100 are in-progress. + int percent = -2; + base::string16 progress_status_text; + bool retry = false; + const char* state = nullptr; + + switch (download_item->GetState()) { + case content::DownloadItem::IN_PROGRESS: { + if (download_item->IsDangerous()) { + state = "DANGEROUS"; + danger_type = GetDangerTypeString(download_item->GetDangerType()); + } else if (download_item->IsPaused()) { + state = "PAUSED"; + } else { + state = "IN_PROGRESS"; + } + progress_status_text = download_model.GetTabProgressStatusText(); + percent = download_item->PercentComplete(); + break; + } + + case content::DownloadItem::INTERRUPTED: + state = "INTERRUPTED"; + progress_status_text = download_model.GetTabProgressStatusText(); + + if (download_item->CanResume()) + percent = download_item->PercentComplete(); + + last_reason_text = download_model.GetInterruptReasonText(); + if (content::DOWNLOAD_INTERRUPT_REASON_CRASH == + download_item->GetLastReason() && !download_item->CanResume()) { + retry = true; + } + break; + + case content::DownloadItem::CANCELLED: + state = "CANCELLED"; + retry = true; + break; + + case content::DownloadItem::COMPLETE: + DCHECK(!download_item->IsDangerous()); + state = "COMPLETE"; + break; + + case content::DownloadItem::MAX_DOWNLOAD_STATE: + NOTREACHED(); + } + + DCHECK(state); + + file_value->SetString("danger_type", danger_type); + file_value->SetString("last_reason_text", last_reason_text); + file_value->SetInteger("percent", percent); + file_value->SetString("progress_status_text", progress_status_text); + file_value->SetBoolean("retry", retry); + file_value->SetString("state", state); + + return file_value.Pass(); +} + +const DownloadItem* DownloadsListTracker::GetItemForTesting(size_t index) + const { + if (index >= sorted_visible_items_.size()) + return nullptr; + + SortedSet::iterator it = sorted_visible_items_.begin(); + std::advance(it, index); + return *it; +} + +bool DownloadsListTracker::ShouldShow(const DownloadItem& item) const { + return !download_crx_util::IsExtensionDownload(item) && + !item.IsTemporary() && + !item.GetFileNameToReportUser().empty() && + !item.GetTargetFilePath().empty() && + DownloadItemModel(const_cast<DownloadItem*>(&item)).ShouldShowInShelf() && + DownloadQuery::MatchesQuery(search_terms_, item); +} + +bool DownloadsListTracker::StartTimeComparator::operator() ( + const content::DownloadItem* a, const content::DownloadItem* b) const { + return a->GetStartTime() > b->GetStartTime(); +} + +void DownloadsListTracker::Init() { + Profile* profile = Profile::FromBrowserContext( + GetMainNotifierManager()->GetBrowserContext()); + if (profile->IsOffTheRecord()) { + original_notifier_.reset(new AllDownloadItemNotifier( + BrowserContext::GetDownloadManager(profile->GetOriginalProfile()), + this)); + } + + RebuildSortedSet(); +} + +void DownloadsListTracker::RebuildSortedSet() { + DownloadVector all_items, visible_items; + + GetMainNotifierManager()->GetAllDownloads(&all_items); + + if (GetOriginalNotifierManager()) + GetOriginalNotifierManager()->GetAllDownloads(&all_items); + + DownloadQuery query; + query.AddFilter(should_show_); + query.Search(all_items.begin(), all_items.end(), &visible_items); + + SortedSet sorted_visible_items(visible_items.begin(), visible_items.end()); + sorted_visible_items_.swap(sorted_visible_items); +} + +void DownloadsListTracker::CallInsertItem(const SortedSet::iterator& insert) { + if (!sending_updates_) + return; + + base::ListValue list; + list.Append(CreateDownloadItemValue(*insert).Pass()); + + web_ui_->CallJavascriptFunction("downloads.Manager.insertItems", + base::FundamentalValue(GetIndex(insert)), + list); +} + +void DownloadsListTracker::CallUpdateItem(const SortedSet::iterator& update) { + if (!sending_updates_) + return; + + web_ui_->CallJavascriptFunction("downloads.Manager.updateItem", + base::FundamentalValue(GetIndex(update)), + *CreateDownloadItemValue(*update)); +} + +int DownloadsListTracker::GetIndex(const SortedSet::iterator& position) const { + // TODO(dbeam): this could be log(N) if |position| was random access. + return std::distance(sorted_visible_items_.begin(), position); +} + +void DownloadsListTracker::RemoveItem(const SortedSet::iterator& remove) { + if (sending_updates_) { + web_ui_->CallJavascriptFunction("downloads.Manager.removeItem", + base::FundamentalValue(GetIndex(remove))); + } + sorted_visible_items_.erase(remove); +}
diff --git a/chrome/browser/ui/webui/md_downloads/downloads_list_tracker.h b/chrome/browser/ui/webui/md_downloads/downloads_list_tracker.h new file mode 100644 index 0000000..05399b3 --- /dev/null +++ b/chrome/browser/ui/webui/md_downloads/downloads_list_tracker.h
@@ -0,0 +1,125 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_UI_WEBUI_MD_DOWNLOADS_DOWNLOADS_LIST_TRACKER_H_ +#define CHROME_BROWSER_UI_WEBUI_MD_DOWNLOADS_DOWNLOADS_LIST_TRACKER_H_ + +#include <set> + +#include "base/callback_forward.h" +#include "base/macros.h" +#include "base/memory/scoped_ptr.h" +#include "base/strings/string16.h" +#include "base/time/time.h" +#include "base/values.h" +#include "chrome/browser/download/all_download_item_notifier.h" +#include "content/public/browser/download_item.h" + +namespace base { +class DictionaryValue; +class ListValue; +} + +namespace content { +class DownloadManager; +class WebUI; +} + +// A class that tracks all downloads activity and keeps a sorted representation +// of the downloads as chrome://downloads wants to display them. +class DownloadsListTracker : public AllDownloadItemNotifier::Observer { + public: + DownloadsListTracker(content::DownloadManager* download_manager, + content::WebUI* web_ui); + ~DownloadsListTracker() override; + + // Clears all downloads on the page (if it's ready). + void CallClearAll(); + + // Shows only downloads that match |search_terms|. An empty list shows all + // downloads. Returns whether |search_terms.Equals(&search_terms_)|. + bool SetSearchTerms(const base::ListValue& search_terms); + + // Sends all downloads and enables updates. + void Start(); + + // Stops sending updates to the page. + void Stop(); + + content::DownloadManager* GetMainNotifierManager() const; + content::DownloadManager* GetOriginalNotifierManager() const; + + // AllDownloadItemNotifier::Observer: + void OnDownloadCreated(content::DownloadManager* manager, + content::DownloadItem* download_item) override; + void OnDownloadUpdated(content::DownloadManager* manager, + content::DownloadItem* download_item) override; + void OnDownloadRemoved(content::DownloadManager* manager, + content::DownloadItem* download_item) override; + + protected: + // Testing constructor. + DownloadsListTracker(content::DownloadManager* download_manager, + content::WebUI* web_ui, + base::Callback<bool(const content::DownloadItem&)>); + + // Creates a dictionary value that's sent to the page as JSON. + virtual scoped_ptr<base::DictionaryValue> CreateDownloadItemValue( + content::DownloadItem* item) const; + + const content::DownloadItem* GetItemForTesting(size_t index) const; + + private: + struct StartTimeComparator { + bool operator() (const content::DownloadItem* a, + const content::DownloadItem* b) const; + }; + using SortedSet = std::set<content::DownloadItem*, StartTimeComparator>; + + // Called by both constructors to initialize common state. + void Init(); + + // Clears and re-inserts all visible items in a sorted order into + // |sorted_visible_items_|. + void RebuildSortedSet(); + + // Whether |item| should show on the current page. + bool ShouldShow(const content::DownloadItem& item) const; + + // Gets a page index for |position| from |sorted_visible_items_|. + int GetIndex(const SortedSet::iterator& position) const; + + // Calls "insertItems" if |sending_updates_|. + void CallInsertItem(const SortedSet::iterator& insert); + + // Calls "updateItem" if |sending_updates_|. + void CallUpdateItem(const SortedSet::iterator& update); + + // Removes the item that corresponds to |remove| and sends a "removeItems" + // message to the page if |sending_updates_|. + void RemoveItem(const SortedSet::iterator& remove); + + AllDownloadItemNotifier main_notifier_; + scoped_ptr<AllDownloadItemNotifier> original_notifier_; + + // The WebUI object corresponding to the page we care about. + content::WebUI* const web_ui_; + + // Callback used to determine if an item should show on the page. Set to + // |ShouldShow()| in default constructor, passed in while testing. + base::Callback<bool(const content::DownloadItem&)> should_show_; + + // When this is true, all changes to downloads that affect the page are sent + // via JavaScript. + bool sending_updates_ = false; + + SortedSet sorted_visible_items_; + + // Current search terms. + std::vector<base::string16> search_terms_; + + DISALLOW_COPY_AND_ASSIGN(DownloadsListTracker); +}; + +#endif // CHROME_BROWSER_UI_WEBUI_MD_DOWNLOADS_DOWNLOADS_LIST_TRACKER_H_
diff --git a/chrome/browser/ui/webui/md_downloads/downloads_list_tracker_unittest.cc b/chrome/browser/ui/webui/md_downloads/downloads_list_tracker_unittest.cc new file mode 100644 index 0000000..fef30f3 --- /dev/null +++ b/chrome/browser/ui/webui/md_downloads/downloads_list_tracker_unittest.cc
@@ -0,0 +1,276 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/ui/webui/md_downloads/downloads_list_tracker.h" + +#include <vector> + +#include "base/files/file_path.h" +#include "base/memory/scoped_ptr.h" +#include "base/message_loop/message_loop.h" +#include "base/stl_util.h" +#include "base/time/time.h" +#include "chrome/browser/download/download_item_model.h" +#include "chrome/test/base/testing_profile.h" +#include "content/public/test/mock_download_item.h" +#include "content/public/test/mock_download_manager.h" +#include "content/public/test/test_browser_thread.h" +#include "content/public/test/test_web_ui.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +using content::DownloadItem; +using content::MockDownloadItem; +using DownloadVector = std::vector<DownloadItem*>; +using testing::_; +using testing::Return; + +namespace { + +uint64 GetId(const base::Value* value) { + const base::DictionaryValue* dict; + CHECK(value->GetAsDictionary(&dict)); + + int id; + CHECK(dict->GetInteger("id", &id)); + CHECK_GE(id, 0); + return static_cast<uint64>(id); +} + +std::vector<uint64> GetIds(const base::Value* value) { + CHECK(value); + + std::vector<uint64> ids; + + if (value->GetType() == base::Value::TYPE_LIST) { + const base::ListValue* list; + value->GetAsList(&list); + + for (auto* list_item : *list) + ids.push_back(GetId(list_item)); + } else { + ids.push_back(GetId(value)); + } + + return ids; +} + +int GetIndex(const base::Value* value) { + CHECK(value); + int index; + CHECK(value->GetAsInteger(&index)); + return index; +} + +bool ShouldShowItem(const DownloadItem& item) { + DownloadItemModel model(const_cast<DownloadItem*>(&item)); + return model.ShouldShowInShelf(); +} + +} // namespace + +// A test version of DownloadsListTracker. +class TestDownloadsListTracker : public DownloadsListTracker { + public: + TestDownloadsListTracker(content::DownloadManager* manager, + content::WebUI* web_ui) + : DownloadsListTracker(manager, web_ui, base::Bind(&ShouldShowItem)) {} + ~TestDownloadsListTracker() override {} + + using DownloadsListTracker::GetItemForTesting; + + protected: + scoped_ptr<base::DictionaryValue> CreateDownloadItemValue( + content::DownloadItem* item) const override { + scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue); + CHECK_LE(item->GetId(), static_cast<uint64>(INT_MAX)); + dict->SetInteger("id", item->GetId()); + return dict.Pass(); + } +}; + +// A fixture to test DownloadsListTracker. +class DownloadsListTrackerTest : public testing::Test { + public: + DownloadsListTrackerTest() + : ui_thread_(content::BrowserThread::UI, &message_loop_) {} + ~DownloadsListTrackerTest() override { + for (auto* mock_item : mock_items_) + testing::Mock::VerifyAndClear(mock_item); + STLDeleteElements(&mock_items_); + } + + // testing::Test: + void SetUp() override { + ON_CALL(manager_, GetBrowserContext()).WillByDefault(Return(&profile_)); + ON_CALL(manager_, GetAllDownloads(_)).WillByDefault( + testing::Invoke(this, &DownloadsListTrackerTest::GetAllDownloads)); + } + + MockDownloadItem* CreateMock(uint64 id, const base::Time& started) { + MockDownloadItem* new_item = new testing::NiceMock<MockDownloadItem>(); + mock_items_.push_back(new_item); + + ON_CALL(*new_item, GetId()).WillByDefault(Return(id)); + ON_CALL(*new_item, GetStartTime()).WillByDefault(Return(started)); + + return new_item; + } + + MockDownloadItem* CreateNextItem() { + return CreateMock(mock_items_.size(), base::Time::UnixEpoch() + + base::TimeDelta::FromHours(mock_items_.size())); + } + + void CreateTracker() { + tracker_.reset(new TestDownloadsListTracker(manager(), web_ui())); + } + + content::DownloadManager* manager() { return &manager_; } + content::TestWebUI* web_ui() { return &web_ui_; } + TestDownloadsListTracker* tracker() { return tracker_.get(); } + + private: + void GetAllDownloads(DownloadVector* result) { + for (auto* mock_item : mock_items_) + result->push_back(mock_item); + } + + // NOTE: The initialization order of these members matters. + base::MessageLoopForUI message_loop_; + content::TestBrowserThread ui_thread_; + TestingProfile profile_; + + testing::NiceMock<content::MockDownloadManager> manager_; + content::TestWebUI web_ui_; + scoped_ptr<TestDownloadsListTracker> tracker_; + + std::vector<MockDownloadItem*> mock_items_; +}; + +TEST_F(DownloadsListTrackerTest, SetSearchTerms) { + CreateTracker(); + + const base::ListValue empty_terms; + EXPECT_FALSE(tracker()->SetSearchTerms(empty_terms)); + + base::ListValue search_terms; + search_terms.AppendString("search"); + EXPECT_TRUE(tracker()->SetSearchTerms(search_terms)); + + EXPECT_FALSE(tracker()->SetSearchTerms(search_terms)); + + EXPECT_TRUE(tracker()->SetSearchTerms(empty_terms)); + + // Notifying the page is left up to the handler in this case. + EXPECT_TRUE(web_ui()->call_data().empty()); +} + +TEST_F(DownloadsListTrackerTest, StartCallsInsertItems) { + DownloadItem* first_item = CreateNextItem(); + + CreateTracker(); + ASSERT_TRUE(tracker()->GetItemForTesting(0)); + EXPECT_TRUE(web_ui()->call_data().empty()); + + tracker()->Start(); + ASSERT_FALSE(web_ui()->call_data().empty()); + + EXPECT_EQ("downloads.Manager.insertItems", + web_ui()->call_data()[0]->function_name()); + EXPECT_EQ(0, GetIndex(web_ui()->call_data()[0]->arg1())); + + std::vector<uint64> ids = GetIds(web_ui()->call_data()[0]->arg2()); + ASSERT_FALSE(ids.empty()); + EXPECT_EQ(first_item->GetId(), ids[0]); +} + +// The page is in a loading state until it gets an insertItems call. Ensure that +// happens even without downloads. +TEST_F(DownloadsListTrackerTest, EmptyGetAllItemsStillCallsInsertItems) { + CreateTracker(); + + ASSERT_FALSE(tracker()->GetItemForTesting(0)); + ASSERT_TRUE(web_ui()->call_data().empty()); + + tracker()->Start(); + + ASSERT_FALSE(web_ui()->call_data().empty()); + EXPECT_EQ("downloads.Manager.insertItems", + web_ui()->call_data()[0]->function_name()); + ASSERT_TRUE(web_ui()->call_data()[0]->arg2()); + EXPECT_TRUE(GetIds(web_ui()->call_data()[0]->arg2()).empty()); +} + +TEST_F(DownloadsListTrackerTest, OnDownloadCreatedCallsInsertItems) { + CreateTracker(); + tracker()->Start(); + web_ui()->ClearTrackedCalls(); + + ASSERT_FALSE(tracker()->GetItemForTesting(0)); + DownloadItem* first_item = CreateNextItem(); + tracker()->OnDownloadCreated(manager(), first_item); + + ASSERT_FALSE(web_ui()->call_data().empty()); + EXPECT_EQ("downloads.Manager.insertItems", + web_ui()->call_data()[0]->function_name()); + EXPECT_EQ(0, GetIndex(web_ui()->call_data()[0]->arg1())); + + std::vector<uint64> ids = GetIds(web_ui()->call_data()[0]->arg2()); + ASSERT_FALSE(ids.empty()); + EXPECT_EQ(first_item->GetId(), ids[0]); +} + +TEST_F(DownloadsListTrackerTest, OnDownloadRemovedCallsRemoveItem) { + DownloadItem* first_item = CreateNextItem(); + + CreateTracker(); + tracker()->Start(); + web_ui()->ClearTrackedCalls(); + + EXPECT_TRUE(tracker()->GetItemForTesting(0)); + tracker()->OnDownloadRemoved(manager(), first_item); + EXPECT_FALSE(tracker()->GetItemForTesting(0)); + + ASSERT_FALSE(web_ui()->call_data().empty()); + + EXPECT_EQ("downloads.Manager.removeItem", + web_ui()->call_data()[0]->function_name()); + EXPECT_EQ(0, GetIndex(web_ui()->call_data()[0]->arg1())); +} + +TEST_F(DownloadsListTrackerTest, OnDownloadUpdatedCallsRemoveItem) { + DownloadItem* first_item = CreateNextItem(); + + CreateTracker(); + tracker()->Start(); + web_ui()->ClearTrackedCalls(); + + EXPECT_TRUE(tracker()->GetItemForTesting(0)); + + DownloadItemModel(first_item).SetShouldShowInShelf(false); + tracker()->OnDownloadUpdated(manager(), first_item); + + EXPECT_FALSE(tracker()->GetItemForTesting(0)); + + ASSERT_FALSE(web_ui()->call_data().empty()); + + EXPECT_EQ("downloads.Manager.removeItem", + web_ui()->call_data()[0]->function_name()); + EXPECT_EQ(0, GetIndex(web_ui()->call_data()[0]->arg1())); +} + +TEST_F(DownloadsListTrackerTest, StartExcludesHiddenItems) { + DownloadItem* first_item = CreateNextItem(); + DownloadItemModel(first_item).SetShouldShowInShelf(false); + + CreateTracker(); + tracker()->Start(); + + ASSERT_FALSE(web_ui()->call_data().empty()); + + EXPECT_EQ("downloads.Manager.insertItems", + web_ui()->call_data()[0]->function_name()); + EXPECT_TRUE(GetIds(web_ui()->call_data()[0]->arg2()).empty()); +}
diff --git a/chrome/browser/ui/webui/md_downloads/md_downloads_dom_handler.cc b/chrome/browser/ui/webui/md_downloads/md_downloads_dom_handler.cc index 403cbbf..39d3cc3f 100644 --- a/chrome/browser/ui/webui/md_downloads/md_downloads_dom_handler.cc +++ b/chrome/browser/ui/webui/md_downloads/md_downloads_dom_handler.cc
@@ -11,10 +11,7 @@ #include "base/bind.h" #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" #include "base/prefs/pref_service.h" #include "base/strings/string_number_conversions.h" @@ -22,10 +19,8 @@ #include "base/strings/utf_string_conversions.h" #include "base/supports_user_data.h" #include "base/threading/thread.h" -#include "base/value_conversions.h" #include "base/values.h" #include "chrome/browser/browser_process.h" -#include "chrome/browser/download/download_crx_util.h" #include "chrome/browser/download/download_danger_prompt.h" #include "chrome/browser/download/download_history.h" #include "chrome/browser/download/download_item_model.h" @@ -34,8 +29,6 @@ #include "chrome/browser/download/download_service.h" #include "chrome/browser/download/download_service_factory.h" #include "chrome/browser/download/drag_download_item.h" -#include "chrome/browser/extensions/api/downloads/downloads_api.h" -#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/platform_util.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/webui/fileicon_source.h" @@ -44,26 +37,20 @@ #include "chrome/common/url_constants.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/download_item.h" +#include "content/public/browser/download_manager.h" #include "content/public/browser/url_data_source.h" #include "content/public/browser/user_metrics.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_ui.h" -#include "extensions/browser/extension_system.h" #include "net/base/filename_util.h" -#include "third_party/icu/source/i18n/unicode/datefmt.h" #include "ui/base/l10n/time_format.h" #include "ui/gfx/image/image.h" using base::UserMetricsAction; -using content::BrowserContext; using content::BrowserThread; namespace { -// Maximum number of downloads to show. TODO(glen): Remove this and instead -// stuff the downloads down the pipe slowly. -size_t kMaxNumberOfDownloads = 150; - enum DownloadsDOMEvent { DOWNLOADS_DOM_EVENT_GET_DOWNLOADS = 0, DOWNLOADS_DOM_EVENT_OPEN_FILE = 1, @@ -86,203 +73,11 @@ DOWNLOADS_DOM_EVENT_MAX); } -// Returns a string constant to be used as the |danger_type| value in -// CreateDownloadItemValue(). Only return strings for DANGEROUS_FILE, -// DANGEROUS_URL, DANGEROUS_CONTENT, and UNCOMMON_CONTENT because the -// |danger_type| value is only defined if the value of |state| is |DANGEROUS|. -const char* GetDangerTypeString(content::DownloadDangerType danger_type) { - switch (danger_type) { - case content::DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE: - return "DANGEROUS_FILE"; - case content::DOWNLOAD_DANGER_TYPE_DANGEROUS_URL: - return "DANGEROUS_URL"; - case content::DOWNLOAD_DANGER_TYPE_DANGEROUS_CONTENT: - return "DANGEROUS_CONTENT"; - case content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT: - return "UNCOMMON_CONTENT"; - case content::DOWNLOAD_DANGER_TYPE_DANGEROUS_HOST: - return "DANGEROUS_HOST"; - case content::DOWNLOAD_DANGER_TYPE_POTENTIALLY_UNWANTED: - return "POTENTIALLY_UNWANTED"; - default: - // Don't return a danger type string if it is NOT_DANGEROUS or - // MAYBE_DANGEROUS_CONTENT. - NOTREACHED(); - return ""; - } -} - -// TODO(dbeam): if useful elsewhere, move to base/i18n/time_formatting.h? -base::string16 TimeFormatLongDate(const base::Time& time) { - scoped_ptr<icu::DateFormat> formatter( - icu::DateFormat::createDateInstance(icu::DateFormat::kLong)); - icu::UnicodeString date_string; - formatter->format(static_cast<UDate>(time.ToDoubleT() * 1000), date_string); - return base::string16(date_string.getBuffer(), - static_cast<size_t>(date_string.length())); -} - -// Returns a JSON dictionary containing some of the attributes of |download|. -// The JSON dictionary will also have a field "id" set to |id|, and a field -// "otr" set to |incognito|. -base::DictionaryValue* CreateDownloadItemValue( - content::DownloadItem* download_item, - bool incognito) { - // TODO(asanka): Move towards using download_model here for getting status and - // progress. The difference currently only matters to Drive downloads and - // those don't show up on the downloads page, but should. - DownloadItemModel download_model(download_item); - - // The items which are to be written into file_value are also described in - // chrome/browser/resources/downloads/downloads.js in @typedef for - // BackendDownloadObject. Please update it whenever you add or remove - // any keys in file_value. - base::DictionaryValue* file_value = new base::DictionaryValue(); - - file_value->SetInteger( - "started", static_cast<int>(download_item->GetStartTime().ToTimeT())); - file_value->SetString( - "since_string", ui::TimeFormat::RelativeDate( - download_item->GetStartTime(), NULL)); - - base::Time start_time = download_item->GetStartTime(); - base::string16 date_string = TimeFormatLongDate(start_time); - file_value->SetString("date_string", date_string); - - file_value->SetString("id", base::Uint64ToString(download_item->GetId())); - - base::FilePath download_path(download_item->GetTargetFilePath()); - file_value->Set("file_path", base::CreateFilePathValue(download_path)); - file_value->SetString("file_url", - net::FilePathToFileURL(download_path).spec()); - - extensions::DownloadedByExtension* by_ext = - extensions::DownloadedByExtension::Get(download_item); - std::string by_ext_id; - std::string by_ext_name; - if (by_ext) { - by_ext_id = by_ext->id(); - // TODO(dbeam): why doesn't DownloadsByExtension::name() return a string16? - by_ext_name = by_ext->name(); - - // Lookup the extension's current name() in case the user changed their - // language. This won't work if the extension was uninstalled, so the name - // might be the wrong language. - bool include_disabled = true; - const extensions::Extension* extension = extensions::ExtensionSystem::Get( - Profile::FromBrowserContext(download_item->GetBrowserContext()))-> - extension_service()->GetExtensionById(by_ext->id(), include_disabled); - if (extension) - file_value->SetString("by_ext_name", extension->name()); - } - file_value->SetString("by_ext_id", by_ext_id); - file_value->SetString("by_ext_name", by_ext_name); - - // Keep file names as LTR. - base::string16 file_name = - download_item->GetFileNameToReportUser().LossyDisplayName(); - file_name = base::i18n::GetDisplayStringInLTRDirectionality(file_name); - file_value->SetString("file_name", file_name); - file_value->SetString("url", download_item->GetURL().spec()); - file_value->SetBoolean("otr", incognito); - file_value->SetInteger("total", static_cast<int>( - download_item->GetTotalBytes())); - file_value->SetBoolean("file_externally_removed", - download_item->GetFileExternallyRemoved()); - file_value->SetBoolean("resume", download_item->CanResume()); - - const char* danger_type = ""; - base::string16 last_reason_text; - // -2 is invalid, -1 means indeterminate, and 0-100 are in-progress. - int percent = -2; - base::string16 progress_status_text; - bool retry = false; - const char* state = nullptr; - - switch (download_item->GetState()) { - case content::DownloadItem::IN_PROGRESS: { - if (download_item->IsDangerous()) { - state = "DANGEROUS"; - // These are the only danger states that the UI is equipped to handle. - DCHECK(download_item->GetDangerType() == - content::DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE || - download_item->GetDangerType() == - content::DOWNLOAD_DANGER_TYPE_DANGEROUS_URL || - download_item->GetDangerType() == - content::DOWNLOAD_DANGER_TYPE_DANGEROUS_CONTENT || - download_item->GetDangerType() == - content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT || - download_item->GetDangerType() == - content::DOWNLOAD_DANGER_TYPE_DANGEROUS_HOST || - download_item->GetDangerType() == - content::DOWNLOAD_DANGER_TYPE_POTENTIALLY_UNWANTED); - danger_type = GetDangerTypeString(download_item->GetDangerType()); - } else if (download_item->IsPaused()) { - state = "PAUSED"; - } else { - state = "IN_PROGRESS"; - } - progress_status_text = download_model.GetTabProgressStatusText(); - percent = download_item->PercentComplete(); - break; - } - - case content::DownloadItem::INTERRUPTED: - state = "INTERRUPTED"; - progress_status_text = download_model.GetTabProgressStatusText(); - - if (download_item->CanResume()) - percent = download_item->PercentComplete(); - - last_reason_text = download_model.GetInterruptReasonText(); - if (content::DOWNLOAD_INTERRUPT_REASON_CRASH == - download_item->GetLastReason() && !download_item->CanResume()) { - retry = true; - } - break; - - case content::DownloadItem::CANCELLED: - state = "CANCELLED"; - retry = true; - break; - - case content::DownloadItem::COMPLETE: - DCHECK(!download_item->IsDangerous()); - state = "COMPLETE"; - break; - - case content::DownloadItem::MAX_DOWNLOAD_STATE: - NOTREACHED(); - } - - DCHECK(state); - - file_value->SetString("danger_type", danger_type); - file_value->SetString("last_reason_text", last_reason_text); - file_value->SetInteger("percent", percent); - file_value->SetString("progress_status_text", progress_status_text); - file_value->SetBoolean("retry", retry); - file_value->SetString("state", state); - - return file_value; -} - -// Filters out extension downloads and downloads that don't have a filename yet. -bool IsDownloadDisplayable(const content::DownloadItem& item) { - return !download_crx_util::IsExtensionDownload(item) && - !item.IsTemporary() && - !item.GetFileNameToReportUser().empty() && - !item.GetTargetFilePath().empty() && - DownloadItemModel( - const_cast<content::DownloadItem*>(&item)).ShouldShowInShelf(); -} - } // namespace MdDownloadsDOMHandler::MdDownloadsDOMHandler( - content::DownloadManager* download_manager) - : download_manager_(download_manager), - update_scheduled_(false), + content::DownloadManager* download_manager, content::WebUI* web_ui) + : list_tracker_(download_manager, web_ui), weak_ptr_factory_(this) { // Create our fileicon data source. Profile* profile = Profile::FromBrowserContext( @@ -338,95 +133,19 @@ weak_ptr_factory_.GetWeakPtr())); } -void MdDownloadsDOMHandler::OnDownloadCreated( - content::DownloadManager* manager, content::DownloadItem* download_item) { - if (IsDownloadDisplayable(*download_item)) - ScheduleSendCurrentDownloads(); - else - new_downloads_.insert(download_item->GetId()); -} - -void MdDownloadsDOMHandler::OnDownloadUpdated( - content::DownloadManager* manager, - content::DownloadItem* download_item) { - if (update_scheduled_) - return; - - bool showing_new_item = false; - - if (new_downloads_.count(download_item->GetId())) { - // A new download (that the page doesn't know about yet) has been updated. - if (!IsDownloadDisplayable(*download_item)) { - // Item isn't ready to be displayed yet. Wait until it is. - return; - } - - new_downloads_.erase(download_item->GetId()); - showing_new_item = true; - } - - if (showing_new_item || DownloadItemModel(download_item).IsBeingRevived() || - !IsDownloadDisplayable(*download_item)) { - // 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; - } - - DCHECK(manager); - scoped_ptr<base::DictionaryValue> item(CreateDownloadItemValue( - download_item, - original_notifier_ && manager == GetMainNotifierManager())); - CallUpdateItem(*item); -} - -void MdDownloadsDOMHandler::OnDownloadRemoved( - content::DownloadManager* manager, - content::DownloadItem* download_item) { - if (!DownloadItemModel(download_item).ShouldShowInShelf()) - return; - - // This relies on |download_item| being removed from DownloadManager in this - // MessageLoop iteration. |download_item| may not have been removed from - // DownloadManager when OnDownloadRemoved() is fired, so bounce off the - // MessageLoop to give it a chance to be removed. SendCurrentDownloads() looks - // at all downloads, and we do not tell it that |download_item| is being - // removed. If DownloadManager is ever changed to not immediately remove - // |download_item| from its map when OnDownloadRemoved is sent, then - // MdDownloadsDOMHandler::OnDownloadRemoved() will need to explicitly tell - // SendCurrentDownloads() that |download_item| was removed. A - // SupportsUserData::Data would be the correct way to do this. - ScheduleSendCurrentDownloads(); +void MdDownloadsDOMHandler::RenderViewReused( + content::RenderViewHost* render_view_host) { + list_tracker_.Stop(); } void MdDownloadsDOMHandler::HandleGetDownloads(const base::ListValue* args) { CountDownloadsDOMEvents(DOWNLOADS_DOM_EVENT_GET_DOWNLOADS); - search_terms_.reset(args && !args->empty() ? args->DeepCopy() : NULL); - ScheduleSendCurrentDownloads(); - if (!main_notifier_) { - main_notifier_.reset(new AllDownloadItemNotifier(download_manager_, this)); + bool terms_changed = list_tracker_.SetSearchTerms(*args); + if (terms_changed) + list_tracker_.CallClearAll(); - Profile* profile = Profile::FromBrowserContext( - download_manager_->GetBrowserContext()); - if (profile->IsOffTheRecord()) { - original_notifier_.reset(new AllDownloadItemNotifier( - BrowserContext::GetDownloadManager(profile->GetOriginalProfile()), - this)); - } - } + list_tracker_.Start(); } void MdDownloadsDOMHandler::HandleOpenFile(const base::ListValue* args) { @@ -549,12 +268,17 @@ CountDownloadsDOMEvents(DOWNLOADS_DOM_EVENT_CLEAR_ALL); + list_tracker_.CallClearAll(); + list_tracker_.Stop(); + DownloadVector downloads; if (GetMainNotifierManager()) GetMainNotifierManager()->GetAllDownloads(&downloads); if (GetOriginalNotifierManager()) GetOriginalNotifierManager()->GetAllDownloads(&downloads); RemoveDownloads(downloads); + + list_tracker_.Start(); } void MdDownloadsDOMHandler::RemoveDownloads(const DownloadVector& to_remove) { @@ -590,29 +314,14 @@ // MdDownloadsDOMHandler, private: -------------------------------------------- -void MdDownloadsDOMHandler::ScheduleSendCurrentDownloads() { - // Don't call SendCurrentDownloads() every time anything changes. Batch them - // together instead. This may handle hundreds of OnDownloadDestroyed() calls - // 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(&MdDownloadsDOMHandler::SendCurrentDownloads, - weak_ptr_factory_.GetWeakPtr())); -} - content::DownloadManager* MdDownloadsDOMHandler::GetMainNotifierManager() const { - return main_notifier_ ? main_notifier_->GetManager() : nullptr; + return list_tracker_.GetMainNotifierManager(); } content::DownloadManager* MdDownloadsDOMHandler::GetOriginalNotifierManager() const { - return original_notifier_ ? original_notifier_->GetManager() : nullptr; + return list_tracker_.GetOriginalNotifierManager(); } void MdDownloadsDOMHandler::FinalizeRemovals() { @@ -628,37 +337,6 @@ } } -void MdDownloadsDOMHandler::SendCurrentDownloads() { - update_scheduled_ = false; - - content::DownloadManager::DownloadVector all_items, filtered_items; - if (GetMainNotifierManager()) { - GetMainNotifierManager()->GetAllDownloads(&all_items); - GetMainNotifierManager()->CheckForHistoryFilesRemoval(); - } - if (GetOriginalNotifierManager()) { - GetOriginalNotifierManager()->GetAllDownloads(&all_items); - GetOriginalNotifierManager()->CheckForHistoryFilesRemoval(); - } - - DownloadQuery query; - 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(kMaxNumberOfDownloads); - query.Search(all_items.begin(), all_items.end(), &filtered_items); - - base::ListValue results_value; - for (auto* item : filtered_items) { - results_value.Append(CreateDownloadItemValue( - item, - original_notifier_ && GetMainNotifierManager() && - GetMainNotifierManager()->GetDownload(item->GetId()) == item)); - } - CallUpdateAll(results_value); -} - void MdDownloadsDOMHandler::ShowDangerPrompt( content::DownloadItem* dangerous_item) { DownloadDangerPrompt* danger_prompt = DownloadDangerPrompt::Create( @@ -722,11 +400,3 @@ content::WebContents* MdDownloadsDOMHandler::GetWebUIWebContents() { return web_ui()->GetWebContents(); } - -void MdDownloadsDOMHandler::CallUpdateAll(const base::ListValue& list) { - web_ui()->CallJavascriptFunction("downloads.Manager.updateAll", list); -} - -void MdDownloadsDOMHandler::CallUpdateItem(const base::DictionaryValue& item) { - web_ui()->CallJavascriptFunction("downloads.Manager.updateItem", item); -}
diff --git a/chrome/browser/ui/webui/md_downloads/md_downloads_dom_handler.h b/chrome/browser/ui/webui/md_downloads/md_downloads_dom_handler.h index 0aee5da13..a724d6db 100644 --- a/chrome/browser/ui/webui/md_downloads/md_downloads_dom_handler.h +++ b/chrome/browser/ui/webui/md_downloads/md_downloads_dom_handler.h
@@ -8,13 +8,10 @@ #include <set> #include <vector> -#include "base/compiler_specific.h" #include "base/macros.h" #include "base/memory/weak_ptr.h" -#include "chrome/browser/download/all_download_item_notifier.h" #include "chrome/browser/download/download_danger_prompt.h" -#include "content/public/browser/download_item.h" -#include "content/public/browser/download_manager.h" +#include "chrome/browser/ui/webui/md_downloads/downloads_list_tracker.h" #include "content/public/browser/web_ui_message_handler.h" namespace base { @@ -22,29 +19,25 @@ } namespace content { +class DownloadItem; +class DownloadManager; +class RenderViewHost; class WebContents; +class WebUI; } // The handler for Javascript messages related to the "downloads" view, // also observes changes to the download manager. -class MdDownloadsDOMHandler : public content::WebUIMessageHandler, - public AllDownloadItemNotifier::Observer { +class MdDownloadsDOMHandler : public content::WebUIMessageHandler { public: - explicit MdDownloadsDOMHandler(content::DownloadManager* download_manager); + MdDownloadsDOMHandler(content::DownloadManager* download_manager, + content::WebUI* web_ui); ~MdDownloadsDOMHandler() override; - void Init(); - // WebUIMessageHandler implementation. void RegisterMessages() override; - // AllDownloadItemNotifier::Observer interface - void OnDownloadCreated(content::DownloadManager* manager, - content::DownloadItem* download_item) override; - void OnDownloadUpdated(content::DownloadManager* manager, - content::DownloadItem* download_item) override; - void OnDownloadRemoved(content::DownloadManager* manager, - content::DownloadItem* download_item) override; + void RenderViewReused(content::RenderViewHost* render_view_host); // Callback for the "getDownloads" message. void HandleGetDownloads(const base::ListValue* args); @@ -94,12 +87,6 @@ // depend on WebUI. The other methods that depend on WebUI are // RegisterMessages() and HandleDrag(). virtual content::WebContents* GetWebUIWebContents(); - virtual void CallUpdateAll(const base::ListValue& list); - virtual void CallUpdateItem(const base::DictionaryValue& item); - - // Schedules a call to SendCurrentDownloads() in the next message loop - // iteration. Protected rather than private for use in tests. - void ScheduleSendCurrentDownloads(); // Actually remove downloads with an ID in |removals_|. This cannot be undone. void FinalizeRemovals(); @@ -116,9 +103,6 @@ // null-checking |original_notifier_|. content::DownloadManager* GetOriginalNotifierManager() const; - // Sends the current list of downloads to the page. - void SendCurrentDownloads(); - // Displays a native prompt asking the user for confirmation after accepting // the dangerous download specified by |dangerous|. The function returns // immediately, and will invoke DangerPromptAccepted() asynchronously if the @@ -144,29 +128,11 @@ // Remove all downloads in |to_remove| with the ability to undo removal later. void RemoveDownloads(const DownloadVector& to_remove); - // Weak reference to the DownloadManager this class was constructed with. You - // should probably be using use Get{Main,Original}NotifierManager() instead. - content::DownloadManager* download_manager_; - - // Current search terms. - scoped_ptr<base::ListValue> search_terms_; - - // Notifies OnDownload*() and provides safe access to the DownloadManager. - scoped_ptr<AllDownloadItemNotifier> main_notifier_; - - // If |main_notifier_| observes an incognito profile, then this observes the - // DownloadManager for the original profile; otherwise, this is NULL. - scoped_ptr<AllDownloadItemNotifier> original_notifier_; + DownloadsListTracker list_tracker_; // IDs of downloads to remove when this handler gets deleted. std::vector<IdSet> removals_; - // Whether a call to SendCurrentDownloads() is currently scheduled. - bool update_scheduled_; - - // IDs of new downloads that the page doesn't know about yet. - IdSet new_downloads_; - base::WeakPtrFactory<MdDownloadsDOMHandler> weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(MdDownloadsDOMHandler);
diff --git a/chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc b/chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc index 3e8ec98e2..95d3a4e 100644 --- a/chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc +++ b/chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc
@@ -142,8 +142,8 @@ Profile* profile = Profile::FromWebUI(web_ui); DownloadManager* dlm = BrowserContext::GetDownloadManager(profile); - MdDownloadsDOMHandler* handler = new MdDownloadsDOMHandler(dlm); - web_ui->AddMessageHandler(handler); + handler_ = new MdDownloadsDOMHandler(dlm, web_ui); + web_ui->AddMessageHandler(handler_); // Set up the chrome://downloads/ source. content::WebUIDataSource* source = CreateDownloadsUIHTMLSource(profile); @@ -160,3 +160,8 @@ return ResourceBundle::GetSharedInstance(). LoadDataResourceBytesForScale(IDR_DOWNLOADS_FAVICON, scale_factor); } + +void MdDownloadsUI::RenderViewReused( + content::RenderViewHost* render_view_host) { + handler_->RenderViewReused(render_view_host); +}
diff --git a/chrome/browser/ui/webui/md_downloads/md_downloads_ui.h b/chrome/browser/ui/webui/md_downloads/md_downloads_ui.h index 37ce107..55d21e1d 100644 --- a/chrome/browser/ui/webui/md_downloads/md_downloads_ui.h +++ b/chrome/browser/ui/webui/md_downloads/md_downloads_ui.h
@@ -13,6 +13,12 @@ class RefCountedMemory; } +namespace content { +class RenderViewHost; +} + +class MdDownloadsDOMHandler; + class MdDownloadsUI : public content::WebUIController { public: explicit MdDownloadsUI(content::WebUI* web_ui); @@ -20,7 +26,12 @@ static base::RefCountedMemory* GetFaviconResourceBytes( ui::ScaleFactor scale_factor); + // content::WebUIController: + void RenderViewReused(content::RenderViewHost* render_view_host) override; + private: + MdDownloadsDOMHandler* handler_; // Weak. + DISALLOW_COPY_AND_ASSIGN(MdDownloadsUI); };
diff --git a/chrome/chrome_browser_ui.gypi b/chrome/chrome_browser_ui.gypi index b4f7f74..df312851 100644 --- a/chrome/chrome_browser_ui.gypi +++ b/chrome/chrome_browser_ui.gypi
@@ -1815,6 +1815,8 @@ 'browser/ui/webui/identity_internals_ui.h', 'browser/ui/webui/inspect_ui.cc', 'browser/ui/webui/inspect_ui.h', + 'browser/ui/webui/md_downloads/downloads_list_tracker.cc', + 'browser/ui/webui/md_downloads/downloads_list_tracher.h', 'browser/ui/webui/md_downloads/md_downloads_dom_handler.cc', 'browser/ui/webui/md_downloads/md_downloads_dom_handler.h', 'browser/ui/webui/md_downloads/md_downloads_ui.cc',
diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index efa77982..f64bf58 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi
@@ -1093,6 +1093,7 @@ 'browser/resources/google_now/common_test_util.js', 'browser/resources/google_now/utility.js', 'browser/resources/google_now/utility_test_util.js', + 'browser/resources/md_downloads/action_service.js', 'browser/resources/print_preview/data/measurement_system.js', 'browser/resources/print_preview/print_preview_utils.js', 'renderer/resources/extensions/notifications_custom_bindings.js', @@ -1103,6 +1104,7 @@ 'browser/resources/google_now/background_unittest.gtestjs', 'browser/resources/google_now/cards_unittest.gtestjs', 'browser/resources/google_now/utility_unittest.gtestjs', + 'browser/resources/md_downloads/action_service_unittest.js', 'browser/resources/print_preview/data/measurement_system_unittest.gtestjs', 'browser/resources/print_preview/print_preview_utils_unittest.gtestjs', 'renderer/resources/extensions/notifications_custom_bindings.gtestjs', @@ -1579,6 +1581,7 @@ 'browser/ui/website_settings/permission_menu_model_unittest.cc', 'browser/ui/webui/downloads_util_unittest.cc', 'browser/ui/webui/help/version_updater_chromeos_unittest.cc', + 'browser/ui/webui/md_downloads/downloads_list_tracker_unittest.cc', 'browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc', 'browser/ui/webui/options/autofill_options_handler_unittest.cc', 'browser/ui/webui/options/clear_browser_data_handler_unittest.cc',
diff --git a/chrome/test/data/webui/md_downloads/layout_tests.js b/chrome/test/data/webui/md_downloads/layout_tests.js index 7c09cf6..e319b582 100644 --- a/chrome/test/data/webui/md_downloads/layout_tests.js +++ b/chrome/test/data/webui/md_downloads/layout_tests.js
@@ -12,13 +12,14 @@ PolymerTest.clearBody(); manager = document.createElement('downloads-manager'); document.body.appendChild(manager); + assertEquals(manager, downloads.Manager.get()); }); test('long URLs ellide', function() { - manager.updateAll_([{ + downloads.Manager.insertItems(0, [{ file_name: 'file name', state: downloads.States.COMPLETE, - url: 'a'.repeat(10000), + url: 'a'.repeat(1000), }]); Polymer.dom.flush();
diff --git a/content/child/resource_dispatcher.cc b/content/child/resource_dispatcher.cc index 92a0962..3a04cb1 100644 --- a/content/child/resource_dispatcher.cc +++ b/content/child/resource_dispatcher.cc
@@ -209,9 +209,6 @@ return; } - // TODO(erikchen): Temporary debugging. http://crbug.com/527588. - CHECK_GE(shm_size, 0); - CHECK_LE(shm_size, 512 * 1024); request_info->buffer_size = shm_size; } @@ -227,14 +224,6 @@ CHECK(base::SharedMemory::IsHandleValid(request_info->buffer->handle())); CHECK_GE(request_info->buffer_size, data_offset + data_length); - // TODO(erikchen): Temporary debugging. http://crbug.com/527588. - CHECK_GE(request_info->buffer_size, 0); - CHECK_LE(request_info->buffer_size, 512 * 1024); - CHECK_GE(data_length, 0); - CHECK_LE(data_length, 512 * 1024); - CHECK_GE(data_offset, 0); - CHECK_LE(data_offset, 512 * 1024); - // Ensure that the SHM buffer remains valid for the duration of this scope. // It is possible for Cancel() to be called before we exit this scope. // SharedMemoryReceivedDataFactory stores the SHM buffer inside it.
diff --git a/ipc/attachment_broker_privileged.h b/ipc/attachment_broker_privileged.h index 5a55e67da..cbaaaf2 100644 --- a/ipc/attachment_broker_privileged.h +++ b/ipc/attachment_broker_privileged.h
@@ -79,6 +79,9 @@ ERROR_DECREASE_REF = 10, // Couldn't extract a right from the source. ERROR_EXTRACT_SOURCE_RIGHT = 11, + // The broker did not have a channel of communication with the source + // process. + ERROR_SOURCE_NOT_FOUND = 12, ERROR_MAX };
diff --git a/ipc/attachment_broker_privileged_mac.cc b/ipc/attachment_broker_privileged_mac.cc index 16dbff4..f4dc0439 100644 --- a/ipc/attachment_broker_privileged_mac.cc +++ b/ipc/attachment_broker_privileged_mac.cc
@@ -40,13 +40,18 @@ send_msg.data.disposition = disposition; send_msg.data.type = MACH_MSG_PORT_DESCRIPTOR; - return mach_msg(&send_msg.header, - MACH_SEND_MSG | MACH_SEND_TIMEOUT, - send_msg.header.msgh_size, - 0, // receive limit - MACH_PORT_NULL, // receive name - 0, // timeout - MACH_PORT_NULL); // notification port + kern_return_t kr = + mach_msg(&send_msg.header, MACH_SEND_MSG | MACH_SEND_TIMEOUT, + send_msg.header.msgh_size, + 0, // receive limit + MACH_PORT_NULL, // receive name + 0, // timeout + MACH_PORT_NULL); // notification port + + if (kr != KERN_SUCCESS) + mach_port_deallocate(mach_task_self(), endpoint); + + return kr; } } // namespace @@ -96,6 +101,36 @@ return false; } +void AttachmentBrokerPrivilegedMac::DeregisterCommunicationChannel( + Endpoint* endpoint) { + AttachmentBrokerPrivileged::DeregisterCommunicationChannel(endpoint); + + if (!endpoint) + return; + + base::ProcessId pid = endpoint->GetPeerPID(); + if (pid == base::kNullProcessId) + return; + + { + base::AutoLock l(precursors_lock_); + auto it = precursors_.find(pid); + if (it != precursors_.end()) { + delete it->second; + precursors_.erase(pid); + } + } + + { + base::AutoLock l(extractors_lock_); + auto it = extractors_.find(pid); + if (it != extractors_.end()) { + delete it->second; + extractors_.erase(pid); + } + } +} + bool AttachmentBrokerPrivilegedMac::OnMessageReceived(const Message& msg) { bool handled = true; switch (msg.type()) { @@ -169,7 +204,7 @@ HandleReceivedAttachment(attachment); } -void AttachmentBrokerPrivilegedMac::RouteWireFormatToAnother( +bool AttachmentBrokerPrivilegedMac::RouteWireFormatToAnother( const MachPortWireFormat& wire_format) { DCHECK_NE(wire_format.destination_process, base::Process::Current().Pid()); @@ -183,11 +218,12 @@ LOG(ERROR) << "Failed to deliver brokerable attachment to process with id: " << dest; LogError(DESTINATION_NOT_FOUND); - return; + return false; } LogError(DESTINATION_FOUND); sender->Send(new AttachmentBrokerMsg_MachPortHasBeenDuplicated(wire_format)); + return true; } mach_port_name_t AttachmentBrokerPrivilegedMac::CreateIntermediateMachPort( @@ -245,28 +281,13 @@ return endpoint; } -base::mac::ScopedMachSendRight AttachmentBrokerPrivilegedMac::AcquireSendRight( - base::ProcessId pid, - mach_port_name_t named_right) { - if (pid == base::GetCurrentProcId()) { - kern_return_t kr = mach_port_mod_refs(mach_task_self(), named_right, - MACH_PORT_RIGHT_SEND, 1); - if (kr != KERN_SUCCESS) - return base::mac::ScopedMachSendRight(MACH_PORT_NULL); - return base::mac::ScopedMachSendRight(named_right); - } - - mach_port_t task_port = port_provider_->TaskForPid(pid); - return ExtractNamedRight(task_port, named_right); -} - base::mac::ScopedMachSendRight AttachmentBrokerPrivilegedMac::ExtractNamedRight( mach_port_t task_port, mach_port_name_t named_right) { mach_port_t extracted_right = MACH_PORT_NULL; mach_msg_type_name_t extracted_right_type; kern_return_t kr = - mach_port_extract_right(task_port, named_right, MACH_MSG_TYPE_COPY_SEND, + mach_port_extract_right(task_port, named_right, MACH_MSG_TYPE_MOVE_SEND, &extracted_right, &extracted_right_type); if (kr != KERN_SUCCESS) { LogError(ERROR_EXTRACT_SOURCE_RIGHT); @@ -276,14 +297,6 @@ DCHECK_EQ(static_cast<mach_msg_type_name_t>(MACH_MSG_TYPE_PORT_SEND), extracted_right_type); - // Decrement the reference count of the send right from the source process. - kr = mach_port_mod_refs(task_port, named_right, MACH_PORT_RIGHT_SEND, -1); - if (kr != KERN_SUCCESS) { - LogError(ERROR_DECREASE_REF); - // Failure does not actually affect attachment brokering, so there's no need - // to return |MACH_PORT_NULL|. - } - return base::mac::ScopedMachSendRight(extracted_right); } @@ -305,16 +318,32 @@ // Whether this process is the destination process. bool to_self = pid == base::GetCurrentProcId(); + if (!to_self) { + if (!GetSenderWithProcessId(pid)) { + // If there is no sender, then the destination process is no longer + // running, or never existed to begin with. + LogError(DESTINATION_NOT_FOUND); + delete it->second; + precursors_.erase(it); + return; + } + } + mach_port_t task_port = port_provider_->TaskForPid(pid); + + // It's possible that the destination process has not yet provided the + // privileged process with its task port. if (!to_self && task_port == MACH_PORT_NULL) return; while (!it->second->empty()) { auto precursor_it = it->second->begin(); - if (to_self) + if (to_self) { RoutePrecursorToSelf(*precursor_it); - else - SendPrecursor(*precursor_it, task_port); + } else { + if (!SendPrecursor(*precursor_it, task_port)) + break; + } it->second->erase(precursor_it); } @@ -322,7 +351,7 @@ precursors_.erase(it); } -void AttachmentBrokerPrivilegedMac::SendPrecursor( +bool AttachmentBrokerPrivilegedMac::SendPrecursor( AttachmentPrecursor* precursor, mach_port_t task_port) { DCHECK(task_port); @@ -334,7 +363,8 @@ intermediate_port = CreateIntermediateMachPort( task_port, base::mac::ScopedMachSendRight(port_to_insert.release())); } - RouteWireFormatToAnother(CopyWireFormat(wire_format, intermediate_port)); + return RouteWireFormatToAnother( + CopyWireFormat(wire_format, intermediate_port)); } void AttachmentBrokerPrivilegedMac::AddPrecursor( @@ -357,7 +387,18 @@ if (it == extractors_.end()) return; + if (!GetSenderWithProcessId(pid)) { + // If there is no sender, then the source process is no longer running. + LogError(ERROR_SOURCE_NOT_FOUND); + delete it->second; + extractors_.erase(it); + return; + } + mach_port_t task_port = port_provider_->TaskForPid(pid); + + // It's possible that the source process has not yet provided the privileged + // process with its task port. if (task_port == MACH_PORT_NULL) return;
diff --git a/ipc/attachment_broker_privileged_mac.h b/ipc/attachment_broker_privileged_mac.h index 36abaf3..86bfb6f 100644 --- a/ipc/attachment_broker_privileged_mac.h +++ b/ipc/attachment_broker_privileged_mac.h
@@ -64,6 +64,7 @@ bool SendAttachmentToProcess( const scoped_refptr<IPC::BrokerableAttachment>& attachment, base::ProcessId destination_process) override; + void DeregisterCommunicationChannel(Endpoint* endpoint) override; // IPC::Listener overrides. bool OnMessageReceived(const Message& message) override; @@ -150,11 +151,6 @@ mach_port_t task_port, base::mac::ScopedMachSendRight port_to_insert); - // Acquire a send right to a named right in |pid|. - // Returns MACH_PORT_NULL on error. - base::mac::ScopedMachSendRight AcquireSendRight(base::ProcessId pid, - mach_port_name_t named_right); - // Extracts a copy of the send right to |named_right| from |task_port|. // Returns MACH_PORT_NULL on error. base::mac::ScopedMachSendRight ExtractNamedRight( @@ -176,14 +172,16 @@ // |wire_format.mach_port| must be the intermediate Mach port. // Ownership of |wire_format.mach_port| is implicitly passed to the process // that receives the Chrome IPC message. - void RouteWireFormatToAnother(const MachPortWireFormat& wire_format); + // Returns |false| on irrecoverable error. + bool RouteWireFormatToAnother(const MachPortWireFormat& wire_format); // Atempts to broker all precursors whose destination is |pid|. Has no effect // if |port_provider_| does not have the task port for |pid|. void SendPrecursorsForProcess(base::ProcessId pid); // Brokers a single precursor into the task represented by |task_port|. - void SendPrecursor(AttachmentPrecursor* precursor, mach_port_t task_port); + // Returns |false| on irrecoverable error. + bool SendPrecursor(AttachmentPrecursor* precursor, mach_port_t task_port); // Add a precursor to |precursors_|. Takes ownership of |port|. void AddPrecursor(base::ProcessId pid,
diff --git a/testing/buildbot/chromium.webkit.json b/testing/buildbot/chromium.webkit.json index 28aec6d..743911a0 100644 --- a/testing/buildbot/chromium.webkit.json +++ b/testing/buildbot/chromium.webkit.json
@@ -999,9 +999,6 @@ "test": "gpu_unittests" }, { - "test": "html_viewer_unittests" - }, - { "swarming": { "can_use_on_swarming_builders": true }, @@ -1023,9 +1020,6 @@ "test": "jingle_unittests" }, { - "test": "mash_unittests" - }, - { "swarming": { "can_use_on_swarming_builders": true }, @@ -1077,21 +1071,12 @@ "test": "mojo_public_utility_unittests" }, { - "test": "mojo_runner_host_unittests" - }, - { "test": "mojo_shell_unittests" }, { - "test": "mojo_surfaces_lib_unittests" - }, - { "test": "mojo_system_unittests" }, { - "test": "mojo_view_manager_lib_unittests" - }, - { "swarming": { "can_use_on_swarming_builders": true }, @@ -1119,9 +1104,6 @@ "test": "remoting_unittests" }, { - "test": "resource_provider_unittests" - }, - { "swarming": { "can_use_on_swarming_builders": true }, @@ -1185,9 +1167,6 @@ "test": "webkit_unit_tests" }, { - "test": "window_manager_unittests" - }, - { "swarming": { "can_use_on_swarming_builders": true },
diff --git a/third_party/libjingle/README.chromium b/third_party/libjingle/README.chromium index 335920d..2165d46 100644 --- a/third_party/libjingle/README.chromium +++ b/third_party/libjingle/README.chromium
@@ -1,7 +1,7 @@ Name: libjingle URL: http://www.webrtc.org Version: unknown -Revision: 10819 +Revision: 10833 License: BSD License File: source/talk/COPYING Security Critical: yes
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 012139a..186525c 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml
@@ -66322,6 +66322,9 @@ <int value="11" label="ERROR_EXTRACT_RIGHT_SOURCE"> Couldn't extract a right from the source process. </int> + <int value="12" label="ERROR_SOURCE_NOT_FOUND"> + Broker didn't have a channel of communication with the source process. + </int> </enum> <enum name="IPCAttachmentBrokerUnprivilegedBrokerAttachmentError" type="int">