Clang plugin: Allow >1 WeakPtrFactory member in a class

Clang plugin: Allow >1 WeakPtrFactory member in a class

This CL makes the WeakPtrFactory check allow more than 1 WeakPtrFactory at
the end of the class, but still ensures that no other data members appear after the WeakPtrFactory(s).

BUG=303818
Committed: https://crrev.com/b2bf826e635fd514899af6860f712d6b58cf6fdf
Cr-Commit-Position: refs/heads/master@{#305055}

[dmichael (off chromium), 2014-11-20 00:03:48]

dmichael@chromium.org changed reviewers:
+ mohan.reddy@samsung.com, thakis@chromium.org

[dmichael (off chromium), 2014-11-20 00:03:48]

(With big thanks to the Samsung folks who cleaned up the WeakPtrFactory
problems)

[dmichael (off chromium), 2014-11-20 00:04:55]

WAIT, not quite ready to turn this on yet. Still not error free yet.

[Nico, 2014-11-20 00:08:22]

It looks like this CL does two things:

1. Remove the check-weak-ptr-factory-order flag
2. Change the behavior of the check

It also looks like check-weak-ptr-factory-order isn't enabled right now, so
chances are the codebase won't be clean for this check the next time we roll
clang.

If I get 2 right, and you actually want to make this change more strict, I
suggest this order:

1. Make check more strict
2. Wait for more strict warning to make it into a clang roll
3. Add check-weak-ptr-factory-order to  tools/clang/scripts/plugin_flags.sh to
make sure the code is (and stays) with the new check
4. Remove the flag from the plugin, as the check can now always be on.

If the code is clean with this check right now, the check is wrong since I'm
aware of at least one place where the WeakPtrFactory isn't right:
https://code.google.com/p/chromium/issues/detail?id=433583

[dmichael (off chromium), 2014-11-20 17:21:38]

Right, Chromium is not clean w.r.t. this check yet. I published before letting
my compile finish; I assumed from: https://codereview.chromium.org/669853003/
that we were clean.

I narrowed the scope of this CL to only change the check. It makes it more
permissive, to allow class Foo to have >1 WeakPtrFactory<Foo>, so long as they
appear last:
class Foo {
  WeakPtrFactory<Foo> factory1_;
  WeakPtrFactory<Foo> factory2_;
};

Thanks for reminding me about the clang roll. Turning on the flag in Chromium
when it's ready prior to removing the flag SGTM.

[Nico, 2014-11-20 17:57:05]

lgtm

When do you need to WeakPtrFactory members though? If you want one class to hand
out weak ptrs that are used on different threads?

[dmichael (off chromium), 2014-11-20 18:19:58]

On 2014/11/20 17:57:05, Nico wrote:
> lgtm
> 
> When do you need to WeakPtrFactory members though? If you want one class to
hand
> out weak ptrs that are used on different threads?
That's one possible way...  example here:
https://code.google.com/p/chromium/codesearch#chromium/src/net/http/http_serv...

The more common pattern I've seen is for using WeakPtrFactory to cancel some
callbacks but not all. You can call InvalidateWeakPtrs at any time on factory A,
so any callbacks bound with WeakPtrs from it won't run, but callbacks bound with
WeakPtrs from factory B can still run. Here's one of the examples of that:
https://code.google.com/p/chromium/codesearch#chromium/src/net/spdy/spdy_prox...
(But there are many)

Thanks for the review!

[dmichael (off chromium), 2014-11-20 18:20:03]

The CQ bit was checked by dmichael@chromium.org

[commit-bot: I haz the power, 2014-11-20 18:22:00]

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

[commit-bot: I haz the power, 2014-11-20 19:03:21]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-11-20 19:04:09]

Patchset 3 (id:??) landed as
https://crrev.com/b2bf826e635fd514899af6860f712d6b58cf6fdf
Cr-Commit-Position: refs/heads/master@{#305055}