MD Settings: Search engines, use favicon cache.

MD Settings: Search engines, use favicon cache.

BUG=606445
Committed: https://crrev.com/69e9c06e7d12ed26e5a33ede00fe605134e6093e
Cr-Commit-Position: refs/heads/master@{#390229}

[dpapad, 2016-04-25 22:12:20]

dpapad@chromium.org changed reviewers:
+ dbeam@chromium.org

[dpapad, 2016-04-25 22:12:21]

FYI, while working on this issue, I discovered that the old Options code
generates different chrome://favicon URLs than the shared code at util.js/html
(compared [1] and [2]). Surprisingly the favicon URLs generated by [1] work
sometimes but not always (even though I visited all involved pages to populate
the cache). Maybe worth filing as a separate bug.

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/ui/webui/resources...

[dpapad, 2016-04-26 21:14:07]

On 2016/04/25 at 22:12:21, dpapad wrote:
> FYI, while working on this issue, I discovered that the old Options code
generates different chrome://favicon URLs than the shared code at util.js/html
(compared [1] and [2]). Surprisingly the favicon URLs generated by [1] work
sometimes but not always (even though I visited all involved pages to populate
the cache). Maybe worth filing as a separate bug.
> 
> [1]
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> [2]
https://code.google.com/p/chromium/codesearch#chromium/src/ui/webui/resources...

Friendly ping.

[Dan Beam, 2016-04-26 22:33:15]

lgtm

https://codereview.chromium.org/1918053002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/search_engine_entry.html
(right):

https://codereview.chromium.org/1918053002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engine_entry.html:2:
<link rel="import" href="chrome://resources/html/util.html">
totally optional nit: maybe alpha if dependency order is agnostic?

[dpapad, 2016-04-26 22:43:26]

https://codereview.chromium.org/1918053002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/search_engine_entry.html
(right):

https://codereview.chromium.org/1918053002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engine_entry.html:2:
<link rel="import" href="chrome://resources/html/util.html">
On 2016/04/26 at 22:33:14, Dan Beam wrote:
> totally optional nit: maybe alpha if dependency order is agnostic?

Sorted, partially. I am still reluctant not placing polymer.html at the very
top, because of the logic at
https://code.google.com/p/chromium/codesearch#chromium/src/ui/webui/resources....

You might end up using the shady DOM if search_engine_entry.html was the only
HTML imported on a page (not the case for prod code, but could happen in testing
code).

[dpapad, 2016-04-26 23:11:53]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2016-04-26 23:11:54]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/1918053002/#ps20001
(title: "Nit")

[commit-bot: I haz the power, 2016-04-26 23:12:21]

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

[dpapad, 2016-04-26 23:35:11]

The CQ bit was unchecked by dpapad@chromium.org

[dpapad, 2016-04-27 22:13:17]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2016-04-27 22:13:17]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/1918053002/#ps40001
(title: "Handle missing iconURL case.")

[commit-bot: I haz the power, 2016-04-27 22:13:52]

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

[commit-bot: I haz the power, 2016-04-27 23:21:18]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-30 17:14:19]

Patchset 3 (id:??) landed as
https://crrev.com/69e9c06e7d12ed26e5a33ede00fe605134e6093e
Cr-Commit-Position: refs/heads/master@{#390229}

[commit-bot: I haz the power, 2016-04-30 17:52:41]

Description was changed from

==========
MD Settings: Search engines, use favicon cache.

BUG=606445
==========

to

==========
MD Settings: Search engines, use favicon cache.

BUG=606445

Committed: https://crrev.com/69e9c06e7d12ed26e5a33ede00fe605134e6093e
Cr-Commit-Position: refs/heads/master@{#390229}
==========