Cleanup WebState and WebStateUserData API

Cleanup WebState and WebStateUserData API

Remove ios::SupportsUserData shim in favor of directly using
base::SupportsUserData since the shim didn't bring anything
except friction.

BUG=429756
Committed: https://crrev.com/7e09a22272dd1d7da967a3fa1912cd3bdafee57c
Cr-Commit-Position: refs/heads/master@{#307027}

[sdefresne, 2014-12-04 09:06:11]

sdefresne@chromium.org changed reviewers:
+ droger@chromium.org

[sdefresne, 2014-12-04 09:06:12]

Please take a look.

[droger, 2014-12-04 09:39:47]

https://codereview.chromium.org/781643004/diff/1/ios/public/provider/web/web_...
File ios/public/provider/web/web_state.h (right):

https://codereview.chromium.org/781643004/diff/1/ios/public/provider/web/web_...
ios/public/provider/web/web_state.h:15: virtual ~WebState() {}
nit:
~WebState() override {}

https://codereview.chromium.org/781643004/diff/1/ios/public/provider/web/web_...
File ios/public/provider/web/web_state_user_data.h (right):

https://codereview.chromium.org/781643004/diff/1/ios/public/provider/web/web_...
ios/public/provider/web/web_state_user_data.h:30: class WebStateUserData :
public base::SupportsUserData::Data {
Important: This class needs a virtual destructor.

[sdefresne, 2014-12-05 14:32:05]

PTAL

https://codereview.chromium.org/781643004/diff/1/ios/public/provider/web/web_...
File ios/public/provider/web/web_state.h (right):

https://codereview.chromium.org/781643004/diff/1/ios/public/provider/web/web_...
ios/public/provider/web/web_state.h:15: virtual ~WebState() {}
On 2014/12/04 at 09:39:47, droger wrote:
> nit:
> ~WebState() override {}

Done.

https://codereview.chromium.org/781643004/diff/1/ios/public/provider/web/web_...
File ios/public/provider/web/web_state_user_data.h (right):

https://codereview.chromium.org/781643004/diff/1/ios/public/provider/web/web_...
ios/public/provider/web/web_state_user_data.h:30: class WebStateUserData :
public base::SupportsUserData::Data {
On 2014/12/04 at 09:39:47, droger wrote:
> Important: This class needs a virtual destructor.

Done. But base::SupportsUserData::Data already has a virtual destructor and
since this class does not change anything, so AFAICT there is no need to
explicitly add a destructor there, right?

[droger, 2014-12-05 14:44:33]

lgtm

https://codereview.chromium.org/781643004/diff/1/ios/public/provider/web/web_...
File ios/public/provider/web/web_state_user_data.h (right):

https://codereview.chromium.org/781643004/diff/1/ios/public/provider/web/web_...
ios/public/provider/web/web_state_user_data.h:30: class WebStateUserData :
public base::SupportsUserData::Data {
On 2014/12/05 14:32:05, sdefresne wrote:
> On 2014/12/04 at 09:39:47, droger wrote:
> > Important: This class needs a virtual destructor.
> 
> Done. But base::SupportsUserData::Data already has a virtual destructor and
> since this class does not change anything, so AFAICT there is no need to
> explicitly add a destructor there, right?

I think there is a problem in this case, am I wrong?

class MyUserData : public WebStateUserData<MyUserData> {
  ...
};

WebStateUserData<MyUserData>* a = new MyUserData;
delete a;

Since the destructor is not virtual, it will not call the destructor of
MyUserData.

[droger, 2014-12-05 14:57:41]

https://codereview.chromium.org/781643004/diff/1/ios/public/provider/web/web_...
File ios/public/provider/web/web_state_user_data.h (right):

https://codereview.chromium.org/781643004/diff/1/ios/public/provider/web/web_...
ios/public/provider/web/web_state_user_data.h:30: class WebStateUserData :
public base::SupportsUserData::Data {
Ok, it turns out I was wrong.
As long as the base class has a virtual destructor, all the subclasses are
implicitly virtual too.
Sorry about that.

[sdefresne, 2014-12-05 15:11:31]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2014-12-05 15:12:39]

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

[commit-bot: I haz the power, 2014-12-05 15:54:53]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-12-05 15:55:43]

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