Don't assert when ServiceWorker::from is passed an empty promise

Source/bindings/v8/V8GarbageCollected.h

 /*
  * Copyright (C) 2014 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 33 matching lines @@
         ASSERT(value->IsExternal());
         T* result = static_cast<T*>(value.As<v8::External>()->Value());
         RELEASE_ASSERT(result->m_handle == value);
         return result;
     }
 
 protected:
     V8GarbageCollected(v8::Isolate* isolate)
         : m_isolate(isolate)
         , m_handle(isolate, v8::External::New(isolate, static_cast<T*>(this)))
+        , m_releasedToV8GarbageCollector(false)
     {
     }
 
     v8::Handle<v8::External> releaseToV8GarbageCollector()
     {
         ASSERT(!m_handle.isWeak()); // Call this exactly once.
+        m_releasedToV8GarbageCollector = true;
         v8::Handle<v8::External> result = m_handle.newLocal(m_isolate);
         m_handle.setWeak(static_cast<T*>(this), &weakCallback);
         return result;
     }
 
     ~V8GarbageCollected()
     {
-        ASSERT(m_handle.isEmpty());
+        ASSERT(!m_releasedToV8GarbageCollector || m_handle.isEmpty());

[haraken, 2014/06/17 02:46:24]

Can we use m_handle.IsWeak() instead of introducing
m_releasedToV8GarbageCollector? I think you can use ASSERT(m_handle.IsWeak() ||
m_handle.IsEmpty()).

(I'm fine with this CL, but we should remove V8GarbageCollected completely.
V8GarbageCollected is used only by ScriptFunction, and it's just adding a
complexity of unnecessary abstraction.)

[falken, 2014/06/17 03:43:04]

No, m_handle.IsWeak() is false until releaseToV8GarbageCollector() is called. It
should really be (!m_handle.IsWeak() || m_handle.IsEmpty()) but I don't think
that makes sense (I would think m_handle.IsWeak implies it's not empty).
Yeah, I wonder if other classes were planning to use it too?

I also updated the ServiceWorker.cpp bit after talking with yhirano, who pointed
out it's confusing that ServiceWorker continues as normal with the empty
promise. I added a comment and changed the internal state to ContextStopped in
that case.

If things still look OK I'd like to land the patch.

     }
 
     v8::Isolate* isolate() { return m_isolate; }
 
 private:
     static void weakCallback(const v8::WeakCallbackData<v8::External, T>& data)
     {
         T* self = data.GetParameter();
         self->m_handle.clear();
         delete self;
     }
 
     v8::Isolate* m_isolate;
     ScopedPersistent<v8::External> m_handle;
+    bool m_releasedToV8GarbageCollector;
 };
 
 } // namespace WebCore
 
 #endif // V8GarbageCollected_h