Factor and simplify generation of the lookup trie used for HTMLLookupTrie.

Factor and simplify generation of the lookup trie used for HTMLLookupTrie.

This is in preparation for making more classes use the lookup trie.

Difference before and after for HTMLElementLookupTrie.cpp is here:
https://www.diffchecker.com/lqhiueko

BUG=606695
Committed: https://crrev.com/28c11f51b7b91165ea4c108afceb1ac30e066c35
Cr-Commit-Position: refs/heads/master@{#393478}

[meade_UTC10, 2016-05-13 03:46:50]

Description was changed from

==========
Factor and simplify generation of the lookup trie used for HTMLLookupTrie.

This is in preparation for making more classes use the lookup trie.

BUG=606695
==========

to

==========
Factor and simplify generation of the lookup trie used for HTMLLookupTrie.

This is in preparation for making more classes use the lookup trie.

Difference before and after for HTMLElementLookupTrie.cpp is here:
https://www.diffchecker.com/ektrtewz

BUG=606695
==========

[meade_UTC10, 2016-05-13 03:47:22]

meade@chromium.org changed reviewers:
+ timloh@chromium.org

[Timothy Loh, 2016-05-13 04:49:17]

https://codereview.chromium.org/1978703002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/build/scripts/templates/macros.tmpl (right):

https://codereview.chromium.org/1978703002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/templates/macros.tmpl:32: {% macro
trie_switch(trie, index, return_macro, lowercase_data) %}
Do you think it would be clearer to merge the macros, something like as follows
(sorry for typing out a code block, thought it might be faster since you're away
soon):

(I also changed foo.items()[0][1] to foo.values()[0], the for-loop over 1 item
to just use set, and moved the break -- the other macro will need to write break
too)

{% if trie|length == 1 and trie.values()[0] is string %}
    {% set name, value = trie.items()[0] %}
    {% if name|length %}
if (
        {%- for c in name -%} 
            {%- if lowercase_data -%} 
    {{ " && " if not loop.first }}toASCIILower(data[{{index + loop.index0}}]) ==
'{{c}}'
            {%- else -%} 
    {{ " && " if not loop.first }}data[{{index + loop.index0}}] == '{{c}}'
            {%- endif -%} 
        {%- endfor -%} 
    )   
    return {{ return_macro(value) }}; 
break;
    {% else %}
return {{ return_macro(value) }}; 
    {% endif %}
{% else %}
{% if lowercase_data %}
switch (toASCIILower(data[{{index}}])) {
{% else %}
switch (data[{{index}}]) {
{% endif %}
{% for char, value in trie.items()|sort %}
case '{{char}}':
    {{ trie_switch(value, index + 1, return_macro, lowercase_data) | indent(4)
}}
    break;
{% endfor %}
}
{% endif %}
{% endmacro %}

https://codereview.chromium.org/1978703002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/templates/macros.tmpl:52: switch(length)
{
Space after switch

[Timothy Loh, 2016-05-13 05:41:34]

https://codereview.chromium.org/1978703002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/build/scripts/templates/macros.tmpl (right):

https://codereview.chromium.org/1978703002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/templates/macros.tmpl:51: {% macro
trie_length_switch(length_tries, return_macro, default_return_value,
lowercase_data) %}
default_return_value isn't used any more

[meade_UTC10, 2016-05-13 06:48:20]

https://codereview.chromium.org/1978703002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/build/scripts/templates/macros.tmpl (right):

https://codereview.chromium.org/1978703002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/templates/macros.tmpl:32: {% macro
trie_switch(trie, index, return_macro, lowercase_data) %}
On 2016/05/13 04:49:17, Timothy Loh wrote:
> Do you think it would be clearer to merge the macros, something like as
follows
> (sorry for typing out a code block, thought it might be faster since you're
away
> soon):
> 
> (I also changed foo.items()[0][1] to foo.values()[0], the for-loop over 1 item
> to just use set, and moved the break -- the other macro will need to write
break
> too)
> 
> {% if trie|length == 1 and trie.values()[0] is string %}
>     {% set name, value = trie.items()[0] %}
>     {% if name|length %}
> if (
>         {%- for c in name -%} 
>             {%- if lowercase_data -%} 
>     {{ " && " if not loop.first }}toASCIILower(data[{{index + loop.index0}}])
==
> '{{c}}'
>             {%- else -%} 
>     {{ " && " if not loop.first }}data[{{index + loop.index0}}] == '{{c}}'
>             {%- endif -%} 
>         {%- endfor -%} 
>     )   
>     return {{ return_macro(value) }}; 
> break;
>     {% else %}
> return {{ return_macro(value) }}; 
>     {% endif %}
> {% else %}
> {% if lowercase_data %}
> switch (toASCIILower(data[{{index}}])) {
> {% else %}
> switch (data[{{index}}]) {
> {% endif %}
> {% for char, value in trie.items()|sort %}
> case '{{char}}':
>     {{ trie_switch(value, index + 1, return_macro, lowercase_data) | indent(4)
> }}
>     break;
> {% endfor %}
> }
> {% endif %}
> {% endmacro %}

Hmm, I think it's clearer to have the trie_leaf macro, but I moved the check for
whether to call it up to the top of trie_switch. What do you think of this?

https://codereview.chromium.org/1978703002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/templates/macros.tmpl:51: {% macro
trie_length_switch(length_tries, return_macro, default_return_value,
lowercase_data) %}
On 2016/05/13 05:41:33, Timothy Loh wrote:
> default_return_value isn't used any more

Done.

https://codereview.chromium.org/1978703002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/templates/macros.tmpl:52: switch(length)
{
On 2016/05/13 04:49:17, Timothy Loh wrote:
> Space after switch

Done.

[Timothy Loh, 2016-05-13 06:53:52]

lgtm

I would prefer if break statements were paired with case statements, but I'll
let you decide on what you want. Right now the breaks look out of place to me
(ofc moving them means the generated code will have some unreachable breaks that
are right after returns).

[meade_UTC10, 2016-05-13 07:04:46]

Description was changed from

==========
Factor and simplify generation of the lookup trie used for HTMLLookupTrie.

This is in preparation for making more classes use the lookup trie.

Difference before and after for HTMLElementLookupTrie.cpp is here:
https://www.diffchecker.com/ektrtewz

BUG=606695
==========

to

==========
Factor and simplify generation of the lookup trie used for HTMLLookupTrie.

This is in preparation for making more classes use the lookup trie.

Difference before and after for HTMLElementLookupTrie.cpp is here:
https://www.diffchecker.com/lqhiueko

BUG=606695
==========

[meade_UTC10, 2016-05-13 07:06:00]

The CQ bit was checked by meade@chromium.org

[meade_UTC10, 2016-05-13 07:06:00]

The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org
Link to the patchset: https://codereview.chromium.org/1978703002/#ps60001
(title: "Remove newlines")

[commit-bot: I haz the power, 2016-05-13 07:06:11]

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

[commit-bot: I haz the power, 2016-05-13 08:40:06]

Description was changed from

==========
Factor and simplify generation of the lookup trie used for HTMLLookupTrie.

This is in preparation for making more classes use the lookup trie.

Difference before and after for HTMLElementLookupTrie.cpp is here:
https://www.diffchecker.com/lqhiueko

BUG=606695
==========

to

==========
Factor and simplify generation of the lookup trie used for HTMLLookupTrie.

This is in preparation for making more classes use the lookup trie.

Difference before and after for HTMLElementLookupTrie.cpp is here:
https://www.diffchecker.com/lqhiueko

BUG=606695
==========

[commit-bot: I haz the power, 2016-05-13 08:40:07]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-13 08:41:18]

Description was changed from

==========
Factor and simplify generation of the lookup trie used for HTMLLookupTrie.

This is in preparation for making more classes use the lookup trie.

Difference before and after for HTMLElementLookupTrie.cpp is here:
https://www.diffchecker.com/lqhiueko

BUG=606695
==========

to

==========
Factor and simplify generation of the lookup trie used for HTMLLookupTrie.

This is in preparation for making more classes use the lookup trie.

Difference before and after for HTMLElementLookupTrie.cpp is here:
https://www.diffchecker.com/lqhiueko

BUG=606695

Committed: https://crrev.com/28c11f51b7b91165ea4c108afceb1ac30e066c35
Cr-Commit-Position: refs/heads/master@{#393478}
==========

[commit-bot: I haz the power, 2016-05-13 08:41:19]

Patchset 4 (id:??) landed as
https://crrev.com/28c11f51b7b91165ea4c108afceb1ac30e066c35
Cr-Commit-Position: refs/heads/master@{#393478}