Fix ThumbnailDatabase::RetainDataForPageUrls() to drop unretained page mappings

Fix ThumbnailDatabase::RetainDataForPageUrls() to drop unretained page mappings

This CL makes always ThumbnailDatabase::RetainDataForPageUrls()  drop
Page URL -> Icon URL mappings for unretained page URLs.

Previously, a Page URL -> Icon URL (e.g. PageURL1 -> IconURLA) mapping was not
dropped for an unretained Page URL (PageURL1) if:
- the favicon data for a different page URL (e.g. PageURL2) was retained
AND
- PageURL2 is mapped to the same Icon URL as the unretained page URL
  (PageURL2 -> IconURLA)

BUG=468353
TEST=ThumbnailDatabaseTest.RetainDataForPageUrls

Committed: https://crrev.com/d1e44aa311e9bfb28cf1962dcc27a3b7a7a1244f
Cr-Commit-Position: refs/heads/master@{#321221}

[pkotwicz, 2015-03-18 17:37:23]

pkotwicz@chromium.org changed reviewers:
+ shess@chromium.org

[pkotwicz, 2015-03-18 17:39:34]

shess@ PTAL

[Scott Hess - ex-Googler, 2015-03-18 18:27:26]

https://codereview.chromium.org/1005783003/diff/1/components/history/core/bro...
File components/history/core/browser/thumbnail_database.cc (right):

https://codereview.chromium.org/1005783003/diff/1/components/history/core/bro...
components/history/core/browser/thumbnail_database.cc:1065:
statement.Reset(true);
Given this table, can the earlier loop be rewritten as an execute statement
like:
  INSERT OR IGNORE INTO temp.icon_id_mapping (old_icon_id) SELECT icon_id FROM
icon_mapping JOIN temp.retained_urls ON (temp.retained_urls.url =
icon_mapping.page_url)
That would make it harder to accidentally have these loops diverge.

https://codereview.chromium.org/1005783003/diff/1/components/history/core/bro...
components/history/core/browser/thumbnail_database.cc:1074: "FROM
temp.retained_urls AS retained_urls "
I don't think the AS adds much, since these lines aren't going to hit the
80-column limit AFAICT.  I think reflecting "This is our constructed temp table"
is worthwhile.

[pkotwicz, 2015-03-18 19:31:21]

shess@ Can you please take another look?

[pkotwicz, 2015-03-18 19:32:33]

Patchset #2 (id:20001) has been deleted

[Scott Hess - ex-Googler, 2015-03-18 19:35:35]

lgtm

[pkotwicz, 2015-03-18 20:00:44]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2015-03-18 20:00:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1005783003/40001

[pkotwicz, 2015-03-18 20:01:35]

The CQ bit was unchecked by pkotwicz@chromium.org

[pkotwicz, 2015-03-18 20:02:07]

pkotwicz@chromium.org changed reviewers:
+ sky@chromium.org

[pkotwicz, 2015-03-18 20:02:08]

sky@ for OWNERS

[sky, 2015-03-18 21:03:18]

LGTM

[pkotwicz, 2015-03-18 21:05:36]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2015-03-18 21:06:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1005783003/40001

[commit-bot: I haz the power, 2015-03-18 22:18:45]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2015-03-18 22:19:15]

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