Oilpan: add transition types to remaining Fetch and ServiceWorker objects

Oilpan: add transition types to remaining Fetch and ServiceWorker objects

R=dominicc@chromium.org,haraken@chromium.org,tkent@chromium.org
BUG=340522
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=177938

[sof, 2014-07-10 17:58:56]

Please take a look.

The more involved SW objects have transition types already by now; following up
with the rest.

[haraken, 2014-07-10 23:25:48]

On 2014/07/10 17:58:56, sof wrote:
> Please take a look.
> 
> The more involved SW objects have transition types already by now; following
up
> with the rest.

Yeah, but I'm not quite sure if now is a good timing to introduce oilpan to
serviceworkers/. I'll delegate the decision to dominicc@ and falken@.

[dominicc (has gone to gerrit), 2014-07-11 05:08:58]

On 2014/07/10 23:25:48, haraken wrote:
> On 2014/07/10 17:58:56, sof wrote:
> > Please take a look.
> > 
> > The more involved SW objects have transition types already by now; following
> up
> > with the rest.
> 
> Yeah, but I'm not quite sure if now is a good timing to introduce oilpan to
> serviceworkers/. I'll delegate the decision to dominicc@ and falken@.

LGTM

I think the sooner we can start using Oilpan, the better and simpler our code
will be. So if this helps that happen, let's go for it.

[haraken, 2014-07-11 05:18:46]

On 2014/07/11 05:08:58, dominicc wrote:
> On 2014/07/10 23:25:48, haraken wrote:
> > On 2014/07/10 17:58:56, sof wrote:
> > > Please take a look.
> > > 
> > > The more involved SW objects have transition types already by now;
following
> > up
> > > with the rest.
> > 
> > Yeah, but I'm not quite sure if now is a good timing to introduce oilpan to
> > serviceworkers/. I'll delegate the decision to dominicc@ and falken@.
> 
> LGTM
> 
> I think the sooner we can start using Oilpan, the better and simpler our code
> will be. So if this helps that happen, let's go for it.

ok, that's great. Let me take a close look at this CL.

[haraken, 2014-07-11 05:39:12]

LGTM

https://codereview.chromium.org/384633002/diff/40002/Source/modules/servicewo...
File Source/modules/serviceworkers/FetchEvent.h (right):

https://codereview.chromium.org/384633002/diff/40002/Source/modules/servicewo...
Source/modules/serviceworkers/FetchEvent.h:43: RefPtr<RespondWithObserver>
m_observer;

You can consider moving RespondWithObserver in a follow-up CL.

https://codereview.chromium.org/384633002/diff/40002/Source/modules/servicewo...
File Source/modules/serviceworkers/Response.h (right):

https://codereview.chromium.org/384633002/diff/40002/Source/modules/servicewo...
Source/modules/serviceworkers/Response.h:26: class Response FINAL : public
RefCountedWillBeGarbageCollectedFinalized<Response>, public ScriptWrappable {

Can we remove the destructor from Response and make it
RefCountedWillBeGarbageCollected?

https://codereview.chromium.org/384633002/diff/40002/Source/modules/servicewo...
File Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp (right):

https://codereview.chromium.org/384633002/diff/40002/Source/modules/servicewo...
Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp:109: return
m_clients;

m_clients.release()

[tkent, 2014-07-11 05:47:27]

lgtm

https://codereview.chromium.org/384633002/diff/40002/Source/modules/servicewo...
File Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp (right):

https://codereview.chromium.org/384633002/diff/40002/Source/modules/servicewo...
Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp:109: return
m_clients;
On 2014/07/11 05:39:12, haraken wrote:
> 
> m_clients.release()

I don't think so.  This function should not move ownership.

[haraken, 2014-07-11 05:48:10]

https://codereview.chromium.org/384633002/diff/40002/Source/modules/servicewo...
File Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp (right):

https://codereview.chromium.org/384633002/diff/40002/Source/modules/servicewo...
Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp:109: return
m_clients;
On 2014/07/11 05:47:27, tkent wrote:
> On 2014/07/11 05:39:12, haraken wrote:
> > 
> > m_clients.release()
> 
> I don't think so.  This function should not move ownership.

oh, you're right.

[sof, 2014-07-11 06:11:21]

https://codereview.chromium.org/384633002/diff/40002/Source/modules/servicewo...
File Source/modules/serviceworkers/FetchEvent.h (right):

https://codereview.chromium.org/384633002/diff/40002/Source/modules/servicewo...
Source/modules/serviceworkers/FetchEvent.h:43: RefPtr<RespondWithObserver>
m_observer;
On 2014/07/11 05:39:11, haraken wrote:
> 
> You can consider moving RespondWithObserver in a follow-up CL.

It looks feasible to move the ContextLifecycleObserver objects herein.

https://codereview.chromium.org/384633002/diff/40002/Source/modules/servicewo...
File Source/modules/serviceworkers/Response.h (right):

https://codereview.chromium.org/384633002/diff/40002/Source/modules/servicewo...
Source/modules/serviceworkers/Response.h:26: class Response FINAL : public
RefCountedWillBeGarbageCollectedFinalized<Response>, public ScriptWrappable {
On 2014/07/11 05:39:12, haraken wrote:
> 
> Can we remove the destructor from Response and make it
> RefCountedWillBeGarbageCollected?

Yes, that is possible. Done.

(It wasn't possible before r177817 (https://codereview.chromium.org/373613004)
that landed yesterday, but it now is. I failed to pick up on that when merging.)

[sof, 2014-07-11 06:21:49]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-07-11 06:22:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sigbjornf@opera.com/384633002/50001

[commit-bot: I haz the power, 2014-07-11 07:29:33]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)

[commit-bot: I haz the power, 2014-07-11 07:35:27]

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

[commit-bot: I haz the power, 2014-07-11 07:35:28]

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

[sof, 2014-07-11 07:43:04]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-07-11 07:43:53]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sigbjornf@opera.com/384633002/50001

[commit-bot: I haz the power, 2014-07-11 07:44:14]

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

[commit-bot: I haz the power, 2014-07-11 07:44:15]

Failed to apply patch for Source/bindings/core/v8/Dictionary.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/bindings/core/v8/Dictionary.cpp
  Hunk #1 FAILED at 584.
  1 out of 1 hunk FAILED -- saving rejects to file
Source/bindings/core/v8/Dictionary.cpp.rej

Patch:       Source/bindings/core/v8/Dictionary.cpp
Index: Source/bindings/core/v8/Dictionary.cpp
diff --git a/Source/bindings/core/v8/Dictionary.cpp
b/Source/bindings/core/v8/Dictionary.cpp
index
bcdb11df661b17d28fe53000590b0d594dcab7a9..d69bb24fc935721f78920f8a3e694d2150fef234
100644
--- a/Source/bindings/core/v8/Dictionary.cpp
+++ b/Source/bindings/core/v8/Dictionary.cpp
@@ -584,7 +584,7 @@ bool Dictionary::get(const String& key, Dictionary& value)
const
     return true;
 }
 
-bool Dictionary::get(const String& key, RefPtr<Headers>& value) const
+bool Dictionary::get(const String& key, RefPtrWillBeMember<Headers>& value)
const
 {
     v8::Local<v8::Value> v8Value;
     if (!getKey(key, v8Value))

[sof, 2014-07-11 10:08:47]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-07-11 10:09:44]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sigbjornf@opera.com/384633002/70001

[commit-bot: I haz the power, 2014-07-11 11:15:10]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)
  win_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_compile_dbg/bu...)

[commit-bot: I haz the power, 2014-07-11 11:17:52]

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

[commit-bot: I haz the power, 2014-07-11 11:17:53]

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

[sof, 2014-07-11 11:27:46]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-07-11 11:28:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sigbjornf@opera.com/384633002/70001

[commit-bot: I haz the power, 2014-07-11 11:29:28]

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

[commit-bot: I haz the power, 2014-07-11 11:29:29]

Failed to apply patch for Source/modules/serviceworkers/Headers.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/modules/serviceworkers/Headers.cpp
  Hunk #2 FAILED at 50.
  Hunk #3 succeeded at 324 (offset -2 lines).
  1 out of 3 hunks FAILED -- saving rejects to file
Source/modules/serviceworkers/Headers.cpp.rej

Patch:       Source/modules/serviceworkers/Headers.cpp
Index: Source/modules/serviceworkers/Headers.cpp
diff --git a/Source/modules/serviceworkers/Headers.cpp
b/Source/modules/serviceworkers/Headers.cpp
index
133e4933e7224006f6ca99b646a5356e546180ec..6478d73e2c171f04af65d71cef1e3f62c96a381c
100644
--- a/Source/modules/serviceworkers/Headers.cpp
+++ b/Source/modules/serviceworkers/Headers.cpp
@@ -17,32 +17,32 @@
 
 namespace WebCore {
 
-PassRefPtr<Headers> Headers::create()
+PassRefPtrWillBeRawPtr<Headers> Headers::create()
 {
-    return adoptRef(new Headers);
+    return adoptRefWillBeNoop(new Headers);
 }
 
-PassRefPtr<Headers> Headers::create(ExceptionState&)
+PassRefPtrWillBeRawPtr<Headers> Headers::create(ExceptionState&)
 {
     return create();
 }
 
-PassRefPtr<Headers> Headers::create(const Headers* init, ExceptionState&
exceptionState)
+PassRefPtrWillBeRawPtr<Headers> Headers::create(const Headers* init,
ExceptionState& exceptionState)
 {
     // "The Headers(|init|) constructor, when invoked, must run these steps:"
     // "1. Let |headers| be a new Headers object."
-    RefPtr<Headers> headers = create();
+    RefPtrWillBeRawPtr<Headers> headers = create();
     // "2. If |init| is given, fill headers with |init|. Rethrow any
exception."
     headers->fillWith(init, exceptionState);
     // "3. Return |headers|."
     return headers.release();
 }
 
-PassRefPtr<Headers> Headers::create(const Dictionary& init, ExceptionState&
exceptionState)
+PassRefPtrWillBeRawPtr<Headers> Headers::create(const Dictionary& init,
ExceptionState& exceptionState)
 {
     // "The Headers(|init|) constructor, when invoked, must run these steps:"
     // "1. Let |headers| be a new Headers object."
-    RefPtr<Headers> headers = create();
+    RefPtrWillBeRawPtr<Headers> headers = create();
     // "2. If |init| is given, fill headers with |init|. Rethrow any
exception."
     headers->fillWith(init, exceptionState);
     // "3. Return |headers|."
@@ -50,22 +50,20 @@ PassRefPtr<Headers> Headers::create(const Dictionary& init,
ExceptionState& exce
 }
 
 // Called when creating Request.
-PassRefPtr<Headers> Headers::create(FetchHeaderList* headerList)
+PassRefPtrWillBeRawPtr<Headers> Headers::create(FetchHeaderList* headerList)
 {
-    return adoptRef(new Headers(headerList));
+    return adoptRefWillBeNoop(new Headers(headerList));
 }
 
-PassRefPtr<Headers> Headers::createCopy() const
+PassRefPtrWillBeRawPtr<Headers> Headers::createCopy() const
 {
-    RefPtr<FetchHeaderList> headerList = m_headerList->createCopy();
-    RefPtr<Headers> headers = create(headerList.get());
+    RefPtrWillBeRawPtr<FetchHeaderList> headerList =
m_headerList->createCopy();
+    RefPtrWillBeRawPtr<Headers> headers = create(headerList.get());
     headers->m_guard = m_guard;
     return headers.release();
 }
 
-Headers::~Headers()
-{
-}
+DEFINE_EMPTY_DESTRUCTOR_WILL_BE_REMOVED(Headers);
 
 unsigned long Headers::size() const
 {
@@ -326,4 +324,9 @@ void
Headers::forEachInternal(PassOwnPtr<HeadersForEachCallback> callback, Scrip
     }
 }
 
+void Headers::trace(Visitor* visitor)
+{
+    visitor->trace(m_headerList);
+}
+
 } // namespace WebCore

[sof, 2014-07-11 11:38:51]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-07-11 11:39:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sigbjornf@opera.com/384633002/90001

[commit-bot: I haz the power, 2014-07-11 12:43:25]

Change committed as 177938