For non-editable content, selection editor commands are disabled

Editor commands are disabled on non-editable content. The execution of command
    relies on following factors. i.e either caret browsing is on/the content is
    editable provided selection type is caret/the selection type is range. Due to
    the last factor, keyboard selection works on top of an existing selection
    (done by mouse) on non-editable content. So, to make the selection possible
    when there is no existing selection, enabled the command execution by adding
    new enabled* method with all the above constraint without content editable
    check.
    
    BUG=663

[Inactive, 2014-05-13 19:28:51]

Please wrap your CL description at ~80 chars.

This change needs to be covered by a test.

FYI, adding 8 reviewers for such a tiny patch seems a bit overkill. In the
future, please add a more reasonable number of reviewers. If you're not sure who
is a good reviewer for a particular change, it is a good idea to check who made
changes to the surrounding code recently and who reviewed these changes.

https://codereview.chromium.org/268993002/diff/20001/Source/core/editing/Edit...
File Source/core/editing/EditorCommand.cpp (right):

https://codereview.chromium.org/268993002/diff/20001/Source/core/editing/Edit...
Source/core/editing/EditorCommand.cpp:1441: { "AlignCenter", { 139,
executeJustifyCenter, supportedFromMenuOrKeyBinding,
enabledInRichlyEditableText, stateNone, valueNull, notTextInsertion,
doNotAllowExecutionWhenDisabled } },
What is this change? looks unrelated.

https://codereview.chromium.org/268993002/diff/20001/Source/core/page/EventHa...
File Source/core/page/EventHandler.cpp (right):

https://codereview.chromium.org/268993002/diff/20001/Source/core/page/EventHa...
Source/core/page/EventHandler.cpp:770: // for non-ediatble texts.
"non-editable"

[tkent, 2014-05-13 22:18:09]

-abarth, krit, acolwell
+yosin, yutak

[b.rout, 2014-05-14 06:59:45]

I was trying to run the layout tests. But, it was giving content_shell crashed
for all tests even without my changes. But launching content_shell alone was
working.
Is there any other way I could run the tests and identify what is broken?

Next time onwards, I will take care of adding reasonable no. of reviewers.

[b.rout, 2014-05-14 07:00:46]

https://codereview.chromium.org/268993002/diff/20001/Source/core/editing/Edit...
File Source/core/editing/EditorCommand.cpp (right):

https://codereview.chromium.org/268993002/diff/20001/Source/core/editing/Edit...
Source/core/editing/EditorCommand.cpp:1441: { "AlignCenter", { 139,
executeJustifyCenter, supportedFromMenuOrKeyBinding,
enabledInRichlyEditableText, stateNone, valueNull, notTextInsertion,
doNotAllowExecutionWhenDisabled } },
Due to "git cl format" which was asked while submitting the 2nd patch, its
showing some unrelated changes.

The actual changes lie in the line no# 1497,1501,1511,1515,1523,1533,1539,1541
where the last flag is modified from doNotAllowExecutionWhenDisabled to
allowExecutionWhenDisabled.

On 2014/05/13 19:28:52, Chris Dumez wrote:
> What is this change? looks unrelated.

https://codereview.chromium.org/268993002/diff/20001/Source/core/page/EventHa...
File Source/core/page/EventHandler.cpp (right):

https://codereview.chromium.org/268993002/diff/20001/Source/core/page/EventHa...
Source/core/page/EventHandler.cpp:770: // for non-ediatble texts.
will correct it. Thanks.

On 2014/05/13 19:28:52, Chris Dumez wrote:
> "non-editable"

[Yuta Kitamura, 2014-05-20 09:11:33]

I'm not sure I understand the issue here. A test is
definitely desired for the reviewers to understand
the issue and to confirm you are making a right change.

I don't know what happened to your local builds, but usually
the layout tests should (mostly) pass without any special
trick.

To manually run a layout test, you can run the following
command (for example):

$ content_shell --dump-render-tree
/path/to/LayoutTests/fast/js/array-foreach.html

... and it should print something that looks similar to
array-foreach-expected.txt. You can attach your debugger
to content_shell to see what's happening behind the crashes.

[b.rout, 2014-05-20 12:21:17]

Thanks for confirming.
Currently, I am in the process of writing layout tests. Since I am unable to
verify the tests in chromium at present, trying to verify it in webkit repo.
Once done, I will upload the patch and could cross-verify with the trybot. 
Probably could try to do a fresh set-up separately to run it using
content_shell.

[b.rout, 2014-05-22 15:10:53]

I have uploaded the 3rd patch with the layout tests. But, all these tests I have
run on webkit repository. So, need to be tried on trybots. I encountered some
problem while writing the layout tests which is why somehow I feel its still
incomplete. So, I want to clarify some of my doubts.

1. In selection-in-paragraph-using-keys-with-caret-mode-off.html I have written
one test where shift + home/end selects the texts from the caret position to the
beginning of a line and end of a line respectively. I wanted to write it in a
way where I wanted to do these operations one after the other so that it give me
the expected result.
For ex: if I press shift + home followed by shift + end, the selection is
expected to be from the caret position (before home + shift pressed) till the
end of the line as if only shift + end is pressed, which is happening when I do
it manually. But, when I run test case I see its selecting the whole line after
both the operations. Is there any limitation in the simulator of these mouse and
keyboard events ?

2. In place-caret-on-single-click-within-selected-readonly-text.html I tried to
test for the caret placement when mouse clicked once within any existing
selection. Here I initially tried to do some selection with mouse events and
then click the mouse within the selection followed by keyboard selection which
should have been allowed. Here again I felt some sort of limitation where
mouseMoveTo()->mouseDown()->mouseUp() should be doing the job same as
getSelection().setPosition() ultimately. But, I didn't notice it.
So, I had to drop my way of writing the test. Then I had to adopt render tree
visualization way which I hardly understand at this point. I have no idea how
could I ensure the renderblock dimensions and positions of elements are correct.
So, I assumed the actual result is correct. 

Again by looking at other such layout tests it seems its platform specific. Due
to system limitation, I don't know what should be the expected values in other
platforms like mac. Is there any way I could build it for platform specific
ports?

3. foreignObject-repaint.xml test case is breaking since earlier it wasn't
allowing caret to be placed when mouse is clicked within any existing selection
for readonly texts, which I have lifted to enable further selection after a
single click within any existing selection so that we don't have to wait for
another mouse click to start our selection. So, I have added extra line which
was appearing in the actual result. But, I am not sure if its correct when the
position value is concerned. 

I need some help here to clear all my doubts to write the tests in a better way.

[Yuta Kitamura, 2014-05-23 08:07:36]

I kicked off try jobs. Let me take a close look after these jobs
finish.

[Yuta Kitamura, 2014-05-26 06:15:16]

On 2014/05/23 08:07:36, Yuta Kitamura wrote:
> I kicked off try jobs. Let me take a close look after these jobs
> finish.

Sounds like these tests have failed on the trybots?

[b.rout, 2014-05-28 06:18:46]

> Sounds like these tests have failed on the trybots?

Now, my content_shell issue is resolved. So, I am able to run layout tests. 
I have revised the tests and verified. Could you please run trybots for the
tests again.

[b.rout, 2014-05-29 14:16:45]

updated the layout tests by checking the behavior in mac.

[b.rout, 2014-06-02 09:17:18]

Could someone please kick off the trybots on the last patch.

[ojan, 2014-06-02 19:28:26]

On 2014/06/02 09:17:18, b.rout wrote:
> Could someone please kick off the trybots on the last patch.

Done.

[Yuta Kitamura, 2014-06-03 06:47:14]

Seems like your patch doesn't apply to the ToT. Please
rebase your patch and upload again.

I tried to understand what the patch is trying to do, but
I couldn't reach the conclution if your changes on C++ files
were legitimate. Your changes to EditorCommand.cpp
definitely make Blink behave differently, but I could not
judge if this difference was okay or not.

Also, it was hard for me to interpret the change
description, so I suggest rewriting the change description
with the following points in mind:

* What's wrong with the current code,
* How your patch fixes the issue, and
* Rationale why your change is legit (e.g. why your change
  will not break existing websites, etc.).

[b.rout, 2014-06-03 06:49:35]

It resulted in some apply_issue error. Is it something I need to check? Kindly
help me.

[b.rout, 2014-06-03 12:12:43]

On 2014/06/03 06:47:14, Yuta Kitamura wrote:
> Seems like your patch doesn't apply to the ToT. Please
> rebase your patch and upload again.

I updated local repo and uploaded the patch again. 

> I tried to understand what the patch is trying to do, but
> I couldn't reach the conclution if your changes on C++ files
> were legitimate. Your changes to EditorCommand.cpp
> definitely make Blink behave differently, but I could not
> judge if this difference was okay or not.

I am just trying to allow selection command execution by setting
allowExecutionWhenDisabled flag. Since selection is not a modification to
content, I feel there is no harm to set it for this operation. 

> Also, it was hard for me to interpret the change
> description, so I suggest rewriting the change description
> with the following points in mind:
> 
> * What's wrong with the current code,
> * How your patch fixes the issue, and
> * Rationale why your change is legit (e.g. why your change
>   will not break existing websites, etc.).

Changed the description. Hope, its bit more understandable now.

[Yuta Kitamura, 2014-06-04 08:19:39]

Okay, I've got the idea of what this patch is trying to do.
But I have a few questions on the approach (see below).

https://codereview.chromium.org/268993002/diff/100001/LayoutTests/editing/sel...
File
LayoutTests/editing/selection/place-caret-on-single-click-within-selected-readonly-text.html
(right):

https://codereview.chromium.org/268993002/diff/100001/LayoutTests/editing/sel...
LayoutTests/editing/selection/place-caret-on-single-click-within-selected-readonly-text.html:4:
<p>This tests caret placement when clicked once on existing selection in
readonly texts to allow further selection using keys.</p>
You can use description() function from js-test to show
the test description.

https://codereview.chromium.org/268993002/diff/100001/LayoutTests/editing/sel...
LayoutTests/editing/selection/place-caret-on-single-click-within-selected-readonly-text.html:5:
<div id="console"></div>
js-test automatically creates the element for output console,
so you don't have to write your own one.

https://codereview.chromium.org/268993002/diff/100001/LayoutTests/editing/sel...
LayoutTests/editing/selection/place-caret-on-single-click-within-selected-readonly-text.html:8:
if (window.testRunner) {
As a general rule, it is recommended to make the test
runnable on the normal browsers that do not expose
test-only objects.

If your test requires manual operations (like clicking
in this test), it is encouraged to put the instructions
of how to run the test manually, so people can run the
test in their browsers.

https://codereview.chromium.org/268993002/diff/100001/LayoutTests/editing/sel...
File
LayoutTests/editing/selection/select-by-char-and-word-in-readonly-texts-using-keys-with-caret-mode-off.html
(right):

https://codereview.chromium.org/268993002/diff/100001/LayoutTests/editing/sel...
LayoutTests/editing/selection/select-by-char-and-word-in-readonly-texts-using-keys-with-caret-mode-off.html:1:
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
Use HTML5 doctype instead of this legacy one.

https://codereview.chromium.org/268993002/diff/100001/Source/core/editing/Edi...
File Source/core/editing/EditorCommand.cpp (right):

https://codereview.chromium.org/268993002/diff/100001/Source/core/editing/Edi...
Source/core/editing/EditorCommand.cpp:1579: { "YankAndSelect", { 138,
executeYankAndSelect, supportedFromMenuOrKeyBinding, enabledInEditableText,
stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
Why do you touch "allowExecutionWhenDisabled" field instead of
changing enabled* field? I guess the point here is to enable
the commands in more situations, which makes me think changing
the enabled* field values makes more sense.

Apparently allowExecutionWhenDisabled is only used for copy/paste
commands, so I'm afraid that it may be inappropriate to make use
of it in this case.

https://codereview.chromium.org/268993002/diff/100001/Source/core/page/EventH...
File Source/core/page/EventHandler.cpp (right):

https://codereview.chromium.org/268993002/diff/100001/Source/core/page/EventH...
Source/core/page/EventHandler.cpp:776: if (node && node->renderer()) {
I think this change means that you will get a caret if you
click on a (non-input, non-editable) normal text, right?
If I'm not mistaken, this change may be going to confuse
people, since it changes the behavior of a click on
vast majority of areas of the screen (rendered text nodes).

[b.rout, 2014-06-04 12:53:04]

https://codereview.chromium.org/268993002/diff/100001/LayoutTests/editing/sel...
File
LayoutTests/editing/selection/place-caret-on-single-click-within-selected-readonly-text.html
(right):

https://codereview.chromium.org/268993002/diff/100001/LayoutTests/editing/sel...
LayoutTests/editing/selection/place-caret-on-single-click-within-selected-readonly-text.html:4:
<p>This tests caret placement when clicked once on existing selection in
readonly texts to allow further selection using keys.</p>
On 2014/06/04 08:19:38, Yuta Kitamura wrote:
> You can use description() function from js-test to show
> the test description.

Ok.

https://codereview.chromium.org/268993002/diff/100001/LayoutTests/editing/sel...
LayoutTests/editing/selection/place-caret-on-single-click-within-selected-readonly-text.html:5:
<div id="console"></div>
On 2014/06/04 08:19:38, Yuta Kitamura wrote:
> js-test automatically creates the element for output console,
> so you don't have to write your own one.

Actually, I want the HTML content to come before the test results. Thats why I
explicitly placed it.

https://codereview.chromium.org/268993002/diff/100001/LayoutTests/editing/sel...
LayoutTests/editing/selection/place-caret-on-single-click-within-selected-readonly-text.html:8:
if (window.testRunner) {
On 2014/06/04 08:19:38, Yuta Kitamura wrote:
> As a general rule, it is recommended to make the test
> runnable on the normal browsers that do not expose
> test-only objects.
> 
> If your test requires manual operations (like clicking
> in this test), it is encouraged to put the instructions
> of how to run the test manually, so people can run the
> test in their browsers.

Ok. I will update the tests with instructions to manually perform the test.
But, I see some limitations in the eventSender simulator for keyboard and 
mouse events, which is why I couldn't write the actual test case I intended.
For example, in this test case I wanted to write keyboard selection behavior.
Even if I click in between the selection, caret position is always set to 
the end of the content. However, I could write tests to show the keyboard 
selection considering that position. But, the concern is it does set the 
position correctly.

https://codereview.chromium.org/268993002/diff/100001/LayoutTests/editing/sel...
File
LayoutTests/editing/selection/select-by-char-and-word-in-readonly-texts-using-keys-with-caret-mode-off.html
(right):

https://codereview.chromium.org/268993002/diff/100001/LayoutTests/editing/sel...
LayoutTests/editing/selection/select-by-char-and-word-in-readonly-texts-using-keys-with-caret-mode-off.html:1:
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
On 2014/06/04 08:19:38, Yuta Kitamura wrote:
> Use HTML5 doctype instead of this legacy one.

Ok.

https://codereview.chromium.org/268993002/diff/100001/Source/core/editing/Edi...
File Source/core/editing/EditorCommand.cpp (right):

https://codereview.chromium.org/268993002/diff/100001/Source/core/editing/Edi...
Source/core/editing/EditorCommand.cpp:1579: { "YankAndSelect", { 138,
executeYankAndSelect, supportedFromMenuOrKeyBinding, enabledInEditableText,
stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
On 2014/06/04 08:19:38, Yuta Kitamura wrote:
> Why do you touch "allowExecutionWhenDisabled" field instead of
> changing enabled* field? I guess the point here is to enable
> the commands in more situations, which makes me think changing
> the enabled* field values makes more sense.

Yes, I agree with you that enabled* field will do the operation as well. 
Changing this would require me to drop selection.isContentEditable() check
 in enabledVisibleSelection() method. It will affect all sorts of 
selection operations available. Unfortunately, I am not aware of keyboard 
shortcuts for all possible selections. If I have to do it for the selection 
operations I have considered till now, I can have a new method for 
enabled* field. Please suggest. 

> Apparently allowExecutionWhenDisabled is only used for copy/paste
> commands, so I'm afraid that it may be inappropriate to make use
> of it in this case.

Is there any reason behind allowExecutionWhenDisabled being used for
copy/paste alone?

https://codereview.chromium.org/268993002/diff/100001/Source/core/page/EventH...
File Source/core/page/EventHandler.cpp (right):

https://codereview.chromium.org/268993002/diff/100001/Source/core/page/EventH...
Source/core/page/EventHandler.cpp:776: if (node && node->renderer()) {
On 2014/06/04 08:19:38, Yuta Kitamura wrote:
> I think this change means that you will get a caret if you
> click on a (non-input, non-editable) normal text, right?
> If I'm not mistaken, this change may be going to confuse
> people, since it changes the behavior of a click on
> vast majority of areas of the screen (rendered text nodes).

It will not show the caret, but it will set selection type to caret and 
position of the caret. Entry to the if block is only when we click 
within a selected section. For non-editable content, the selection type
was set as none since it was not allowed to set the position. And with 
selection type none we won't be able to select.

[b.rout, 2014-06-04 13:53:49]

How could I see the expected and actual result being generated in trybots?

Just noticed in my local repo currently foreignObject-repaint.xml is also
failing. 
I figured out even though I have updated the expected result, it still hasn't
updated 
it in
src/webkit/Debug/layout-test-results/svg/text/foreignObject-repaint-expected.txt.
Could you please help me know how src/webkit/Debug/layout-test-results/ gets
updated if 
any expected output is modified.

[Inactive, 2014-06-04 14:32:39]

On 2014/06/04 13:53:49, b.rout wrote:
> How could I see the expected and actual result being generated in trybots?

Click on the bot, then looks for "archive_webkit_tests_results" section (18?)
and click the "layout_test_results" link:
https://storage.googleapis.com/chromium-layout-test-archives/linux_blink_dbg/...

[b.rout, 2014-06-10 06:20:14]

Hi Yuta,

I need some clarification on the suggestions you have given, 
which I have mentioned in the patch. Could you please suggest 
what should be the right approach.

Regd. layout test, I want to know how src/webkit/Debug/layout-test-results/
directory is updated with the layout test expected files.

Thanks,
Bhanu

[Yuta Kitamura, 2014-06-11 08:34:22]

Okay, after having tested locally, I started to think this
is a good change, but this definitely changes the user-
visible behavior, so we might need some approval from
somebody (UI people?), maybe? Yoshi, what do you think?

Note that Firefox does implement this behavior, but
apparently IE does not (in fact, IE did not allow me to
modify selection with keyboard shortcuts).

https://codereview.chromium.org/268993002/diff/100001/Source/core/editing/Edi...
File Source/core/editing/EditorCommand.cpp (right):

https://codereview.chromium.org/268993002/diff/100001/Source/core/editing/Edi...
Source/core/editing/EditorCommand.cpp:1579: { "YankAndSelect", { 138,
executeYankAndSelect, supportedFromMenuOrKeyBinding, enabledInEditableText,
stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
On 2014/06/04 12:53:04, b.rout wrote:
> On 2014/06/04 08:19:38, Yuta Kitamura wrote:
> > Why do you touch "allowExecutionWhenDisabled" field instead of
> > changing enabled* field? I guess the point here is to enable
> > the commands in more situations, which makes me think changing
> > the enabled* field values makes more sense.
> 
> Yes, I agree with you that enabled* field will do the operation as well. 
> Changing this would require me to drop selection.isContentEditable() check
>  in enabledVisibleSelection() method. It will affect all sorts of 
> selection operations available. Unfortunately, I am not aware of keyboard 
> shortcuts for all possible selections. If I have to do it for the selection 
> operations I have considered till now, I can have a new method for 
> enabled* field. Please suggest. 
> 
> > Apparently allowExecutionWhenDisabled is only used for copy/paste
> > commands, so I'm afraid that it may be inappropriate to make use
> > of it in this case.
> 
> Is there any reason behind allowExecutionWhenDisabled being used for
> copy/paste alone? 

I checked the history and found that the following commit had added
the flag:

https://chromium.googlesource.com/chromium/blink/+/cf45000f64bf0005be84110b80...

This change tries to fix the oncut/oncopy/onpaste events
so they fire when a key combination for these events is hit
even if the command is disabled.

To summarize:
* isEnabled() == true
  -> execute and fire events
* isEnabled() == false and allowExecutionWhenDisabled == true
  -> don't execute but fire events
     (will be taken care by each command implementation)
* isEnabled() == false and allowExecutionWhenDisabled == false
  -> do nothing

(Yeah, I agree that these namings are a bit misleading.)

In this case, what you want to do is to execute the command
in some circumstances, so I'd bet the right approach is to
change enabled* properties (possibly with a new enabled*
function added), instead of touching allowExecutionWhenDisabled
properties.

Also, it looks like your changes to these definitions
are not exhaustive, like MoveForwardAndModifySelection
or MoveToBeginningOfParagraphAndModifySelection. Generally,
I think all Move*AndModifySelection commands should be
treated equally, even if they cannot be invoked directly
from key bindings (see EditingBehavior.cpp).

[yosin_UTC9, 2014-06-11 09:16:40]

On 2014/06/11 08:34:22, Yuta Kitamura wrote:
> Okay, after having tested locally, I started to think this
> is a good change, but this definitely changes the user-
> visible behavior, so we might need some approval from
> somebody (UI people?), maybe? Yoshi, what do you think?

I think extending selection by keyboard is confusing to users, because users
don't know which end will be extend by keyboard after selecting text by mouse. I
guess this is why IE doesn't do so.

Anyway, devs aren't man who decided UX changes. I assigned the bug to Kenji@ for
communicating with UX team.
Please wait going forward for UX judgement.

Thanks for your understanding.

[b.rout, 2014-06-26 09:08:17]

I have implemented review comments given in the previous patch and uploaded
another patch. But, I still kept it to the particular selection commands for
which I know the keyboard shortcuts as mentioned earlier. This time I added a
new method enabledSelection which does the similar operation as
enabledVisibleSelectionOrCaretBrowsing except the content editable check. I have
reverted back the previous fix where I was using allowExecutionWhenDisabled flag
to execute the command even when command was disabled. Please review and let me
know your comments or suggestions.

And could someone please kickoff the trybots.

[b.rout, 2014-06-30 06:16:48]

Could someone please kickoff the trybots.

[yosin_UTC9, 2014-06-30 06:37:48]

The CQ bit was checked by yosin@chromium.org

[yosin_UTC9, 2014-06-30 06:38:01]

Kicked try bots.

[commit-bot: I haz the power, 2014-06-30 06:38:41]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/b.rout@samsung.com/268993002/120001

[commit-bot: I haz the power, 2014-06-30 06:38:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-30 06:38:46]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[b.rout, 2014-06-30 10:32:17]

It seems the last attempt to run bots on my patch somehow failed. Could someone
please re-initiate it.

[yosin_UTC9, 2014-07-01 00:51:22]

Oops, my brain was damaged. :-<
Kick try bots.

[b.rout, 2014-07-18 14:44:45]

I have submitted another patch where I have modified
enabling-and-selection-2.txt (which was failing in the previous patch) since the
change is to enable the commands now. Here I have introduced a new category
"selectable, not necessarily editable" for the selection operations that has
been considered. For these selection operations all bits except
enabledWithNoSelection bit (in enabling-and-selection-2.html) are set.
enabledWithNoSelection bit is not set because it signifies selection type as
None.

Additionally, I don't see the following layout test files (which failed in my
previous patch for win_blink_rel) in the repo
virtual/antialiasedtext/fast/text/emphasis-vertical.html
virtual/antialiasedtext/fast/text/cg-fallback-bolding.html
virtual/antialiasedtext/fast/text/fallback-for-custom-font.html

And could someone please initiate try-bots.

[b.rout, 2014-07-21 05:48:11]

Could someone please kickoff the trybots.

[b.rout, 2014-07-22 06:24:55]

The win_blink_rel failure for patch 8 seems to be unrelated to my change. 
I couldn't figure out why win_blink_compile_rel bot has failed.
mac_blink_rel failure was due to one additional line in the expected result. I
have corrected it in patch 9. 
Could someone please initiate the trybots again.

[tkent, 2014-09-17 05:16:08]

tkent@chromium.org changed reviewers:
- tkent@chromium.org