Remove ContextLifecycleObserver from ScopedWindowFocusAllowedIndicator

third_party/WebKit/Source/core/dom/ScopedWindowFocusAllowedIndicator.h

 // Copyright 2014 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 ScopedWindowFocusAllowedIndicator_h
 #define ScopedWindowFocusAllowedIndicator_h
 
 #include "core/dom/ContextLifecycleObserver.h"

[sof, 2016/12/07 08:02:18]

unused

 #include "core/dom/ExecutionContext.h"
 #include "wtf/Noncopyable.h"
 
 namespace blink {
 
 class ScopedWindowFocusAllowedIndicator final {
   USING_FAST_MALLOC(ScopedWindowFocusAllowedIndicator);
   WTF_MAKE_NONCOPYABLE(ScopedWindowFocusAllowedIndicator);
 
  public:
   explicit ScopedWindowFocusAllowedIndicator(ExecutionContext* executionContext)
-      : m_observer(new Observer(executionContext)) {}
-  ~ScopedWindowFocusAllowedIndicator() { m_observer->dispose(); }
+      : m_executionContext(executionContext) {
+    executionContext->allowWindowInteraction();
+  }
+  ~ScopedWindowFocusAllowedIndicator() {
+    m_executionContext->consumeWindowInteraction();
+  }
 
  private:
-  class Observer final : public GarbageCollected<Observer>,
-                         public ContextLifecycleObserver {
-    USING_GARBAGE_COLLECTED_MIXIN(Observer);
-
-   public:
-    explicit Observer(ExecutionContext* executionContext)
-        : ContextLifecycleObserver(executionContext) {
-      if (executionContext)
-        executionContext->allowWindowInteraction();
-    }
-
-    void dispose() {
-      if (getExecutionContext())
-        getExecutionContext()->consumeWindowInteraction();
-    }
-  };
-
-  // In Oilpan, destructors are not allowed to touch other on-heap objects.
-  // The Observer indirection is needed to keep
-  // ScopedWindowFocusAllowedIndicator off-heap and thus allows its destructor
-  // to call getExecutionContext()->consumeWindowInteraction().

[haraken, 2016/12/07 07:49:17]

I don't understand this comment. The observer indirection is just not needed,
right?

[sof, 2016/12/07 08:02:18]

We need(ed) to inject an on-heap ContextLifecycleObserver in some way, so I do
think it was needed without shortening out the ExecutionContext, like you've
done now.

-  Persistent<Observer> m_observer;
+  // This doesn't create a cycle because ScopedWindowFocusAllowedIndicator
+  // is used only on a machine stack.

[haraken, 2016/12/07 07:49:17]

But we cannot use STACK_ALLOCATED because the object is exposed to content/ via
WebPrivatePtr. (The content/ uses this object on stack in essence.)

[sof, 2016/12/07 08:02:18]

Having a Persistent<ExecutionContext> is scary (wrt leaking), but no different
to what we had previously.

+  Persistent<ExecutionContext> m_executionContext;
 };
 
 }  // namespace blink
 
 #endif  // ScopedWindowFocusAllowedIndicator_h