Removal of more temporary Range objects.

Removal of more temporary Range objects.

The DOM/textarea-edit benchmark now creates no temporary ranges at all.

R=yosin@chromium.org
BUG=388681
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182224

[yosin_UTC9, 2014-09-18 01:08:52]

LGTM

Thanks!

https://codereview.chromium.org/576073003/diff/40001/Source/core/dom/Document...
File Source/core/dom/DocumentMarkerController.cpp (right):

https://codereview.chromium.org/576073003/diff/40001/Source/core/dom/Document...
Source/core/dom/DocumentMarkerController.cpp:134:
addMarker(markedText.startContainer(), DocumentMarker(type,
markedText.startOffset(), markedText.endOffset()));
nit: It seems ctor of |DocumentMarker| check |description.isEmpty()|, so we can
make this function to call |addMarker(const Position&, const Position&,
DocumentMarker::MakrerType, const String&)| with |emptyString()|.

It is OK to do this in another patch, because other overload functions don't do
so.

https://codereview.chromium.org/576073003/diff/40001/Source/core/editing/Text...
File Source/core/editing/TextCheckingHelper.cpp (right):

https://codereview.chromium.org/576073003/diff/40001/Source/core/editing/Text...
Source/core/editing/TextCheckingHelper.cpp:524: Document& doc =
m_start.containerNode()->document();
Since, |m_start.m_anchorNode| can be null. Could you add
|ASSERT(m_start.isNotNull())|?

[Mads Ager (chromium), 2014-09-18 06:58:49]

Thanks for the review! :)

https://codereview.chromium.org/576073003/diff/40001/Source/core/dom/Document...
File Source/core/dom/DocumentMarkerController.cpp (right):

https://codereview.chromium.org/576073003/diff/40001/Source/core/dom/Document...
Source/core/dom/DocumentMarkerController.cpp:134:
addMarker(markedText.startContainer(), DocumentMarker(type,
markedText.startOffset(), markedText.endOffset()));
On 2014/09/18 01:08:51, Yosi_UTC9 wrote:
> nit: It seems ctor of |DocumentMarker| check |description.isEmpty()|, so we
can
> make this function to call |addMarker(const Position&, const Position&,
> DocumentMarker::MakrerType, const String&)| with |emptyString()|.
> 
> It is OK to do this in another patch, because other overload functions don't
do
> so.

Thanks, that is true. Looks like this can be simplified a bit using default
arguments. Let me create a separate patch for that.

https://codereview.chromium.org/576073003/diff/40001/Source/core/editing/Text...
File Source/core/editing/TextCheckingHelper.cpp (right):

https://codereview.chromium.org/576073003/diff/40001/Source/core/editing/Text...
Source/core/editing/TextCheckingHelper.cpp:524: Document& doc =
m_start.containerNode()->document();
On 2014/09/18 01:08:52, Yosi_UTC9 wrote:
> Since, |m_start.m_anchorNode| can be null. Could you add
> |ASSERT(m_start.isNotNull())|?

Done.

[Mads Ager (chromium), 2014-09-18 06:58:53]

The CQ bit was checked by ager@chromium.org

[commit-bot: I haz the power, 2014-09-18 06:59:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/576073003/60001

[commit-bot: I haz the power, 2014-09-18 07:11:31]

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

[commit-bot: I haz the power, 2014-09-18 07:11:32]

Try jobs failed on following builders:
  mac_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)

[Mads Ager (chromium), 2014-09-18 07:17:25]

The CQ bit was checked by ager@chromium.org

[commit-bot: I haz the power, 2014-09-18 07:17:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/576073003/80001

[commit-bot: I haz the power, 2014-09-18 08:22:22]

Committed patchset #5 (id:80001) as 182224