Convert WebRange to be a simple pair of numbers.

Convert WebRange to be a simple pair of numbers.

Instead of wrapping and carrying a fairly complex Range object, just use a pair of numbers to convey this information outside of the renderer. The embedders don't
need anything else.

R=avi,esprehn
BUG=635386
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/8f8060f3c58fe047bc3d9de90752cf3735f6beac
Cr-Commit-Position: refs/heads/master@{#411104}

[dglazkov, 2016-07-30 00:41:47]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-30 00:42:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-30 02:50:38]

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

[commit-bot: I haz the power, 2016-07-30 02:50:39]

Dry run: This issue passed the CQ dry run.

[dglazkov, 2016-08-02 22:09:06]

Created WebDeprecatedRange.

[dglazkov, 2016-08-02 22:09:07]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[dglazkov, 2016-08-02 22:09:08]

Description was changed from

==========
Clean up the use of WebRange in RenderFrameImpl.

* Removed the WebRange::fromDocumentRange
* Instead of using WebRange, which wraps a real blink Range object,
  turned to start+length pair of numbers that actually simplifies
  the code quite a bit.
* Added WebLocalFrame::rangeAsText as a more specialized API.
* Simplified API surface of WebRange.

R=avi,esprehn
BUG=
==========

to

==========
Clean up the use of WebRange in RenderFrameImpl.

* Removed the WebRange::fromDocumentRange
* Instead of using WebRange, which wraps a real blink Range object,
  turned to start+length pair of numbers that actually simplifies
  the code quite a bit.
* Added WebLocalFrame::rangeAsText as a more specialized API.
* Simplified API surface of WebRange.

R=avi,esprehn
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-08-02 22:09:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 22:25:22]

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

[commit-bot: I haz the power, 2016-08-02 22:25:23]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dglazkov, 2016-08-02 22:42:49]

WIP

[dglazkov, 2016-08-05 04:20:11]

Let's try things.

[dglazkov, 2016-08-05 04:20:12]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-05 04:20:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-05 04:39:03]

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

[commit-bot: I haz the power, 2016-08-05 04:39:04]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dglazkov, 2016-08-05 04:54:07]

Fix oopsies.

[dglazkov, 2016-08-05 04:54:08]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-05 04:54:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

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

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

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

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[dglazkov, 2016-08-08 04:22:50]

Let's see now...

[dglazkov, 2016-08-08 04:22:51]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-08 04:23:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-08 05:56:30]

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

[commit-bot: I haz the power, 2016-08-08 05:56:32]

Dry run: This issue passed the CQ dry run.

[dglazkov, 2016-08-09 05:03:59]

Clean up.

[dglazkov, 2016-08-09 05:04:00]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-09 05:04:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-09 09:24:42]

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

[commit-bot: I haz the power, 2016-08-09 09:24:43]

Dry run: This issue passed the CQ dry run.

[dglazkov, 2016-08-09 17:57:06]

Description was changed from

==========
Clean up the use of WebRange in RenderFrameImpl.

* Removed the WebRange::fromDocumentRange
* Instead of using WebRange, which wraps a real blink Range object,
  turned to start+length pair of numbers that actually simplifies
  the code quite a bit.
* Added WebLocalFrame::rangeAsText as a more specialized API.
* Simplified API surface of WebRange.

R=avi,esprehn
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Convert WebRange to be a simple pair of numbers.

Instead of wrapping and carrying a fairly complex Range object, just use a pair
of numbers to convey this information outside of the renderer. The embedders
don't
need anything else.

R=avi,esprehn
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dglazkov, 2016-08-09 17:57:27]

Description was changed from

==========
Convert WebRange to be a simple pair of numbers.

Instead of wrapping and carrying a fairly complex Range object, just use a pair
of numbers to convey this information outside of the renderer. The embedders
don't
need anything else.

R=avi,esprehn
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Convert WebRange to be a simple pair of numbers.

Instead of wrapping and carrying a fairly complex Range object, just use a pair
of numbers to convey this information outside of the renderer. The embedders
don't
need anything else.

R=avi,esprehn
BUG=635386
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dglazkov, 2016-08-09 18:09:44]

Moar cleanup.

[dglazkov, 2016-08-09 18:09:45]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-09 18:10:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dglazkov, 2016-08-09 18:10:13]

PTAL.

[ojan, 2016-08-09 18:15:04]

ojan@chromium.org changed reviewers:
+ ojan@chromium.org

[ojan, 2016-08-09 18:15:05]

lgtm

https://codereview.chromium.org/2199523002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebRange.cpp (right):

https://codereview.chromium.org/2199523002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebRange.cpp:45: DCHECK(start != 1 && length != 0)
<< "These values are reserved to indicate that the range is null";
s/1/-1/ ?

[commit-bot: I haz the power, 2016-08-09 18:35:07]

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

[commit-bot: I haz the power, 2016-08-09 18:35:08]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[esprehn, 2016-08-10 04:12:06]

https://codereview.chromium.org/2199523002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp (right):

https://codereview.chromium.org/2199523002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp:903: // silly. What is
going on here?
yeah seems like some bad layering

https://codereview.chromium.org/2199523002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebLocalFrameImpl.cpp (right):

https://codereview.chromium.org/2199523002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebLocalFrameImpl.cpp:1180: if (Range* range =
webRange.createRange(frame()))
making a live Range like this is pretty expensive, especially since we call Text
on it immediately and then discard it. We should be using an EphemeralRange
instead.

https://codereview.chromium.org/2199523002/diff/140001/third_party/WebKit/pub...
File third_party/WebKit/public/web/WebRange.h (right):

https://codereview.chromium.org/2199523002/diff/140001/third_party/WebKit/pub...
third_party/WebKit/public/web/WebRange.h:48: int endOffset() const { return
m_end; }
I'd probs order this

startOffset()
endOffset()
length()

:P

[esprehn, 2016-08-10 04:12:28]

lgtm w/ Ojan's fix, but we really need to switch this to EphemeralRange.

[dglazkov, 2016-08-10 05:10:43]

Comments addressed.

[dglazkov, 2016-08-10 05:10:46]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 05:11:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dglazkov, 2016-08-10 05:12:13]

https://codereview.chromium.org/2199523002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebLocalFrameImpl.cpp (right):

https://codereview.chromium.org/2199523002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebLocalFrameImpl.cpp:1180: if (Range* range =
webRange.createRange(frame()))
On 2016/08/10 at 04:12:06, esprehn wrote:
> making a live Range like this is pretty expensive, especially since we call
Text on it immediately and then discard it. We should be using an EphemeralRange
instead.

Agreed, will change in the next CL.

https://codereview.chromium.org/2199523002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebRange.cpp (right):

https://codereview.chromium.org/2199523002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebRange.cpp:45: DCHECK(start != 1 && length != 0)
<< "These values are reserved to indicate that the range is null";
On 2016/08/09 at 18:15:05, ojan wrote:
> s/1/-1/ ?

LOL. YES.

https://codereview.chromium.org/2199523002/diff/140001/third_party/WebKit/pub...
File third_party/WebKit/public/web/WebRange.h (right):

https://codereview.chromium.org/2199523002/diff/140001/third_party/WebKit/pub...
third_party/WebKit/public/web/WebRange.h:48: int endOffset() const { return
m_end; }
On 2016/08/10 at 04:12:06, esprehn wrote:
> I'd probs order this
> 
> startOffset()
> endOffset()
> length()
> 
> :P

Done.

[commit-bot: I haz the power, 2016-08-10 05:23:39]

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

[commit-bot: I haz the power, 2016-08-10 05:23:40]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dglazkov, 2016-08-10 05:34:33]

Forgot the exports.

[dglazkov, 2016-08-10 05:34:34]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 05:35:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 06:44:50]

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

[commit-bot: I haz the power, 2016-08-10 06:44:52]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dglazkov, 2016-08-10 17:30:20]

The CQ bit was checked by dglazkov@chromium.org

[dglazkov, 2016-08-10 17:30:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
esprehn@chromium.org, ojan@chromium.org
Link to the patchset: https://codereview.chromium.org/2199523002/#ps180001
(title: "Forgot the exports.")

[commit-bot: I haz the power, 2016-08-10 17:32:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 17:44:06]

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

[commit-bot: I haz the power, 2016-08-10 17:44:07]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[esprehn, 2016-08-10 17:55:55]

esprehn@chromium.org changed reviewers:
+ thestig@chromium.org

[esprehn, 2016-08-10 17:55:56]

thestig@ Can we get a stamp on the printing change here? It's totally
mechanical.

[Lei Zhang, 2016-08-10 18:01:34]

lgtm

[esprehn, 2016-08-10 18:02:51]

The CQ bit was checked by esprehn@chromium.org

[commit-bot: I haz the power, 2016-08-10 18:03:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 18:47:59]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-08-10 18:49:47]

Description was changed from

==========
Convert WebRange to be a simple pair of numbers.

Instead of wrapping and carrying a fairly complex Range object, just use a pair
of numbers to convey this information outside of the renderer. The embedders
don't
need anything else.

R=avi,esprehn
BUG=635386
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Convert WebRange to be a simple pair of numbers.

Instead of wrapping and carrying a fairly complex Range object, just use a pair
of numbers to convey this information outside of the renderer. The embedders
don't
need anything else.

R=avi,esprehn
BUG=635386
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/8f8060f3c58fe047bc3d9de90752cf3735f6beac
Cr-Commit-Position: refs/heads/master@{#411104}
==========

[commit-bot: I haz the power, 2016-08-10 18:49:50]

Patchset 10 (id:??) landed as
https://crrev.com/8f8060f3c58fe047bc3d9de90752cf3735f6beac
Cr-Commit-Position: refs/heads/master@{#411104}