Replace metadata if checks with an array.indexOf check.

Replace metadata if checks with an array.indexOf check.

BUG=#1137
R=fmeawad@chromium.org

Committed: https://chromium.googlesource.com/external/trace-viewer/+/975f1ff17f1383c132c64a3308c24d1b2848d1f2

[prabhur, 2015-07-27 17:40:10]

PTAL.

[nduca, 2015-07-27 17:43:42]

we don't reference chromium bug numbers, only github issue numbers

BUG=#374 for instance

[sullivan, 2015-07-27 17:43:50]

Fadi, can you take a look?

[nduca, 2015-07-27 17:44:13]

https://codereview.appspot.com/256200043/diff/1/tracing/tracing/extras/import...
File tracing/tracing/extras/importer/trace_event_importer.html (right):

https://codereview.appspot.com/256200043/diff/1/tracing/tracing/extras/import...
tracing/tracing/extras/importer/trace_event_importer.html:153: if
(knownFieldNames.indexOf(fieldName) > -1)
can you use a dict so this isn't a linear scan?

[fmeawad, 2015-07-27 17:59:46]

https://codereview.appspot.com/256200043/diff/1/tracing/tracing/extras/import...
File tracing/tracing/extras/importer/trace_event_importer.html (right):

https://codereview.appspot.com/256200043/diff/1/tracing/tracing/extras/import...
tracing/tracing/extras/importer/trace_event_importer.html:153: if
(knownFieldNames.indexOf(fieldName) > -1)
On 2015/07/27 17:44:12, nduca wrote:
> can you use a dict so this isn't a linear scan?

Give the size of the list, the dict is going to be 10% faster but uses more
memory. I am fine with either way.

[prabhur, 2015-07-27 18:11:27]

On 2015/07/27 17:59:46, fmeawad wrote:
>
https://codereview.appspot.com/256200043/diff/1/tracing/tracing/extras/import...
> File tracing/tracing/extras/importer/trace_event_importer.html (right):
> 
>
https://codereview.appspot.com/256200043/diff/1/tracing/tracing/extras/import...
> tracing/tracing/extras/importer/trace_event_importer.html:153: if
> (knownFieldNames.indexOf(fieldName) > -1)
> On 2015/07/27 17:44:12, nduca wrote:
> > can you use a dict so this isn't a linear scan?
> 
> Give the size of the list, the dict is going to be 10% faster but uses more
> memory. I am fine with either way.

If using a dict, it would be something like this:
var knownFieldNames = {traceEvents:true, systemTraceEvents:true, ... }
The value in the dict is pretty much useless - it is always set to 'true'.
I feel array would be much cleaner & concise (given its a very small array, perf
hit should be very small/negligible?) 
nduca@, let me know if you feel strongly about changing it to dict. I'm open to
either.

[nduca, 2015-07-27 18:23:13]

i'm fine either way just want rationale. lets have fadi decide and give you lg.
:)

[nduca, 2015-07-27 18:23:35]

your cl BUG= description is wrong still --- BUG=#1137

[prabhur, 2015-07-27 18:27:41]

On 2015/07/27 18:23:35, nduca wrote:
> your cl BUG= description is wrong still --- BUG=#1137

ok. updated BUG=. Thanks.

[fmeawad, 2015-07-27 18:39:38]

https://codereview.appspot.com/256200043/diff/1/tracing/tracing/extras/import...
File tracing/tracing/extras/importer/trace_event_importer.html (right):

https://codereview.appspot.com/256200043/diff/1/tracing/tracing/extras/import...
tracing/tracing/extras/importer/trace_event_importer.html:150: 'stackFrames',
'traceAnnotations', 'battorLogAsString'];
Can you sort them alphabetically, if sorted by # of occurrences please add a
comment.

[prabhur, 2015-07-27 18:56:39]

PTAL.

https://codereview.appspot.com/256200043/diff/1/tracing/tracing/extras/import...
File tracing/tracing/extras/importer/trace_event_importer.html (right):

https://codereview.appspot.com/256200043/diff/1/tracing/tracing/extras/import...
tracing/tracing/extras/importer/trace_event_importer.html:150: 'stackFrames',
'traceAnnotations', 'battorLogAsString'];
On 2015/07/27 18:39:38, fmeawad wrote:
> Can you sort them alphabetically, if sorted by # of occurrences please add a
> comment.

This is the original ordering of the if checks I'm replacing. I'm not 100%
certain if it was based on frequency of occurrences - but it does seem like it
(especially since traceEvents is the first one in the list).
Sorting it alphabetically moves 'traceEvents' to the last index and I believe
traceEvents would be more common than the other ones - so not good moving it to
the last. So keeping the ordering as before. Added a comment as you mentioned.

https://codereview.appspot.com/256200043/diff/1/tracing/tracing/extras/import...
tracing/tracing/extras/importer/trace_event_importer.html:153: if
(knownFieldNames.indexOf(fieldName) > -1)
On 2015/07/27 17:59:46, fmeawad wrote:
> On 2015/07/27 17:44:12, nduca wrote:
> > can you use a dict so this isn't a linear scan?
> 
> Give the size of the list, the dict is going to be 10% faster but uses more
> memory. I am fine with either way.

Acknowledged.

[fmeawad, 2015-07-27 19:02:22]

lgtm

[prabhur, 2015-07-27 19:05:07]

Committed patchset #2 (id:20001) manually as
975f1ff17f1383c132c64a3308c24d1b2848d1f2 (presubmit successful).