IDBObserver: Lifetime Management: Adding Observer

third_party/WebKit/public/platform/modules/indexeddb/WebIDBObserver.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef WebRemotePlaybackState_h
-#define WebRemotePlaybackState_h
+#ifndef WebIDBObserver_h
+#define WebIDBObserver_h
 
 namespace blink {
 
-enum class WebRemotePlaybackState {
-    Connected = 0,
-    Disconnected,
+class WebIDBObserver {
+public:
+    virtual ~WebIDBObserver() {}
+    virtual void removeObserver(int32_t id) = 0;

[Marijn Kruisselbrink, 2016/06/28 18:58:31]

I'm a bit confused about this API. First of all you should add some comments
describing what this class is used for, what it's ownership and thread safety
guarantees etc are. 

But then specifically this removeObserver method I don't understand. As far as I
can tell this method is called at most once on any WebIDBObserver instance, and
that one time is right before the instance is destroyed. If that is the case,
couldn't you just store the observer id in the WebIDBObserverImpl instance, and
use the destructor being called to do what you currently have removeObserver
for?

Essentially it seems WebIDBObserver represents a particular observer() call, and
as such already is associated with an id, so no need to pass the id to
removeObserver. And on top of that the content layer will already destroy the
WebIDBObserver instance when it calls removeObserver. So you could just use the
destructor of WebIDBObserver to do what removeObserver currently does?

Although I guess you wouldn't actually have an id until after you've transferred
ownership of the WebIDBObserver instance to the content layer, so setting the ID
would be a bit tricky. If you decide to go this way you could either add a
setId() method to WebIDBObserver, or have IDBObserver hold on to a raw pointer
to the WebIDBObserverImpl after passing it to the content layer and use that to
store the returned id in the WebIDBObserverImpl.

[palakj1, 2016/06/29 23:02:41]

Not really sure about this. We hold reference to WebIDBObserver objects each of
which in turn holds IDBObserver. Isn't that a little weird? Maybe I can discuss
this with you.

 };
 
 } // namespace blink
 
-#endif // WebRemotePlaybackState_h
+#endif // WebIDBObserver_h