Issue 11419169: Use DownloadItemModel for storing chrome/ specific UI data for DownloadItems.

Issue 11419169: Use DownloadItemModel for storing chrome/ specific UI data for DownloadItems. (Closed)

Created:
8 years ago by asanka

Modified:
8 years ago

Reviewers:
Randy Smith (Not in Mondays), sky

CC:
chromium-reviews, gbillock+watch_chromium.org, smckay+watch_chromium.org, groby+watch_chromium.org, rouslan+watch_chromium.org, rdsmith+dwatch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Use DownloadItemModel for storing chrome/ specific UI data for DownloadItems. BUG=116551 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=172178

Patch Set 2 : @170231 picking up changes to download_item_gtk #

Patch Set 3 : Merge with r171563 to pick up updates to download_item_model.* #

Patch Set 4 : Keep DisplayName in content/ #

Patch Set 5 : Fix tests #

Total comments: 4

Created: 8 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+153 lines, -101 lines)			Patch
M	chrome/browser/download/download_browsertest.cc	View	1 2	2 chunks	+2 lines, -1 line	0 comments	Download
M	chrome/browser/download/download_item_model.h	View	1 2 3	3 chunks	+22 lines, -14 lines	0 comments	Download
M	chrome/browser/download/download_item_model.cc	View	1 2 3	7 chunks	+91 lines, -26 lines	2 comments	Download
M	chrome/browser/download/download_item_model_unittest.cc	View	1 2 3 4	5 chunks	+33 lines, -25 lines	0 comments	Download
M	chrome/browser/download/download_util.h	View	1 2 3	1 chunk	+0 lines, -6 lines	0 comments	Download
M	chrome/browser/download/download_util.cc	View	1 2 3	2 chunks	+0 lines, -25 lines	0 comments	Download
M	chrome/browser/ui/browser.cc	View	1 2	2 chunks	+3 lines, -2 lines	2 comments	Download
M	chrome/browser/ui/intents/web_intent_picker_controller.cc	View	1 2	2 chunks	+2 lines, -2 lines	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

Randy Smith (Not in Mondays)

LGTM. https://codereview.chromium.org/11419169/diff/14001/chrome/browser/download/download_item_model.cc File chrome/browser/download/download_item_model.cc (right): https://codereview.chromium.org/11419169/diff/14001/chrome/browser/download/download_item_model.cc#newcode346 chrome/browser/download/download_item_model.cc:346: // instead of calling into the DownloadItem. Despite ...

8 years ago (2012-12-08 19:37:16 UTC) #2

asanka

+sky: Could you take a look at chrome/browser/ui? https://codereview.chromium.org/11419169/diff/14001/chrome/browser/download/download_item_model.cc File chrome/browser/download/download_item_model.cc (right): https://codereview.chromium.org/11419169/diff/14001/chrome/browser/download/download_item_model.cc#newcode346 chrome/browser/download/download_item_model.cc:346: // ...

8 years ago (2012-12-10 16:30:35 UTC) #3

sky

LGTM https://codereview.chromium.org/11419169/diff/14001/chrome/browser/ui/browser.cc File chrome/browser/ui/browser.cc (right): https://codereview.chromium.org/11419169/diff/14001/chrome/browser/ui/browser.cc#newcode1498 chrome/browser/ui/browser.cc:1498: shelf->AddDownload(download_model.release()); Would be nice to use download_model.Pass() here. ...

8 years ago (2012-12-10 18:04:51 UTC) #4

asanka

Thanks! https://codereview.chromium.org/11419169/diff/14001/chrome/browser/ui/browser.cc File chrome/browser/ui/browser.cc (right): https://codereview.chromium.org/11419169/diff/14001/chrome/browser/ui/browser.cc#newcode1498 chrome/browser/ui/browser.cc:1498: shelf->AddDownload(download_model.release()); On 2012/12/10 18:04:52, sky wrote: > Would ...

8 years ago (2012-12-10 18:52:14 UTC) #5

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/asanka@chromium.org/11419169/14001

8 years ago (2012-12-10 21:25:59 UTC) #6

Message was sent while issue was closed.

Change committed as 172178

Expand Messages | Collapse Messages