WebUI: cr-search-field: Remove delegation pattern, use simple event instead.

WebUI: cr-search-field: Remove delegation pattern, use simple event instead.

CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/304e28f381898961316124e5319d580835e025de
Cr-Commit-Position: refs/heads/master@{#399995}

[dpapad, 2016-06-10 23:34:40]

Description was changed from

==========
cr-search-field: Remove delegation pattern, use simple event instead.
==========

to

==========
cr-search-field: Remove delegation pattern, use simple event instead.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[dpapad, 2016-06-10 23:49:14]

Description was changed from

==========
cr-search-field: Remove delegation pattern, use simple event instead.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
cr-search-field: Remove delegation pattern, use simple event instead.

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

[dpapad, 2016-06-11 00:11:01]

Description was changed from

==========
cr-search-field: Remove delegation pattern, use simple event instead.

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

to

==========
WebUI: cr-search-field: Remove delegation pattern, use simple event instead.

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

[dpapad, 2016-06-13 21:40:46]

Patchset #4 (id:60001) has been deleted

[dpapad, 2016-06-13 21:40:50]

Patchset #1 (id:1) has been deleted

[dpapad, 2016-06-13 21:40:54]

Patchset #1 (id:20001) has been deleted

[dpapad, 2016-06-13 21:40:57]

Patchset #1 (id:40001) has been deleted

[dpapad, 2016-06-13 21:42:02]

Patchset #1 (id:80001) has been deleted

[dpapad, 2016-06-13 21:52:36]

Patchset #2 (id:120001) has been deleted

[dpapad, 2016-06-13 21:52:39]

Patchset #1 (id:100001) has been deleted

[dpapad, 2016-06-14 18:46:42]

dpapad@chromium.org changed reviewers:
+ dbeam@chromium.org, rdevlin.cronin@chromium.org, tsergeant@chromium.org

[dpapad, 2016-06-14 18:46:43]

Green tests on patch3.

@dbeam: Please review md_downloads/
@rdevlin.cronin: Please review md_extensions/
@tsergeant: Please review md_history/, cr_search_field/

[Dan Beam, 2016-06-14 18:52:01]

you also need to run chrome/browser/resources/md_downloads/vulcanize.py

are you doing use_vulcanize=0 (or false in GN)?

https://codereview.chromium.org/2060023002/diff/200001/chrome/browser/resourc...
File chrome/browser/resources/md_downloads/toolbar.html (right):

https://codereview.chromium.org/2060023002/diff/200001/chrome/browser/resourc...
chrome/browser/resources/md_downloads/toolbar.html:29:
on-search-changed="onSearch_"></cr-search-field>
nit: onSearchChanged_

https://codereview.chromium.org/2060023002/diff/200001/chrome/browser/resourc...
File chrome/browser/resources/md_downloads/toolbar.js (right):

https://codereview.chromium.org/2060023002/diff/200001/chrome/browser/resourc...
chrome/browser/resources/md_downloads/toolbar.js:54: * @param {!Event} event
nit: CustomEvent

[dpapad, 2016-06-14 19:17:35]

https://codereview.chromium.org/2060023002/diff/200001/chrome/browser/resourc...
File chrome/browser/resources/md_downloads/toolbar.html (right):

https://codereview.chromium.org/2060023002/diff/200001/chrome/browser/resourc...
chrome/browser/resources/md_downloads/toolbar.html:29:
on-search-changed="onSearch_"></cr-search-field>
On 2016/06/14 at 18:52:01, Dan Beam wrote:
> nit: onSearchChanged_

Done.

https://codereview.chromium.org/2060023002/diff/200001/chrome/browser/resourc...
File chrome/browser/resources/md_downloads/toolbar.js (right):

https://codereview.chromium.org/2060023002/diff/200001/chrome/browser/resourc...
chrome/browser/resources/md_downloads/toolbar.js:54: * @param {!Event} event
On 2016/06/14 at 18:52:01, Dan Beam wrote:
> nit: CustomEvent

Done.

[dpapad, 2016-06-14 19:18:08]

On 2016/06/14 at 18:52:01, dbeam wrote:
> you also need to run chrome/browser/resources/md_downloads/vulcanize.py
Done.

> are you doing use_vulcanize=0 (or false in GN)?
Yes, using use_vulcanize=false.

[Devlin, 2016-06-14 19:19:33]

https://codereview.chromium.org/2060023002/diff/220001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/manager.js (right):

https://codereview.chromium.org/2060023002/diff/220001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/manager.js:98: * @param {!Event} event
!CustomEvent

https://codereview.chromium.org/2060023002/diff/220001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/manager.js:102: this.filter =
event.detail;
Also wrong before this, but I think this should be this.set('filter',
event.detail)

https://codereview.chromium.org/2060023002/diff/240001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/toolbar.js (left):

https://codereview.chromium.org/2060023002/diff/240001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/toolbar.js:9: /** @param
{SearchFieldDelegate} delegate */
Can we remove cr_search_field from compiled_resources2.gyp?

[dpapad, 2016-06-14 19:29:48]

https://codereview.chromium.org/2060023002/diff/220001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/manager.js (right):

https://codereview.chromium.org/2060023002/diff/220001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/manager.js:98: * @param {!Event} event
On 2016/06/14 at 19:19:32, Devlin wrote:
> !CustomEvent

Already done at patch 6.

https://codereview.chromium.org/2060023002/diff/220001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/manager.js:102: this.filter =
event.detail;
On 2016/06/14 at 19:19:32, Devlin wrote:
> Also wrong before this, but I think this should be this.set('filter',
event.detail)

My understanding is that direct assignment and usage of this.set() helper are
entirely equivalent in this case. The only time where these two are not
equivalent is when modifying a deep object, as follows

this.myObj.foo.bar = 'baz'; // does not trigger observers.
this.set('myObj.foo.bar', 'baz'); // triggers observers.

https://codereview.chromium.org/2060023002/diff/240001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/toolbar.js (left):

https://codereview.chromium.org/2060023002/diff/240001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/toolbar.js:9: /** @param
{SearchFieldDelegate} delegate */
On 2016/06/14 at 19:19:33, Devlin wrote:
> Can we remove cr_search_field from compiled_resources2.gyp?

Done.

[Devlin, 2016-06-14 20:07:45]

extensions lgtm

https://codereview.chromium.org/2060023002/diff/220001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/manager.js (right):

https://codereview.chromium.org/2060023002/diff/220001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/manager.js:102: this.filter =
event.detail;
On 2016/06/14 19:29:48, dpapad wrote:
> On 2016/06/14 at 19:19:32, Devlin wrote:
> > Also wrong before this, but I think this should be this.set('filter',
> event.detail)
> 
> My understanding is that direct assignment and usage of this.set() helper are
> entirely equivalent in this case. The only time where these two are not
> equivalent is when modifying a deep object, as follows
> 
> this.myObj.foo.bar = 'baz'; // does not trigger observers.
> this.set('myObj.foo.bar', 'baz'); // triggers observers.

Ah, I didn't realize that it was only for deep properties.  Good to know.

https://codereview.chromium.org/2060023002/diff/260001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/manager.js (right):

https://codereview.chromium.org/2060023002/diff/260001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/manager.js:102: this.filter = /** @type
{string} */ (event.detail);
nit: I think no space between */ and (event.detail), but I'll let Dan confirm.

[Dan Beam, 2016-06-14 22:11:20]

https://codereview.chromium.org/2060023002/diff/260001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/manager.js (right):

https://codereview.chromium.org/2060023002/diff/260001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/manager.js:102: this.filter = /** @type
{string} */ (event.detail);
On 2016/06/14 20:07:44, Devlin wrote:
> nit: I think no space between */ and (event.detail), but I'll let Dan confirm.

i think i originally discouraged this, but clang format might disagree with me

never cared enough to find out

[Dan Beam, 2016-06-14 22:13:29]

lgtm

https://codereview.chromium.org/2060023002/diff/260001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/toolbar.js (right):

https://codereview.chromium.org/2060023002/diff/260001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/toolbar.js:7: is: 'extensions-toolbar',
i wonder what the point of keeping this as a custom element is now?

[dpapad, 2016-06-14 22:28:16]

https://codereview.chromium.org/2060023002/diff/260001/chrome/browser/resourc...
File chrome/browser/resources/md_extensions/toolbar.js (right):

https://codereview.chromium.org/2060023002/diff/260001/chrome/browser/resourc...
chrome/browser/resources/md_extensions/toolbar.js:7: is: 'extensions-toolbar',
On 2016/06/14 at 22:13:29, Dan Beam wrote:
> i wonder what the point of keeping this as a custom element is now?

Don't see a point in keeping this custom element anymore. I prefer to do this
cleanup separately though.

[tsergeant, 2016-06-15 02:53:05]

lgtm

[dpapad, 2016-06-15 17:06:45]

The CQ bit was checked by dpapad@chromium.org

[commit-bot: I haz the power, 2016-06-15 17:07:22]

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

[commit-bot: I haz the power, 2016-06-15 20:15:44]

Committed patchset #7 (id:260001)

[commit-bot: I haz the power, 2016-06-15 20:17:09]

Description was changed from

==========
WebUI: cr-search-field: Remove delegation pattern, use simple event instead.

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

to

==========
WebUI: cr-search-field: Remove delegation pattern, use simple event instead.

CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/304e28f381898961316124e5319d580835e025de
Cr-Commit-Position: refs/heads/master@{#399995}
==========

[commit-bot: I haz the power, 2016-06-15 20:17:11]

Patchset 7 (id:??) landed as
https://crrev.com/304e28f381898961316124e5319d580835e025de
Cr-Commit-Position: refs/heads/master@{#399995}