Cleanup: Extract icon related methods outside of util.js

Cleanup: Extract favicon related methods outside of util.js

util.js inevitably (because of its name) is becoming a dumping ground
for unrelated one-off helper methods.

Benefits of splitting:
 - Usage of cr.icon namespace instead of polluting global namespace.
 - cr.icon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.

CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

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

[dpapad, 2016-04-28 00:09:19]

Description was changed from

==========
Cleanup: Extract favicon related methods outside of util.js

BUG=
==========

to

==========
Cleanup: Extract favicon related methods outside of util.js

Benefits:
 - Usage of cr.favicon namespace instead of polluting global.
 - cr.favicon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.
==========

[dpapad, 2016-04-28 00:11:45]

Description was changed from

==========
Cleanup: Extract favicon related methods outside of util.js

Benefits:
 - Usage of cr.favicon namespace instead of polluting global.
 - cr.favicon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.
==========

to

==========
Cleanup: Extract favicon related methods outside of util.js

util.js inevitably (because of its name) is becoming a dumping ground
for unrelated code one-off helper methods.

Benefits of splitting:
 - Usage of cr.favicon namespace instead of polluting global.
 - cr.favicon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.
==========

[dpapad, 2016-04-28 00:29:41]

Description was changed from

==========
Cleanup: Extract favicon related methods outside of util.js

util.js inevitably (because of its name) is becoming a dumping ground
for unrelated code one-off helper methods.

Benefits of splitting:
 - Usage of cr.favicon namespace instead of polluting global.
 - cr.favicon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.
==========

to

==========
Cleanup: Extract favicon related methods outside of util.js

util.js inevitably (because of its name) is becoming a dumping ground
for unrelated code one-off helper methods.

Benefits of splitting:
 - Usage of cr.favicon namespace instead of polluting global namespace.
 - cr.favicon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.
==========

[dpapad, 2016-04-28 00:55:28]

Description was changed from

==========
Cleanup: Extract favicon related methods outside of util.js

util.js inevitably (because of its name) is becoming a dumping ground
for unrelated code one-off helper methods.

Benefits of splitting:
 - Usage of cr.favicon namespace instead of polluting global namespace.
 - cr.favicon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.
==========

to

==========
Cleanup: Extract favicon related methods outside of util.js

util.js inevitably (because of its name) is becoming a dumping ground
for unrelated one-off helper methods.

Benefits of splitting:
 - Usage of cr.favicon namespace instead of polluting global namespace.
 - cr.favicon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.
==========

[dpapad, 2016-04-28 00:59:12]

Patchset #1 (id:1) has been deleted

[dpapad, 2016-04-28 00:59:19]

Patchset #1 (id:20001) has been deleted

[dpapad, 2016-04-28 02:01:35]

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

[dpapad, 2016-04-28 02:01:36]

This is addressing the TODO that I added at
https://codereview.chromium.org/1914073005. The CL is smaller than it looks
(most files have a one line change).

[dpapad, 2016-04-29 22:33:10]

On 2016/04/28 at 02:01:36, dpapad wrote:
> This is addressing the TODO that I added at
https://codereview.chromium.org/1914073005. The CL is smaller than it looks
(most files have a one line change).

Friendly ping.

[dpapad, 2016-05-02 19:12:54]

On 2016/04/29 at 22:33:10, dpapad wrote:
> On 2016/04/28 at 02:01:36, dpapad wrote:
> > This is addressing the TODO that I added at
https://codereview.chromium.org/1914073005. The CL is smaller than it looks
(most files have a one line change).
> 
> Friendly ping.

FYI, I need to rebase this cleanup CL after the regression fix at
https://codereview.chromium.org/1938063002 lands.

[dpapad, 2016-05-02 19:27:04]

Description was changed from

==========
Cleanup: Extract favicon related methods outside of util.js

util.js inevitably (because of its name) is becoming a dumping ground
for unrelated one-off helper methods.

Benefits of splitting:
 - Usage of cr.favicon namespace instead of polluting global namespace.
 - cr.favicon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.
==========

to

==========
Cleanup: Extract favicon related methods outside of util.js

util.js inevitably (because of its name) is becoming a dumping ground
for unrelated one-off helper methods.

Benefits of splitting:
 - Usage of cr.favicon namespace instead of polluting global namespace.
 - cr.favicon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[dpapad, 2016-05-02 19:27:29]

On 2016/05/02 at 19:12:54, dpapad wrote:
> On 2016/04/29 at 22:33:10, dpapad wrote:
> > On 2016/04/28 at 02:01:36, dpapad wrote:
> > > This is addressing the TODO that I added at
https://codereview.chromium.org/1914073005. The CL is smaller than it looks
(most files have a one line change).
> > 
> > Friendly ping.
> 
> FYI, I need to rebase this cleanup CL after the regression fix at
https://codereview.chromium.org/1938063002 lands.

Rebase done.

[dpapad, 2016-05-03 23:14:08]

On 2016/05/02 at 19:27:29, dpapad wrote:
> On 2016/05/02 at 19:12:54, dpapad wrote:
> > On 2016/04/29 at 22:33:10, dpapad wrote:
> > > On 2016/04/28 at 02:01:36, dpapad wrote:
> > > > This is addressing the TODO that I added at
https://codereview.chromium.org/1914073005. The CL is smaller than it looks
(most files have a one line change).
> > > 
> > > Friendly ping.
> > 
> > FYI, I need to rebase this cleanup CL after the regression fix at
https://codereview.chromium.org/1938063002 lands.
> 
> Rebase done.

Friendly daily ping.
@dbeam: Can you take a look? I have not heard anything back for this CL in days.

[Dan Beam, 2016-05-04 02:06:58]

yeah, this seems fine but you should probably s/favicon/icon/g

[dpapad, 2016-05-04 17:27:26]

Patchset #4 (id:100001) has been deleted

[dpapad, 2016-05-04 17:28:34]

On 2016/05/04 at 02:06:58, dbeam wrote:
> yeah, this seems fine but you should probably s/favicon/icon/g

Done.

[dpapad, 2016-05-04 17:29:00]

Description was changed from

==========
Cleanup: Extract favicon related methods outside of util.js

util.js inevitably (because of its name) is becoming a dumping ground
for unrelated one-off helper methods.

Benefits of splitting:
 - Usage of cr.favicon namespace instead of polluting global namespace.
 - cr.favicon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Cleanup: Extract favicon related methods outside of util.js

util.js inevitably (because of its name) is becoming a dumping ground
for unrelated one-off helper methods.

Benefits of splitting:
 - Usage of cr.icon namespace instead of polluting global namespace.
 - cr.icon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[dpapad, 2016-05-04 17:35:08]

Description was changed from

==========
Cleanup: Extract favicon related methods outside of util.js

util.js inevitably (because of its name) is becoming a dumping ground
for unrelated one-off helper methods.

Benefits of splitting:
 - Usage of cr.icon namespace instead of polluting global namespace.
 - cr.icon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Cleanup: Extract favicon related methods outside of util.js

util.js inevitably (because of its name) is becoming a dumping ground
for unrelated one-off helper methods.

Benefits of splitting:
 - Usage of cr.icon namespace instead of polluting global namespace.
 - cr.icon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.

CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[dpapad, 2016-05-05 17:23:18]

On 2016/05/04 at 17:28:34, dpapad wrote:
> On 2016/05/04 at 02:06:58, dbeam wrote:
> > yeah, this seems fine but you should probably s/favicon/icon/g
> 
> Done.

Friendly ping.

[Dan Beam, 2016-05-06 02:50:51]

shouldn't favicon.html be named icon.html?

[dpapad, 2016-05-06 17:03:28]

On 2016/05/06 at 02:50:51, dbeam wrote:
> shouldn't favicon.html be named icon.html?

Done.

[Dan Beam, 2016-05-06 22:39:06]

lgtm w/requested fix

https://codereview.chromium.org/1919183005/diff/140001/chrome/test/data/webui...
File chrome/test/data/webui/webui_resource_browsertest.cc (right):

https://codereview.chromium.org/1919183005/diff/140001/chrome/test/data/webui...
chrome/test/data/webui/webui_resource_browsertest.cc:238:
IN_PROC_BROWSER_TEST_F(WebUIResourceBrowserTest, UtilTest) {
doesn't this need a:

  AddLibrary(IDR_WEBUI_JS_CR);

above it like previously?

https://codereview.chromium.org/1919183005/diff/140001/ui/webui/resources/js/...
File ui/webui/resources/js/util.js (right):

https://codereview.chromium.org/1919183005/diff/140001/ui/webui/resources/js/...
ui/webui/resources/js/util.js:295: cr.dispatchSimpleEvent(el,
'webkitTransitionEnd', true);
because ^

[dpapad, 2016-05-06 22:56:33]

https://codereview.chromium.org/1919183005/diff/140001/chrome/test/data/webui...
File chrome/test/data/webui/webui_resource_browsertest.cc (right):

https://codereview.chromium.org/1919183005/diff/140001/chrome/test/data/webui...
chrome/test/data/webui/webui_resource_browsertest.cc:238:
IN_PROC_BROWSER_TEST_F(WebUIResourceBrowserTest, UtilTest) {
On 2016/05/06 at 22:39:06, Dan Beam wrote:
> doesn't this need a:
> 
>   AddLibrary(IDR_WEBUI_JS_CR);
> 
> above it like previously?

Strictly speaking it is not needed. The non-favicon related tests of
util_test.html do not exercise any code that depends on cr.js. I had actually
added this line at
https://codereview.chromium.org/1914073005/diff/60001/chrome/test/data/webui/...,
when I added favicon related tests.

https://codereview.chromium.org/1919183005/diff/140001/ui/webui/resources/js/...
File ui/webui/resources/js/util.js (right):

https://codereview.chromium.org/1919183005/diff/140001/ui/webui/resources/js/...
ui/webui/resources/js/util.js:295: cr.dispatchSimpleEvent(el,
'webkitTransitionEnd', true);
On 2016/05/06 at 22:39:06, Dan Beam wrote:
> because ^

Expanding on previous comment, this file as a whole depends on cr.js, but the
the only method being tested is quoteString(), therefore util_test.html does not
depend on cr.js.

[Dan Beam, 2016-05-06 23:01:55]

okkkkkkkkk

https://codereview.chromium.org/1919183005/diff/140001/chrome/test/data/webui...
File chrome/test/data/webui/webui_resource_browsertest.cc (right):

https://codereview.chromium.org/1919183005/diff/140001/chrome/test/data/webui...
chrome/test/data/webui/webui_resource_browsertest.cc:238:
IN_PROC_BROWSER_TEST_F(WebUIResourceBrowserTest, UtilTest) {
On 2016/05/06 22:56:33, dpapad wrote:
> On 2016/05/06 at 22:39:06, Dan Beam wrote:
> > doesn't this need a:
> > 
> >   AddLibrary(IDR_WEBUI_JS_CR);
> > 
> > above it like previously?
> 
> Strictly speaking it is not needed. The non-favicon related tests of
> util_test.html do not exercise any code that depends on cr.js. I had actually
> added this line at
>
https://codereview.chromium.org/1914073005/diff/60001/chrome/test/data/webui/...,
> when I added favicon related tests.

Acknowledged.

[dpapad, 2016-05-09 17:08:53]

The CQ bit was checked by dpapad@chromium.org

[commit-bot: I haz the power, 2016-05-09 17:09:17]

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

[commit-bot: I haz the power, 2016-05-09 18:28:51]

Committed patchset #5 (id:140001)

[commit-bot: I haz the power, 2016-05-09 18:30:20]

Description was changed from

==========
Cleanup: Extract favicon related methods outside of util.js

util.js inevitably (because of its name) is becoming a dumping ground
for unrelated one-off helper methods.

Benefits of splitting:
 - Usage of cr.icon namespace instead of polluting global namespace.
 - cr.icon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.

CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Cleanup: Extract favicon related methods outside of util.js

util.js inevitably (because of its name) is becoming a dumping ground
for unrelated one-off helper methods.

Benefits of splitting:
 - Usage of cr.icon namespace instead of polluting global namespace.
 - cr.icon only exports public methods getProfileAvatarIcon and
   getFaviconImageSet, remaining methods/vars are kept private.

CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/d376bdc646067976df6c24a879e6e1f46919793a
Cr-Commit-Position: refs/heads/master@{#392372}
==========

[commit-bot: I haz the power, 2016-05-09 18:30:22]

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