Migrate child element manipulation, innerHTML, classList, attributes,

Migrate child element manipulation, innerHTML, classList, attributes,
        and miscellaneous others to use Polymer.dom.
Plus initial shadowRoot -> root migration.

BUG=catapult:#2285
NOTRY=true
NOPRESUBMIT=true

Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/2f870b177e34a7ff3fc55936bdf4fc1f24ef3566

[aiolos (Not reviewing), 2016-05-10 17:45:24]

Description was changed from

==========
Migrate child element manipulation, innerHTML, classList, attributes,
        and miscellaneous others to use Polymer.dom.
Plus initial shadowRoot -> root migration.

BUG=catapult:#2285
NOTRY=true
NOPRESUBMIT=true
==========

to

==========
Migrate child element manipulation, innerHTML, classList, attributes,
        and miscellaneous others to use Polymer.dom.
Plus initial shadowRoot -> root migration.

BUG=catapult:#2285
NOTRY=true
NOPRESUBMIT=true
==========

[aiolos (Not reviewing), 2016-05-10 17:45:24]

aiolos@chromium.org changed reviewers:
+ charliea@chromium.org

[aiolos (Not reviewing), 2016-05-10 17:52:46]

https://codereview.chromium.org/1967613003/diff/1/tracing/tracing/ui/timeline...
File tracing/tracing/ui/timeline_track_view.html (right):

https://codereview.chromium.org/1967613003/diff/1/tracing/tracing/ui/timeline...
tracing/tracing/ui/timeline_track_view.html:112: if (!this.modelTrack_)
whoops, I forgot to strip this. I'll remove.

[charliea (OOO until 10-5), 2016-05-10 18:53:18]

https://codereview.chromium.org/1967613003/diff/1/tracing/tracing/base/base.html
File tracing/tracing/base/base.html (right):

https://codereview.chromium.org/1967613003/diff/1/tracing/tracing/base/base.h...
tracing/tracing/base/base.html:125:
Polymer.dom(panicMessageEl).querySelector('#message').
nit (here and elswhere): we always break before the '.' in TV

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/base/un...
File tracing/tracing/base/unittest/html_test_results.html (right):

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/base/un...
tracing/tracing/base/unittest/html_test_results.html:167:
Polymer.dom(titleEl).innerHTML = '<a href="' + this.testCaseHRef_ + '">' +
nit: is this line too long?

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/anal...
File tracing/tracing/ui/analysis/single_cpu_slice_sub_view.html (right):

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/anal...
tracing/tracing/ui/analysis/single_cpu_slice_sub_view.html:107: var shadowRoot =
Polymer.dom(this).root;
Do we want Polymer.dom(this.root) here?

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/anal...
File tracing/tracing/ui/analysis/single_event_sub_view_test.html (right):

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/anal...
tracing/tracing/ui/analysis/single_event_sub_view_test.html:142:
assert.isDefined(Polymer.dom(e).nextSibling.children[0].stackFrame);
This might be a dumb question: do we need
Polymer.dom(Polymer.dom(e).nextSibling).children[0] here? It seems like it might
be dependent on the context: anything that might have either a shadow or shady
DOM below it needs Polymer.dom() wrapper around it. Is that right?

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/anal...
tracing/tracing/ui/analysis/single_event_sub_view_test.html:159:
assert.equal(Polymer.dom(eA).nextSibling.children[0].stackFrame.title,
I think preferred line break would be before Polymer.dom() and to keep
everything else on the same line together

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/anal...
tracing/tracing/ui/analysis/single_event_sub_view_test.html:166:
assert.equal(Polymer.dom(eB).nextSibling.children[0].stackFrame.title,
same as above

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/base...
File tracing/tracing/ui/base/mouse_mode_selector.html (right):

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/base...
tracing/tracing/ui/base/mouse_mode_selector.html:125: this.buttonsEl_ =
Polymer.dom(this.root).querySelector(
nit: can this fit on one line?

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/trac...
File tracing/tracing/ui/tracks/model_track.html (right):

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/trac...
tracing/tracing/ui/tracks/model_track.html:386: if (size === 0)
nit: need brackets around this if statement now that it's multiline

[charliea (OOO until 10-5), 2016-05-10 18:53:27]

lgtm w/ nits

[aiolos (Not reviewing), 2016-05-10 20:43:40]

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/base/un...
File tracing/tracing/base/unittest/html_test_results.html (right):

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/base/un...
tracing/tracing/base/unittest/html_test_results.html:167:
Polymer.dom(titleEl).innerHTML = '<a href="' + this.testCaseHRef_ + '">' +
On 2016/05/10 18:53:17, charliea wrote:
> nit: is this line too long?

oh, yes it is. I wonder why the presubmit didn't catch it.

Could you double check the format on the changed lines?

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/anal...
File tracing/tracing/ui/analysis/single_cpu_slice_sub_view.html (right):

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/anal...
tracing/tracing/ui/analysis/single_cpu_slice_sub_view.html:107: var shadowRoot =
Polymer.dom(this).root;
On 2016/05/10 18:53:17, charliea wrote:
> Do we want Polymer.dom(this.root) here?

Done.

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/anal...
File tracing/tracing/ui/analysis/single_event_sub_view_test.html (right):

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/anal...
tracing/tracing/ui/analysis/single_event_sub_view_test.html:142:
assert.isDefined(Polymer.dom(e).nextSibling.children[0].stackFrame);
On 2016/05/10 18:53:17, charliea wrote:
> This might be a dumb question: do we need
> Polymer.dom(Polymer.dom(e).nextSibling).children[0] here? It seems like it
might
> be dependent on the context: anything that might have either a shadow or shady
> DOM below it needs Polymer.dom() wrapper around it. Is that right?

That is a very good question. The Polymer.dom wrapper doesn't have a "children" 
accessor, but it does have a "childNodes" accessor. So my guess is we either
don't need to add the second Polymer.dom, or we need to also change children to
childNodes. I'd prefer to try it as-is and then change them all in a separate cl
if needed. (This isn't the only instance of children that I've skipped over for
this reason.)

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/anal...
tracing/tracing/ui/analysis/single_event_sub_view_test.html:159:
assert.equal(Polymer.dom(eA).nextSibling.children[0].stackFrame.title,
On 2016/05/10 18:53:17, charliea wrote:
> I think preferred line break would be before Polymer.dom() and to keep
> everything else on the same line together

Done.

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/anal...
tracing/tracing/ui/analysis/single_event_sub_view_test.html:166:
assert.equal(Polymer.dom(eB).nextSibling.children[0].stackFrame.title,
On 2016/05/10 18:53:17, charliea wrote:
> same as above

Done.

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/base...
File tracing/tracing/ui/base/mouse_mode_selector.html (right):

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/base...
tracing/tracing/ui/base/mouse_mode_selector.html:125: this.buttonsEl_ =
Polymer.dom(this.root).querySelector(
On 2016/05/10 18:53:18, charliea wrote:
> nit: can this fit on one line?

Done.

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/trac...
File tracing/tracing/ui/tracks/model_track.html (right):

https://codereview.chromium.org/1967613003/diff/20001/tracing/tracing/ui/trac...
tracing/tracing/ui/tracks/model_track.html:386: if (size === 0)
On 2016/05/10 18:53:18, charliea wrote:
> nit: need brackets around this if statement now that it's multiline

Done.

[charliea (OOO until 10-5), 2016-05-11 17:48:29]

lgtm w/ nit

https://codereview.chromium.org/1967613003/diff/40001/tracing/tracing/base/un...
File tracing/tracing/base/unittest/html_test_results.html (right):

https://codereview.chromium.org/1967613003/diff/40001/tracing/tracing/base/un...
tracing/tracing/base/unittest/html_test_results.html:167: Polymer.dom(titleEl).
I think: 

        Polymer.dom(titleEl).innerHTML =
            '<a href="' + this.testCaseHRef_ + '">' +
            this.testCase_.fullyQualifiedName + '</a>';

is the proper indentation

[aiolos (Not reviewing), 2016-05-11 19:17:08]

The CQ bit was checked by aiolos@chromium.org

[aiolos (Not reviewing), 2016-05-11 19:17:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
charliea@chromium.org
Link to the patchset: https://codereview.chromium.org/1967613003/#ps60001
(title: "white space fix.")

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

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

[commit-bot: I haz the power, 2016-05-11 19:17:41]

Description was changed from

==========
Migrate child element manipulation, innerHTML, classList, attributes,
        and miscellaneous others to use Polymer.dom.
Plus initial shadowRoot -> root migration.

BUG=catapult:#2285
NOTRY=true
NOPRESUBMIT=true
==========

to

==========
Migrate child element manipulation, innerHTML, classList, attributes,
        and miscellaneous others to use Polymer.dom.
Plus initial shadowRoot -> root migration.

BUG=catapult:#2285
NOTRY=true
NOPRESUBMIT=true

Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2016-05-11 19:17:42]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...