Harden changing font style over unknown elements.

Harden changing font style over unknown elements.

When adjusting the start node position of where to apply a font style
command, check that we haven't stepped off the end.

R=leviw
BUG=484434
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=194944

[sof, 2015-05-05 14:36:39]

sigbjornf@opera.com changed reviewers:
+ leviw@chromium.org, yosin@chromium.org, yutak@chromium.org

[sof, 2015-05-05 14:36:39]

please take a look.

[leviw_travelin_and_unemployed, 2015-05-05 18:21:40]

LGTM on the code change.

Remove the "R=" line from your description.

s/Robustify/Harden/ ?

https://codereview.chromium.org/1124863003/diff/1/LayoutTests/editing/style/a...
File LayoutTests/editing/style/apply-style-crash3.html (right):

https://codereview.chromium.org/1124863003/diff/1/LayoutTests/editing/style/a...
LayoutTests/editing/style/apply-style-crash3.html:21: } catch(e) {}
Is this try/catch necessary?

https://codereview.chromium.org/1124863003/diff/1/LayoutTests/editing/style/a...
LayoutTests/editing/style/apply-style-crash3.html:37: window.onload = runTest;
If you move this script tag to the end, you probably don't need the onload
handler.

https://codereview.chromium.org/1124863003/diff/1/LayoutTests/editing/style/a...
LayoutTests/editing/style/apply-style-crash3.html:40: </body>
Nit: </body> and </html> aren't necessary.

[sof, 2015-05-05 19:47:32]

Thanks for the quick review.

https://codereview.chromium.org/1124863003/diff/1/LayoutTests/editing/style/a...
File LayoutTests/editing/style/apply-style-crash3.html (right):

https://codereview.chromium.org/1124863003/diff/1/LayoutTests/editing/style/a...
LayoutTests/editing/style/apply-style-crash3.html:21: } catch(e) {}
On 2015/05/05 18:21:40, leviw wrote:
> Is this try/catch necessary?

An alternative is to add an Element type check, which I've now done.

https://codereview.chromium.org/1124863003/diff/1/LayoutTests/editing/style/a...
LayoutTests/editing/style/apply-style-crash3.html:37: window.onload = runTest;
On 2015/05/05 18:21:40, leviw wrote:
> If you move this script tag to the end, you probably don't need the onload
> handler.

Moved stuff around a bit to avoid having to trigger it via onload.

https://codereview.chromium.org/1124863003/diff/1/LayoutTests/editing/style/a...
LayoutTests/editing/style/apply-style-crash3.html:40: </body>
On 2015/05/05 18:21:40, leviw wrote:
> Nit: </body> and </html> aren't necessary.

Just following local convention :)

[sof, 2015-05-05 19:47:36]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2015-05-05 19:47:37]

The patchset sent to the CQ was uploaded after l-g-t-m from leviw@chromium.org
Link to the patchset: https://codereview.chromium.org/1124863003/#ps20001
(title: "Clean up the test")

[commit-bot: I haz the power, 2015-05-05 19:47:57]

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

[commit-bot: I haz the power, 2015-05-05 22:21:06]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194944