Issue 22339004: Make an AddAll in ListModel.

Issue 22339004: Make an AddAll in ListModel. (Closed)

Created:
7 years, 4 months ago by calamity

Modified:
7 years, 1 month ago

Reviewers:
xiyuan, Ben Goodger (Google)

CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org, tfarina, tapted, koz (OOO until 15th September)

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Visibility:
Public.

More Reviews

Description

Make an AddAll in ListModel. This CL adds an AddAll to ListModel which notifies observers after all items have been added and changes AppModelBuilder to use it for bulk adding items. This is to facilitate the refactor in https://codereview.chromium.org/20656002/.

Patch Set 1 : #

Total comments: 6

Patch Set 2 : rework, add early return #

Created: 7 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+47 lines, -2 lines)			Patch
M	chrome/browser/ui/app_list/apps_model_builder.cc	View		1 chunk	+5 lines, -2 lines	0 comments	Download
M	ui/base/models/list_model.h	View	1	1 chunk	+13 lines, -0 lines	0 comments	Download
M	ui/base/models/list_model_unittest.cc	View		2 chunks	+29 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

xiyuan

LGTM https://codereview.chromium.org/22339004/diff/3001/ui/base/models/list_model.h File ui/base/models/list_model.h (right): https://codereview.chromium.org/22339004/diff/3001/ui/base/models/list_model.h#newcode40 ui/base/models/list_model.h:40: size_t count = item_count(); nit: const size_t https://codereview.chromium.org/22339004/diff/3001/ui/base/models/list_model.h#newcode45 ...

7 years, 4 months ago (2013-08-06 03:42:07 UTC) #2

calamity

7 years, 4 months ago (2013-08-07 07:19:40 UTC) #3

Also added early return on empty input vector.

+ben@ for ui/ OWNERS

https://codereview.chromium.org/22339004/diff/3001/ui/base/models/list_model.h
File ui/base/models/list_model.h (right):

https://codereview.chromium.org/22339004/diff/3001/ui/base/models/list_model....
ui/base/models/list_model.h:40: size_t count = item_count();
On 2013/08/06 03:42:07, xiyuan wrote:
> nit: const size_t

Done.

https://codereview.chromium.org/22339004/diff/3001/ui/base/models/list_model....
ui/base/models/list_model.h:45: new_items_released.end());
On 2013/08/06 03:42:07, xiyuan wrote:
> How about getting rid of |new_items_released| and do:
> 
> items_.insert(items_.end(), new_items.begin(), new_items.end());
> items_.weak_clear();

Done.

https://codereview.chromium.org/22339004/diff/3001/ui/base/models/list_model....
ui/base/models/list_model.h:49: 
On 2013/08/06 03:42:07, xiyuan wrote:
> nit: nuke one empty line

Done.

Expand Messages | Collapse Messages