WebGL: Remove some unused members in WebGLRenderingContext.

WebGL: Remove some unused members in WebGLRenderingContext.

In addition, mark TypedExtensionTracker's destructor virtual for clarity.

BUG=344393
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=167479

[Ken Russell (switch to Gerrit), 2014-02-19 20:23:10]

Thanks for this cleanup. The addition of the "virtual" keyword is not necessary,
but the other cleanups would be good. Could you please update the CL and
description?

https://codereview.chromium.org/167893008/diff/1/Source/core/html/canvas/WebG...
File Source/core/html/canvas/WebGLRenderingContext.h (right):

https://codereview.chromium.org/167893008/diff/1/Source/core/html/canvas/WebG...
Source/core/html/canvas/WebGLRenderingContext.h:614: virtual
~TypedExtensionTracker()
This change is unnecessary because ExtensionTracker's destructor is virtual.

#include <iostream>

using namespace std;

class A {
 public:
  A() { cout << "A::A()" << endl; }
  virtual ~A() { cout << "A::~A()" << endl; }
};

class B : public A {
 public:
  B() { cout << "B::B()" << endl; }
  ~B() { cout << "B::~B()" << endl; }
};

int main(int argc, char** argv) {
  A* p = new B();
  delete p;
}

[piman, 2014-02-19 20:28:24]

https://codereview.chromium.org/167893008/diff/1/Source/core/html/canvas/WebG...
File Source/core/html/canvas/WebGLRenderingContext.h (right):

https://codereview.chromium.org/167893008/diff/1/Source/core/html/canvas/WebG...
Source/core/html/canvas/WebGLRenderingContext.h:614: virtual
~TypedExtensionTracker()
On 2014/02/19 20:23:10, Ken Russell wrote:
> This change is unnecessary because ExtensionTracker's destructor is virtual.

There's a style rule in chromium land that says that overriding methods (and
destructor) should be marked virtual for clarity.
Is it not the case in blink land?

[dshwang, 2014-02-19 21:00:27]

On 2014/02/19 20:23:10, Ken Russell wrote:
> Thanks for this cleanup. The addition of the "virtual" keyword is not
necessary,
> but the other cleanups would be good. Could you please update the CL and
> description?

Thank you for review. I update description, but remain "virtual". See below.
 
> This change is unnecessary because ExtensionTracker's destructor is virtual.
> 
> #include <iostream>
> 
> using namespace std;
> 
> class A {
>  public:
>   A() { cout << "A::A()" << endl; }
>   virtual ~A() { cout << "A::~A()" << endl; }
> };
> 
> class B : public A {
>  public:
>   B() { cout << "B::B()" << endl; }
>   ~B() { cout << "B::~B()" << endl; }
> };
> 
> int main(int argc, char** argv) {
>   A* p = new B();
>   delete p;
> }

Aha, I was missing Effective C++ for a while :)
Thank you for good explanation.

On 2014/02/19 20:28:24, piman wrote:
> On 2014/02/19 20:23:10, Ken Russell wrote:
> > This change is unnecessary because ExtensionTracker's destructor is virtual.
> 
> There's a style rule in chromium land that says that overriding methods (and
> destructor) should be marked virtual for clarity.
> Is it not the case in blink land?

To match chromium's style and clarity, ihmo "virtual" is not bad. wdyt?

[Ken Russell (switch to Gerrit), 2014-02-19 21:08:13]

https://codereview.chromium.org/167893008/diff/1/Source/core/html/canvas/WebG...
File Source/core/html/canvas/WebGLRenderingContext.h (right):

https://codereview.chromium.org/167893008/diff/1/Source/core/html/canvas/WebG...
Source/core/html/canvas/WebGLRenderingContext.h:614: virtual
~TypedExtensionTracker()
On 2014/02/19 20:28:24, piman wrote:
> On 2014/02/19 20:23:10, Ken Russell wrote:
> > This change is unnecessary because ExtensionTracker's destructor is virtual.
> 
> There's a style rule in chromium land that says that overriding methods (and
> destructor) should be marked virtual for clarity.
> Is it not the case in blink land?

I'd forgotten so went back and looked. The Blink style guide is silent on this
issue so the Google C++ style guide should be used. Yes, you're right, the
derived class should use the "virtual" keyword.

However, there is no memory leak in the current code, so the CL description is
wrong. It should say something like "Mark TypedExtensionTracker's destructor
virtual for clarity."

[dshwang, 2014-02-19 21:14:54]

On 2014/02/19 21:08:13, Ken Russell wrote:
> I'd forgotten so went back and looked. The Blink style guide is silent on this
> issue so the Google C++ style guide should be used. Yes, you're right, the
> derived class should use the "virtual" keyword.
> 
> However, there is no memory leak in the current code, so the CL description is
> wrong. It should say something like "Mark TypedExtensionTracker's destructor
> virtual for clarity."

I already changed the description in the reason. :)

[Ken Russell (switch to Gerrit), 2014-02-19 22:14:51]

Thanks for updating the CL. LGTM

[dshwang, 2014-02-19 22:23:32]

On 2014/02/19 22:14:51, Ken Russell wrote:
> Thanks for updating the CL. LGTM

Thanks for lgtm!

[dshwang, 2014-02-19 22:23:36]

The CQ bit was checked by dongseong.hwang@intel.com

[commit-bot: I haz the power, 2014-02-19 22:29:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/167893008/1

[commit-bot: I haz the power, 2014-02-19 23:12:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/167893008/1

[commit-bot: I haz the power, 2014-02-20 07:49:30]

Change committed as 167479