Make ScriptPromiseResolver RefCountedWillBeRefCountedGarbageCollected.

Make ScriptPromiseResolver RefCountedWillBeRefCountedGarbageCollected.

BUG=
TBR=jochen@chromium.org

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

[tasak, 2014-12-09 10:38:27]

tasak@google.com changed reviewers:
+ haraken@chromium.org

[tasak, 2014-12-09 10:38:29]

https://codereview.chromium.org/783423003/diff/1/Source/modules/crypto/Crypto...
File Source/modules/crypto/CryptoResultImpl.cpp (right):

https://codereview.chromium.org/783423003/diff/1/Source/modules/crypto/Crypto...
Source/modules/crypto/CryptoResultImpl.cpp:78: RawPtr<CryptoResultImpl>
m_result;
Since CryptResultImpl has a persistent member: m_resolver, which points 
WeakResolver, RawPtr.

[tasak, 2014-12-11 06:17:37]

Patchset #3 (id:40001) has been deleted

[tasak, 2014-12-11 09:54:05]

Patchset #3 (id:60001) has been deleted

[tasak, 2014-12-16 04:37:59]

Patchset #2 (id:20001) has been deleted

[tasak, 2014-12-16 04:38:11]

Patchset #2 (id:80001) has been deleted

[tasak, 2014-12-16 04:38:20]

Patchset #2 (id:100001) has been deleted

[tasak, 2014-12-16 04:40:31]

Patchset #2 (id:100001) has been deleted

[tasak, 2014-12-16 04:40:53]

Patchset #3 (id:120001) has been deleted

[tasak, 2014-12-16 04:41:02]

Patchset #2 (id:100001) has been deleted

[tasak, 2014-12-16 04:57:01]

Patchset #2 (id:120002) has been deleted

[tasak, 2014-12-16 04:57:54]

Patchset #2 (id:140001) has been deleted

[tasak, 2014-12-16 05:17:23]

Would you review this CL?

[haraken, 2014-12-16 09:43:29]

https://codereview.chromium.org/783423003/diff/160001/Source/modules/crypto/C...
File Source/modules/crypto/CryptoResultImpl.cpp (right):

https://codereview.chromium.org/783423003/diff/160001/Source/modules/crypto/C...
Source/modules/crypto/CryptoResultImpl.cpp:81: virtual void trace(Visitor*
visitor)

Add override.

The same comment for other overridden trace methods.

https://codereview.chromium.org/783423003/diff/160001/Source/modules/crypto/C...
File Source/modules/crypto/CryptoResultImpl.h (right):

https://codereview.chromium.org/783423003/diff/160001/Source/modules/crypto/C...
Source/modules/crypto/CryptoResultImpl.h:87:
OwnPtr<Persistent<ScriptPromiseResolver>> m_resolver;

I guess this is not safe for the same reason why we cannot use
RefPtr<ScriptPromiseResolver>; i.e., Persistent handle is not thread-safe.

We could use OwnPtr<CrossThreadPersistent<ScriptPromiseResolver>>, but I'd
suggest to simply use ScriptPromiseResolver* in both oilpan and non-oilpan. This
raw pointer is as safe in oilpan as in non-oilpan.

I understand this is nasty but a right fix would be to replace the
ScriptPromiseResolver* with a ScriptPromise, instead of adding a more complex
work-around to protect the ScriptPromiseResolver.

https://codereview.chromium.org/783423003/diff/160001/Source/modules/webaudio...
File Source/modules/webaudio/AudioContext.cpp (right):

https://codereview.chromium.org/783423003/diff/160001/Source/modules/webaudio...
Source/modules/webaudio/AudioContext.cpp:955: for
(RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver : m_resumeResolvers) {

Can we use auto? Or we should use RefPtrWillBeMember<ScriptPromiseResolver>.

The same comment for other parts in this CL.

https://codereview.chromium.org/783423003/diff/160001/Source/modules/webmidi/...
File Source/modules/webmidi/MIDIAccessInitializer.h (right):

https://codereview.chromium.org/783423003/diff/160001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIAccessInitializer.h:42:
RefPtrWillBeRawPtr<MIDIAccessInitializer> p = adoptRefWillBeNoop(new
MIDIAccessInitializer(scriptState, options));

p => promise

https://codereview.chromium.org/783423003/diff/160001/public/platform/WebCryp...
File public/platform/WebCrypto.h (right):

https://codereview.chromium.org/783423003/diff/160001/public/platform/WebCryp...
public/platform/WebCrypto.h:42: #include "platform/heap/Heap.h"

Heap.h => Handle.h

Our convention is just to include Handle.h.

[tasak, 2014-12-16 11:54:28]

Thank you for review.

https://codereview.chromium.org/783423003/diff/160001/Source/modules/crypto/C...
File Source/modules/crypto/CryptoResultImpl.cpp (right):

https://codereview.chromium.org/783423003/diff/160001/Source/modules/crypto/C...
Source/modules/crypto/CryptoResultImpl.cpp:81: virtual void trace(Visitor*
visitor)
On 2014/12/16 09:43:29, haraken wrote:
> 
> Add override.
> 
> The same comment for other overridden trace methods.

Done.

https://codereview.chromium.org/783423003/diff/160001/Source/modules/crypto/C...
File Source/modules/crypto/CryptoResultImpl.h (right):

https://codereview.chromium.org/783423003/diff/160001/Source/modules/crypto/C...
Source/modules/crypto/CryptoResultImpl.h:87:
OwnPtr<Persistent<ScriptPromiseResolver>> m_resolver;
On 2014/12/16 09:43:29, haraken wrote:
> 
> I guess this is not safe for the same reason why we cannot use
> RefPtr<ScriptPromiseResolver>; i.e., Persistent handle is not thread-safe.
> 
> We could use OwnPtr<CrossThreadPersistent<ScriptPromiseResolver>>, but I'd
> suggest to simply use ScriptPromiseResolver* in both oilpan and non-oilpan.
This
> raw pointer is as safe in oilpan as in non-oilpan.
> 
> I understand this is nasty but a right fix would be to replace the
> ScriptPromiseResolver* with a ScriptPromise, instead of adding a more complex
> work-around to protect the ScriptPromiseResolver.

Done.

https://codereview.chromium.org/783423003/diff/160001/Source/modules/webaudio...
File Source/modules/webaudio/AudioContext.cpp (right):

https://codereview.chromium.org/783423003/diff/160001/Source/modules/webaudio...
Source/modules/webaudio/AudioContext.cpp:955: for
(RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver : m_resumeResolvers) {
On 2014/12/16 09:43:29, haraken wrote:
> 
> Can we use auto? Or we should use RefPtrWillBeMember<ScriptPromiseResolver>.
> 
> The same comment for other parts in this CL.

Done.

https://codereview.chromium.org/783423003/diff/160001/Source/modules/webmidi/...
File Source/modules/webmidi/MIDIAccessInitializer.h (right):

https://codereview.chromium.org/783423003/diff/160001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIAccessInitializer.h:42:
RefPtrWillBeRawPtr<MIDIAccessInitializer> p = adoptRefWillBeNoop(new
MIDIAccessInitializer(scriptState, options));
On 2014/12/16 09:43:29, haraken wrote:
> 
> p => promise


p => resolver

https://codereview.chromium.org/783423003/diff/160001/public/platform/WebCryp...
File public/platform/WebCrypto.h (right):

https://codereview.chromium.org/783423003/diff/160001/public/platform/WebCryp...
public/platform/WebCrypto.h:42: #include "platform/heap/Heap.h"
On 2014/12/16 09:43:29, haraken wrote:
> 
> Heap.h => Handle.h
> 
> Our convention is just to include Handle.h.

Done.

[haraken, 2014-12-16 13:52:45]

LGTM

https://codereview.chromium.org/783423003/diff/180001/LayoutTests/media/encry...
File
LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeys-with-session.html
(right):

https://codereview.chromium.org/783423003/diff/180001/LayoutTests/media/encry...
LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeys-with-session.html:50:
// immediately. So numActiveDOMObjectsCreate() <= 2.
Simplify the comment.

// In non-Oilpan, numActiveDOMObjetsCreated() == 0.
// In Oilpan, numActiveDOMObjetsCreated() == 2 (1 MediaKeysInitializer and 1
MediaKeySystemAccessInitializer).

The same comment for other comments in test files.

https://codereview.chromium.org/783423003/diff/180001/Source/modules/crypto/C...
File Source/modules/crypto/CryptoResultImpl.cpp (right):

https://codereview.chromium.org/783423003/diff/180001/Source/modules/crypto/C...
Source/modules/crypto/CryptoResultImpl.cpp:132: ScriptPromiseResolver* resolver
= m_resolver;

This is unnecessary.

[haraken, 2014-12-16 13:53:17]

haraken@chromium.org changed reviewers:
+ oilpan-reviews@chromium.org

[haraken, 2014-12-16 13:53:18]

Let's cc oilpan-reviews@ for oilpan related changes.

[haraken, 2014-12-16 13:54:33]

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
File Source/bindings/core/v8/ScriptPromiseResolver.h (right):

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
Source/bindings/core/v8/ScriptPromiseResolver.h:28: class ScriptPromiseResolver
: public RefCountedWillBeRefCountedGarbageCollected<ScriptPromiseResolver>,
public ActiveDOMObject {

Does this need to be RefCountedGarbageCollected? I guess this can be
RefCountedWillBeGarbageCollectedFinalized.

[sof, 2014-12-16 21:34:58]

sigbjornf@opera.com changed reviewers:
+ sigbjornf@opera.com

[sof, 2014-12-16 21:34:59]

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
File Source/bindings/core/v8/ScriptPromiseResolverTest.cpp (right):

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
Source/bindings/core/v8/ScriptPromiseResolverTest.cpp:83:
RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver;
Add " = nullptr;"

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
Source/bindings/core/v8/ScriptPromiseResolverTest.cpp:131:
RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver;
add " = nullptr;"

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
Source/bindings/core/v8/ScriptPromiseResolverTest.cpp:179:
RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver;
ditto

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
Source/bindings/core/v8/ScriptPromiseResolverTest.cpp:210:
RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver;
If you really need a transition type here (and the rest of this #if block), then
nullptr-initializing it is appropriate.

https://codereview.chromium.org/783423003/diff/180001/Source/modules/crypto/C...
File Source/modules/crypto/CryptoResultImpl.h (right):

https://codereview.chromium.org/783423003/diff/180001/Source/modules/crypto/C...
Source/modules/crypto/CryptoResultImpl.h:83: ScriptPromiseResolver* m_resolver;
Shouldn't this be trace()d?

https://codereview.chromium.org/783423003/diff/180001/Source/modules/webmidi/...
File Source/modules/webmidi/MIDIAccessInitializer.h (right):

https://codereview.chromium.org/783423003/diff/180001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIAccessInitializer.h:42:
RefPtrWillBeRawPtr<MIDIAccessInitializer> resolver = adoptRefWillBeNoop(new
MIDIAccessInitializer(scriptState, options));
Not directly overlapping with this change, but how about moving this to the
implementation file at the same time? It seems like too much code to have in a
header file for no particular reason (and why isn't it called create()?)

https://codereview.chromium.org/783423003/diff/180001/Source/platform/exporte...
File Source/platform/exported/WebCryptoResult.cpp (right):

https://codereview.chromium.org/783423003/diff/180001/Source/platform/exporte...
Source/platform/exported/WebCryptoResult.cpp:36: #include "wtf/MainThread.h"
Why is this needed now?

[sof, 2014-12-16 22:07:05]

https://codereview.chromium.org/783423003/diff/180001/Source/modules/webmidi/...
File Source/modules/webmidi/MIDIAccessInitializer.h (right):

https://codereview.chromium.org/783423003/diff/180001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIAccessInitializer.h:42:
RefPtrWillBeRawPtr<MIDIAccessInitializer> resolver = adoptRefWillBeNoop(new
MIDIAccessInitializer(scriptState, options));
On 2014/12/16 21:34:59, sof wrote:
> Not directly overlapping with this change, but how about moving this to the
> implementation file at the same time? It seems like too much code to have in a
> header file for no particular reason (and why isn't it called create()?)

This block of code repeats in a number of create() methods in this CL - possible
to share via a function template? (not for this CL though.)

[tasak, 2014-12-17 08:38:58]

Patchset #4 (id:200001) has been deleted

[tasak, 2014-12-17 08:40:08]

Thank you for review.

https://codereview.chromium.org/783423003/diff/180001/LayoutTests/media/encry...
File
LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeys-with-session.html
(right):

https://codereview.chromium.org/783423003/diff/180001/LayoutTests/media/encry...
LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeys-with-session.html:50:
// immediately. So numActiveDOMObjectsCreate() <= 2.
On 2014/12/16 13:52:45, haraken wrote:
> Simplify the comment.
> 
> // In non-Oilpan, numActiveDOMObjetsCreated() == 0.
> // In Oilpan, numActiveDOMObjetsCreated() == 2 (1 MediaKeysInitializer and 1
> MediaKeySystemAccessInitializer).
> 
> The same comment for other comments in test files.

Done.

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
File Source/bindings/core/v8/ScriptPromiseResolver.h (right):

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
Source/bindings/core/v8/ScriptPromiseResolver.h:28: class ScriptPromiseResolver
: public RefCountedWillBeRefCountedGarbageCollected<ScriptPromiseResolver>,
public ActiveDOMObject {
On 2014/12/16 13:54:33, haraken wrote:
> 
> Does this need to be RefCountedGarbageCollected? I guess this can be
> RefCountedWillBeGarbageCollectedFinalized.

ScriptPromiseResolver uses ref/deref. So need to be RefCountedGarbageCollected.

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
File Source/bindings/core/v8/ScriptPromiseResolverTest.cpp (right):

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
Source/bindings/core/v8/ScriptPromiseResolverTest.cpp:83:
RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver;
On 2014/12/16 21:34:59, sof wrote:
> Add " = nullptr;"

Done.

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
Source/bindings/core/v8/ScriptPromiseResolverTest.cpp:131:
RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver;
On 2014/12/16 21:34:59, sof wrote:
> add " = nullptr;"

Done.

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
Source/bindings/core/v8/ScriptPromiseResolverTest.cpp:179:
RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver;
On 2014/12/16 21:34:59, sof wrote:
> ditto

Done.

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
Source/bindings/core/v8/ScriptPromiseResolverTest.cpp:210:
RefPtrWillBeRawPtr<ScriptPromiseResolver> resolver;
On 2014/12/16 21:34:59, sof wrote:
> If you really need a transition type here (and the rest of this #if block),
then
> nullptr-initializing it is appropriate.

Done.

https://codereview.chromium.org/783423003/diff/180001/Source/modules/crypto/C...
File Source/modules/crypto/CryptoResultImpl.cpp (right):

https://codereview.chromium.org/783423003/diff/180001/Source/modules/crypto/C...
Source/modules/crypto/CryptoResultImpl.cpp:132: ScriptPromiseResolver* resolver
= m_resolver;
On 2014/12/16 13:52:45, haraken wrote:
> 
> This is unnecessary.

Done.

https://codereview.chromium.org/783423003/diff/180001/Source/modules/crypto/C...
File Source/modules/crypto/CryptoResultImpl.h (right):

https://codereview.chromium.org/783423003/diff/180001/Source/modules/crypto/C...
Source/modules/crypto/CryptoResultImpl.h:83: ScriptPromiseResolver* m_resolver;
On 2014/12/16 21:34:59, sof wrote:
> Shouldn't this be trace()d? 

I discussed with haraken@ and yhiarano@.
So we concluded that we don't need to make it "OwnPtr<Persistent>".
Instead, we quickly clear m_resolver when resolving or rejecting.

https://codereview.chromium.org/783423003/diff/180001/Source/modules/webmidi/...
File Source/modules/webmidi/MIDIAccessInitializer.h (right):

https://codereview.chromium.org/783423003/diff/180001/Source/modules/webmidi/...
Source/modules/webmidi/MIDIAccessInitializer.h:42:
RefPtrWillBeRawPtr<MIDIAccessInitializer> resolver = adoptRefWillBeNoop(new
MIDIAccessInitializer(scriptState, options));
On 2014/12/16 22:07:05, sof wrote:
> On 2014/12/16 21:34:59, sof wrote:
> > Not directly overlapping with this change, but how about moving this to the
> > implementation file at the same time? It seems like too much code to have in
a
> > header file for no particular reason (and why isn't it called create()?)
> 
> This block of code repeats in a number of create() methods in this CL -
possible
> to share via a function template? (not for this CL though.)

I see. I will talk with yhirano@.

https://codereview.chromium.org/783423003/diff/180001/Source/platform/exporte...
File Source/platform/exported/WebCryptoResult.cpp (right):

https://codereview.chromium.org/783423003/diff/180001/Source/platform/exporte...
Source/platform/exported/WebCryptoResult.cpp:36: #include "wtf/MainThread.h"
On 2014/12/16 21:34:59, sof wrote:
> Why is this needed now?

We don't need. Removed.

[sof, 2014-12-17 09:32:03]

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
File Source/bindings/core/v8/ScriptPromiseResolver.h (right):

https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
Source/bindings/core/v8/ScriptPromiseResolver.h:28: class ScriptPromiseResolver
: public RefCountedWillBeRefCountedGarbageCollected<ScriptPromiseResolver>,
public ActiveDOMObject {
On 2014/12/17 08:40:08, tasak wrote:
> On 2014/12/16 13:54:33, haraken wrote:
> > 
> > Does this need to be RefCountedGarbageCollected? I guess this can be
> > RefCountedWillBeGarbageCollectedFinalized.
> 
> ScriptPromiseResolver uses ref/deref. So need to be
RefCountedGarbageCollected.

Could you point out where & how?

https://codereview.chromium.org/783423003/diff/220001/Source/modules/battery/...
File Source/modules/battery/BatteryManager.cpp (right):

https://codereview.chromium.org/783423003/diff/220001/Source/modules/battery/...
Source/modules/battery/BatteryManager.cpp:157: visitor->trace(m_resolver);
nit: move this one line up.

https://codereview.chromium.org/783423003/diff/220001/Source/modules/crypto/C...
File Source/modules/crypto/CryptoResultImpl.cpp (right):

https://codereview.chromium.org/783423003/diff/220001/Source/modules/crypto/C...
Source/modules/crypto/CryptoResultImpl.cpp:81: virtual void trace(Visitor*
visitor) override
Redundant override, i think.

[haraken, 2014-12-17 15:54:36]

On 2014/12/17 09:32:03, sof wrote:
>
https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
> File Source/bindings/core/v8/ScriptPromiseResolver.h (right):
> 
>
https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
> Source/bindings/core/v8/ScriptPromiseResolver.h:28: class
ScriptPromiseResolver
> : public RefCountedWillBeRefCountedGarbageCollected<ScriptPromiseResolver>,
> public ActiveDOMObject {
> On 2014/12/17 08:40:08, tasak wrote:
> > On 2014/12/16 13:54:33, haraken wrote:
> > > 
> > > Does this need to be RefCountedGarbageCollected? I guess this can be
> > > RefCountedWillBeGarbageCollectedFinalized.
> > 
> > ScriptPromiseResolver uses ref/deref. So need to be
> RefCountedGarbageCollected.
> 
> Could you point out where & how?

Here:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

We sometimes need to explicitly keep ScriptPromiseResolver alive while a Promise
is pending (see the comment in the above code). In non-oilpan, we're doing that
using ref()/deref(). In oilpan, we need to use a self-keep-alive persistent
handle. The easiest way to do that is to make ScriptPromiseResolver a
RefCountedGarbageCollected and keep using ref()/deref() (which creates a
self-keep-alive handle for us).

[sof, 2014-12-17 16:12:42]

On 2014/12/17 15:54:36, haraken wrote:
> On 2014/12/17 09:32:03, sof wrote:
> >
>
https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
> > File Source/bindings/core/v8/ScriptPromiseResolver.h (right):
> > 
> >
>
https://codereview.chromium.org/783423003/diff/180001/Source/bindings/core/v8...
> > Source/bindings/core/v8/ScriptPromiseResolver.h:28: class
> ScriptPromiseResolver
> > : public RefCountedWillBeRefCountedGarbageCollected<ScriptPromiseResolver>,
> > public ActiveDOMObject {
> > On 2014/12/17 08:40:08, tasak wrote:
> > > On 2014/12/16 13:54:33, haraken wrote:
> > > > 
> > > > Does this need to be RefCountedGarbageCollected? I guess this can be
> > > > RefCountedWillBeGarbageCollectedFinalized.
> > > 
> > > ScriptPromiseResolver uses ref/deref. So need to be
> > RefCountedGarbageCollected.
> > 
> > Could you point out where & how?
> 
> Here:
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> 
> We sometimes need to explicitly keep ScriptPromiseResolver alive while a
Promise
> is pending (see the comment in the above code). In non-oilpan, we're doing
that
> using ref()/deref(). In oilpan, we need to use a self-keep-alive persistent
> handle. The easiest way to do that is to make ScriptPromiseResolver a
> RefCountedGarbageCollected and keep using ref()/deref() (which creates a
> self-keep-alive handle for us).

Thanks, I should have remembered that keep-alive mechanism :) Makes sense to
reuse RefCountedGarbageCollected<>.

[tasak, 2014-12-18 04:30:37]

Thank you for review.

https://codereview.chromium.org/783423003/diff/220001/Source/modules/battery/...
File Source/modules/battery/BatteryManager.cpp (right):

https://codereview.chromium.org/783423003/diff/220001/Source/modules/battery/...
Source/modules/battery/BatteryManager.cpp:157: visitor->trace(m_resolver);
On 2014/12/17 09:32:03, sof wrote:
> nit: move this one line up.

Done.

https://codereview.chromium.org/783423003/diff/220001/Source/modules/crypto/C...
File Source/modules/crypto/CryptoResultImpl.cpp (right):

https://codereview.chromium.org/783423003/diff/220001/Source/modules/crypto/C...
Source/modules/crypto/CryptoResultImpl.cpp:81: virtual void trace(Visitor*
visitor) override
On 2014/12/17 09:32:03, sof wrote:
> Redundant override, i think.

Done.

[haraken, 2014-12-18 04:31:30]

Still LGTM

[haraken, 2014-12-18 04:31:35]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-12-18 04:31:51]

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

[commit-bot: I haz the power, 2014-12-18 04:38:47]

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

[commit-bot: I haz the power, 2014-12-18 04:38:49]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/2...)

[tasak, 2014-12-18 07:03:31]

tasak@google.com changed reviewers:
+ jochen@chromium.org

[tasak, 2014-12-18 07:03:32]

jochen, would you review public/platform/WebCrypto.h change?

[haraken, 2014-12-18 08:07:36]

On 2014/12/18 07:03:32, tasak wrote:
> jochen, would you review public/platform/WebCrypto.h change?

Or you can TBR.

[tasak, 2014-12-18 08:56:56]

On 2014/12/18 08:07:36, haraken wrote:
> On 2014/12/18 07:03:32, tasak wrote:
> > jochen, would you review public/platform/WebCrypto.h change?
> 
> Or you can TBR.

I see.

I will TBR=jochen.

[tasak, 2014-12-18 08:57:30]

The CQ bit was checked by tasak@google.com

[commit-bot: I haz the power, 2014-12-18 08:58:17]

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

[commit-bot: I haz the power, 2014-12-18 09:02:49]

Committed patchset #5 (id:240001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187439