Refactor the way enhanced bookmark gets large icons

Refactor the way enhanced bookmark gets large icons

A LargeIconBridge will be created for every bookmark item shown on
screen, which is not necessary. Also previously we did not call
LargeIconBridge#destroy(), and it might cause memory leak issues.

This CL moves the icon retrieving logic to enhanced bookmark model,
a more approriate place for data query.

BUG=513509
Committed: https://crrev.com/f5e14e5fc4207c9df6da0ad17c180e4aeafabd50
Cr-Commit-Position: refs/heads/master@{#340362}

[Ian Wen, 2015-07-23 22:38:51]

ianwen@chromium.org changed reviewers:
+ kkimlabs@chromium.org, twellington@chromium.org

[Ian Wen, 2015-07-23 22:41:30]

https://codereview.chromium.org/1253783002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkRow.java
(right):

https://codereview.chromium.org/1253783002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkRow.java:57:
* @return The {@link BookmarkItem} corresponding the given {@link BookmarkId}.
Why returning a BookmarkItem? getBookmarkById traverses the bookmark model and
if overriding methods can reuse the same bookmarkItem, it saves some time.

[Kibeom Kim (inactive), 2015-07-24 06:12:50]

https://codereview.chromium.org/1253783002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkBookmarkRow.java
(right):

https://codereview.chromium.org/1253783002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkBookmarkRow.java:76:
BookmarkItem setBookmarkId(BookmarkId bookmarkId, EnhancedBookmarksModel model)
{
why not mDelegate.getModel()?

[Theresa, 2015-07-24 21:34:56]

https://codereview.chromium.org/1253783002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkBookmarkRow.java
(right):

https://codereview.chromium.org/1253783002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkBookmarkRow.java:76:
BookmarkItem setBookmarkId(BookmarkId bookmarkId, EnhancedBookmarksModel model)
{
On 2015/07/24 06:12:50, Kibeom Kim wrote:
> why not mDelegate.getModel()?

I echo this question. Also, if setBookmarkId takes a new parameter (in
EnhancedBookmarkRow specifically), lines 223 and 226 in
EnhancedBookmarkItemsAdapter.java will need to change.

Otherwise, looks good.

[Ian Wen, 2015-07-24 22:01:33]

Kibeom and Theresa:

Thanks for the comments. All done. The reason why I did not use
mDelegate.getModel() was that I thought it was a private attribute (in general
protected attributes are discouraged).

ptal.

[Theresa, 2015-07-24 22:03:12]

lgtm

[Kibeom Kim (inactive), 2015-07-24 22:04:56]

lgtm

[Ian Wen, 2015-07-24 22:09:08]

The CQ bit was checked by ianwen@chromium.org

[commit-bot: I haz the power, 2015-07-24 22:09:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1253783002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1253783002/20001

[commit-bot: I haz the power, 2015-07-24 22:50:09]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-07-24 22:51:01]

Patchset 2 (id:??) landed as
https://crrev.com/f5e14e5fc4207c9df6da0ad17c180e4aeafabd50
Cr-Commit-Position: refs/heads/master@{#340362}