Use tighter typing in editing: ReplaceSelectionCommand

Use tighter typing in editing: ReplaceSelectionCommand

Use tighter typing in editing/ to increase code readability and to avoid
unnecessary type checks. This CL focuses on ReplaceSelectionCommand.

R=leviw@chromium.org, yosin@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180076

[yosin_UTC9, 2014-08-01 02:11:29]

LGTM

[Inactive, 2014-08-01 13:04:49]

On 2014/08/01 02:11:29, Yosi_UTC9 wrote:
> LGTM

Levi, can you please take a look as well?

[Inactive, 2014-08-04 19:45:23]

On 2014/08/01 13:04:49, Chris Dumez wrote:
> On 2014/08/01 02:11:29, Yosi_UTC9 wrote:
> > LGTM
> 
> Levi, can you please take a look as well?

ping review?

[Inactive, 2014-08-06 13:01:33]

I rebased the patch, PTAL.

[leviw_travelin_and_unemployed, 2014-08-06 20:58:07]

https://codereview.chromium.org/423413005/diff/20001/Source/core/editing/Repl...
File Source/core/editing/ReplaceSelectionCommand.cpp (right):

https://codereview.chromium.org/423413005/diff/20001/Source/core/editing/Repl...
Source/core/editing/ReplaceSelectionCommand.cpp:92: void
restoreAndRemoveTestRenderingNodesToFragment(ContainerNode*);
This is less tight typing. Why?

[Inactive, 2014-08-06 21:00:05]

https://codereview.chromium.org/423413005/diff/20001/Source/core/editing/Repl...
File Source/core/editing/ReplaceSelectionCommand.cpp (right):

https://codereview.chromium.org/423413005/diff/20001/Source/core/editing/Repl...
Source/core/editing/ReplaceSelectionCommand.cpp:92: void
restoreAndRemoveTestRenderingNodesToFragment(ContainerNode*);
On 2014/08/06 20:58:07, leviw wrote:
> This is less tight typing. Why?

Hmm, This is a very good question. It looks like a mistake on my part. I'll fix
it.

[Inactive, 2014-08-06 21:07:49]

https://codereview.chromium.org/423413005/diff/20001/Source/core/editing/Repl...
File Source/core/editing/ReplaceSelectionCommand.cpp (right):

https://codereview.chromium.org/423413005/diff/20001/Source/core/editing/Repl...
Source/core/editing/ReplaceSelectionCommand.cpp:92: void
restoreAndRemoveTestRenderingNodesToFragment(ContainerNode*);
On 2014/08/06 21:00:05, Chris Dumez wrote:
> On 2014/08/06 20:58:07, leviw wrote:
> > This is less tight typing. Why?
> 
> Hmm, This is a very good question. It looks like a mistake on my part. I'll
fix
> it.

I probably changed it to HTMLElement*, then noticed it wasn't worth it (or
wouldn't work), and then "reverted" back locally to the wrong type. This is
fixed in the latest patchset. Sorry about this.

[Inactive, 2014-08-11 21:05:38]

Levi, any additional comment on this?

[leviw_travelin_and_unemployed, 2014-08-11 22:13:28]

lgtm.

https://codereview.chromium.org/423413005/diff/40001/Source/core/editing/Repl...
File Source/core/editing/ReplaceSelectionCommand.cpp (right):

https://codereview.chromium.org/423413005/diff/40001/Source/core/editing/Repl...
Source/core/editing/ReplaceSelectionCommand.cpp:523: HTMLQuoteElement*
blockquoteElement = !context ||
isMailPasteAsQuotationHTMLBlockQuoteElement(context) ?
toHTMLQuoteElement(context) :
toHTMLQuoteElement(enclosingNodeOfType(firstPositionInNode(context),
isMailHTMLBlockquoteElement, CanCrossEditingBoundary));
Nit: I'd wrap this longness.

https://codereview.chromium.org/423413005/diff/40001/Source/core/editing/Repl...
Source/core/editing/ReplaceSelectionCommand.cpp:797: HTMLQuoteElement*
blockquoteElement = isMailPasteAsQuotationHTMLBlockQuoteElement(context) ?
toHTMLQuoteElement(context) :
toHTMLQuoteElement(enclosingNodeOfType(firstPositionInNode(context),
isMailHTMLBlockquoteElement, CanCrossEditingBoundary));
Ditto wrapping.

[Inactive, 2014-08-12 12:59:15]

https://codereview.chromium.org/423413005/diff/40001/Source/core/editing/Repl...
File Source/core/editing/ReplaceSelectionCommand.cpp (right):

https://codereview.chromium.org/423413005/diff/40001/Source/core/editing/Repl...
Source/core/editing/ReplaceSelectionCommand.cpp:523: HTMLQuoteElement*
blockquoteElement = !context ||
isMailPasteAsQuotationHTMLBlockQuoteElement(context) ?
toHTMLQuoteElement(context) :
toHTMLQuoteElement(enclosingNodeOfType(firstPositionInNode(context),
isMailHTMLBlockquoteElement, CanCrossEditingBoundary));
On 2014/08/11 22:13:27, leviw wrote:
> Nit: I'd wrap this longness.

Done.

https://codereview.chromium.org/423413005/diff/40001/Source/core/editing/Repl...
Source/core/editing/ReplaceSelectionCommand.cpp:797: HTMLQuoteElement*
blockquoteElement = isMailPasteAsQuotationHTMLBlockQuoteElement(context) ?
toHTMLQuoteElement(context) :
toHTMLQuoteElement(enclosingNodeOfType(firstPositionInNode(context),
isMailHTMLBlockquoteElement, CanCrossEditingBoundary));
On 2014/08/11 22:13:27, leviw wrote:
> Ditto wrapping.

Done.

[Inactive, 2014-08-12 12:59:19]

The CQ bit was checked by ch.dumez@samsung.com

[commit-bot: I haz the power, 2014-08-12 13:00:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/423413005/60001

[commit-bot: I haz the power, 2014-08-12 14:05:24]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/22273)

[commit-bot: I haz the power, 2014-08-12 14:47:54]

Change committed as 180076