Fix some m_instVar instances in wtf/

Fix some m_instVar instances in wtf/

cf https://docs.google.com/spreadsheets/d/1px9A3CxS-Y2r51wsD7_yhqnH_YJTnwb5AQAkRuJAs6E/edit#gid=0

Remove StringImpl's STRING_STATS feature -- it hasn't been compiling for
a while.  If someone wants to use it, they can revive it from revision
history and make it compile then.

BUG=675877
Review-Url: https://codereview.chromium.org/2856123004
Cr-Commit-Position: refs/heads/master@{#470015}
Committed: https://chromium.googlesource.com/chromium/src/+/96c02c8b0f96f995b15d4efe5783a2c4015999a0

[Nico, 2017-05-03 18:45:19]

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

[commit-bot: I haz the power, 2017-05-03 18:46:29]

Dry run: CQ is trying da patch.

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

[Nico, 2017-05-03 18:47:05]

thakis@chromium.org changed reviewers:
+ dcheng@chromium.org

[Nico, 2017-05-03 18:47:06]

https://codereview.chromium.org/2856123004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/wtf/LinkedHashSet.h (right):

https://codereview.chromium.org/2856123004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/wtf/LinkedHashSet.h:709: return
FirstNode()->value_;
Apparently no code ever calls the const version.

https://codereview.chromium.org/2856123004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/wtf/Vector.h (left):

https://codereview.chromium.org/2856123004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/wtf/Vector.h:1754:
ANNOTATE_CHANGE_SIZE(begin(), capacity(), m_size, m_size + 1);
ANNOTATE_CHANGE_SIZE only expands to something if ANNOTATE_CONTIGUOUS_CONTAINER
is defined, and we're in an #else for that -- so this line does nothing.

[commit-bot: I haz the power, 2017-05-03 19:01:15]

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

[commit-bot: I haz the power, 2017-05-03 19:01:16]

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

[Nico, 2017-05-03 19:05:12]

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

[commit-bot: I haz the power, 2017-05-03 19:05:58]

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, 2017-05-03 19:18:01]

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

[commit-bot: I haz the power, 2017-05-03 19:18:02]

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

[Nico, 2017-05-03 19:20:29]

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

[commit-bot: I haz the power, 2017-05-03 19:21:26]

Dry run: CQ is trying da patch.

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

[dcheng, 2017-05-03 19:44:40]

https://codereview.chromium.org/2856123004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/wtf/text/StringImpl.cpp (left):

https://codereview.chromium.org/2856123004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/wtf/text/StringImpl.cpp:70: #ifdef
STRING_STATS
Hmm... FWIW, the last time I asked about removing this, several people were in
favor of retaining it:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/aZSxBxs8_10

So maybe it'd be nice to keep it around for a bit longer... if no one rebases
this code in, say, 6 or 12 months, then I'd say it's probably safe to remove?

[Nico, 2017-05-03 19:54:42]

It currently does not build. If people want to make it build, they can
retrieve it from history without problems, and then make it build. It does
not make sense to keep non-building code around if it's nontrivial to make
it build.

On Wed, May 3, 2017 at 3:44 PM, <dcheng@chromium.org> wrote:

>
> https://codereview.chromium.org/2856123004/diff/40001/
> third_party/WebKit/Source/platform/wtf/text/StringImpl.cpp
> File third_party/WebKit/Source/platform/wtf/text/StringImpl.cpp (left):
>
> https://codereview.chromium.org/2856123004/diff/40001/
> third_party/WebKit/Source/platform/wtf/text/StringImpl.cpp#oldcode70
> third_party/WebKit/Source/platform/wtf/text/StringImpl.cpp:70: #ifdef
> STRING_STATS
> Hmm... FWIW, the last time I asked about removing this, several people
> were in favor of retaining it:
> https://groups.google.com/a/chromium.org/forum/#!topic/
> blink-dev/aZSxBxs8_10
>
> So maybe it'd be nice to keep it around for a bit longer... if no one
> rebases this code in, say, 6 or 12 months, then I'd say it's probably
> safe to remove?
>
> https://codereview.chromium.org/2856123004/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Nico, 2017-05-03 19:54:43]

It currently does not build. If people want to make it build, they can
retrieve it from history without problems, and then make it build. It does
not make sense to keep non-building code around if it's nontrivial to make
it build.

On Wed, May 3, 2017 at 3:44 PM, <dcheng@chromium.org> wrote:

>
> https://codereview.chromium.org/2856123004/diff/40001/
> third_party/WebKit/Source/platform/wtf/text/StringImpl.cpp
> File third_party/WebKit/Source/platform/wtf/text/StringImpl.cpp (left):
>
> https://codereview.chromium.org/2856123004/diff/40001/
> third_party/WebKit/Source/platform/wtf/text/StringImpl.cpp#oldcode70
> third_party/WebKit/Source/platform/wtf/text/StringImpl.cpp:70: #ifdef
> STRING_STATS
> Hmm... FWIW, the last time I asked about removing this, several people
> were in favor of retaining it:
> https://groups.google.com/a/chromium.org/forum/#!topic/
> blink-dev/aZSxBxs8_10
>
> So maybe it'd be nice to keep it around for a bit longer... if no one
> rebases this code in, say, 6 or 12 months, then I'd say it's probably
> safe to remove?
>
> https://codereview.chromium.org/2856123004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dcheng, 2017-05-03 20:15:26]

Right, I understand it doesn't build: I was suggesting using whether or not this
code gets updated in <x> timeframe as a proxy for figuring out if there's any
point in keeping this code around (since no one spoke in favor of removing it
other than me, and at least several people were weakly in favor of retaining it
at the time).

That being said, I'm personally OK with removing it so LGTM.

[commit-bot: I haz the power, 2017-05-03 22:09:45]

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

[commit-bot: I haz the power, 2017-05-03 22:09:46]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-05-04 02:04:36]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org, yutak@chromium.org

[haraken, 2017-05-04 02:04:36]

On 2017/05/03 20:15:26, dcheng wrote:
> Right, I understand it doesn't build: I was suggesting using whether or not
this
> code gets updated in <x> timeframe as a proxy for figuring out if there's any
> point in keeping this code around (since no one spoke in favor of removing it
> other than me, and at least several people were weakly in favor of retaining
it
> at the time).
> 
> That being said, I'm personally OK with removing it so LGTM.

Yeah, I agree that it does't make sense to keep non-building code but people are
sometimes using STRING_STATS.

+yutak: Would you try to make it buildable?

[Nico, 2017-05-05 15:14:55]

On Wed, May 3, 2017 at 10:04 PM, <haraken@chromium.org> wrote:

> On 2017/05/03 20:15:26, dcheng wrote:
> > Right, I understand it doesn't build: I was suggesting using whether or
> not
> this
> > code gets updated in <x> timeframe as a proxy for figuring out if
> there's any
> > point in keeping this code around (since no one spoke in favor of
> removing it
> > other than me, and at least several people were weakly in favor of
> retaining
> it
> > at the time).
> >
> > That being said, I'm personally OK with removing it so LGTM.
>
> Yeah, I agree that it does't make sense to keep non-building code but
> people are
> sometimes using STRING_STATS.
>
> +yutak: Would you try to make it buildable?
>

I'll wait until end-of-day east coast time next week (given that this week
is golden week), then I'll land this.


>
>
> https://codereview.chromium.org/2856123004/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Nico, 2017-05-05 15:14:56]

On Wed, May 3, 2017 at 10:04 PM, <haraken@chromium.org> wrote:

> On 2017/05/03 20:15:26, dcheng wrote:
> > Right, I understand it doesn't build: I was suggesting using whether or
> not
> this
> > code gets updated in <x> timeframe as a proxy for figuring out if
> there's any
> > point in keeping this code around (since no one spoke in favor of
> removing it
> > other than me, and at least several people were weakly in favor of
> retaining
> it
> > at the time).
> >
> > That being said, I'm personally OK with removing it so LGTM.
>
> Yeah, I agree that it does't make sense to keep non-building code but
> people are
> sometimes using STRING_STATS.
>
> +yutak: Would you try to make it buildable?
>

I'll wait until end-of-day east coast time next week (given that this week
is golden week), then I'll land this.


>
>
> https://codereview.chromium.org/2856123004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Yuta Kitamura, 2017-05-08 08:13:16]

Sorry for not responding, I'm just back from the vacation and processing
a long queue of emails...

On 2017/05/04 02:04:36, haraken wrote:
> +yutak: Would you try to make it buildable?

No objection; feel free to remove it.

In the future, I'd like to do something more intelligent
than MACRO-enabled conditional builds, since they are hard to
maintain.

[Yuta Kitamura, 2017-05-08 08:14:45]

With that said, this LGTM.

[Nico, 2017-05-08 13:50:20]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2017-05-08 13:50:38]

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, 2017-05-08 16:44:06]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494251420463530,
"parent_rev": "474dd1ddfd9524b8756a983e221c52a45aff7b63", "commit_rev":
"96c02c8b0f96f995b15d4efe5783a2c4015999a0"}

[commit-bot: I haz the power, 2017-05-08 16:44:18]

Description was changed from

==========
Fix some m_instVar instances in wtf/

cf
https://docs.google.com/spreadsheets/d/1px9A3CxS-Y2r51wsD7_yhqnH_YJTnwb5AQAkR...

Remove StringImpl's STRING_STATS feature -- it hasn't been compiling for
a while.  If someone wants to use it, they can revive it from revision
history and make it compile then.

BUG=675877
==========

to

==========
Fix some m_instVar instances in wtf/

cf
https://docs.google.com/spreadsheets/d/1px9A3CxS-Y2r51wsD7_yhqnH_YJTnwb5AQAkR...

Remove StringImpl's STRING_STATS feature -- it hasn't been compiling for
a while.  If someone wants to use it, they can revive it from revision
history and make it compile then.

BUG=675877

Review-Url: https://codereview.chromium.org/2856123004
Cr-Commit-Position: refs/heads/master@{#470015}
Committed:
https://chromium.googlesource.com/chromium/src/+/96c02c8b0f96f995b15d4efe5783...
==========

[commit-bot: I haz the power, 2017-05-08 16:44:19]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/96c02c8b0f96f995b15d4efe5783...