Duplicate WindowProxy::disposeContext() into subclasses.

third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp

Index: third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp b/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp
index 831e87122f5d88271c64e2239042169cf5987241..efad7d6a828fab92d52b9714bba693e4a4a80078 100644
--- a/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp
@@ -37,6 +37,7 @@
 #include "bindings/core/v8/V8Binding.h"
 #include "bindings/core/v8/V8DOMActivityLogger.h"
 #include "bindings/core/v8/V8DOMWrapper.h"
+#include "bindings/core/v8/V8GCForContextDispose.h"
 #include "bindings/core/v8/V8HTMLDocument.h"
 #include "bindings/core/v8/V8HiddenValue.h"
 #include "bindings/core/v8/V8Initializer.h"
@@ -77,7 +78,32 @@ void LocalWindowProxy::disposeContext(GlobalDetachmentBehavior behavior) {
                                                        m_world->worldId());
   MainThreadDebugger::instance()->contextWillBeDestroyed(m_scriptState.get());
 
-  WindowProxy::disposeContext(behavior);
+  if (behavior == DetachGlobal) {
+    v8::Local<v8::Context> context = m_scriptState->context();
+    // Clean up state on the global proxy, which will be reused.
+    if (!m_globalProxy.isEmpty()) {
+      // TODO(yukishiino): This DCHECK failed on Canary (M57) and Dev (M56).
+      // We need to figure out why m_globalProxy != context->Global().
+      DCHECK(m_globalProxy == context->Global());
+      DCHECK_EQ(toScriptWrappable(context->Global()),
+                toScriptWrappable(
+                    context->Global()->GetPrototype().As<v8::Object>()));
+      m_globalProxy.get().SetWrapperClassId(0);
+    }
+    V8DOMWrapper::clearNativeInfo(isolate(), context->Global());

[haraken, 2017/03/08 06:21:37]

Is your crash related to the fact that we're explicitly clearing an internal
field here? I think this is the only place we clear an internal field.

[dcheng, 2017/03/08 06:46:40]

It's unclear. In theory, we shouldn't clear the internal fields without clearing
the wrapper class ID as well...

However, I think the DCHECK on line 87 has a hint to the problem. It seems like
it's possible to trigger situations where m_globalProxy != context->Global(). I
guess my WindowProxy CL must have made that more common.

Looking at the crash stack in
https://bugs.chromium.org/p/chromium/issues/detail?id=698739,
initializeIfNeeded() is crashing in the already-initialized branch. However, at
this point, we are in WebFrame::swap(): the frame we're swapping in should be
new, and should never have been initialized yet.

If it is initialized, it is a problem because:
- We call setGlobal() to transfer the global proxy during frame swap
- setGlobal() unconditionally overwrites m_globalProxy
- But context->Global() is already initialized and points to another global
proxy
- In teardown, we clear the wrapper class ID on m_globalProxy, but clear the
internal fields on context->Global().

Now, context->Global() has null internal fields but still has a wrapper class
ID.

+mlippautz, who mentioned that we only process non-null things from the marking
deque:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/...

But I'm not sure if that check would help here.

Regardless, I'm planning on investigating this by changing line 87/88 to CHECKs.
In WindowProxy::initializeIfNeeded(), I will capture a stack trace. This should
help us understand how we get into this situation.

+    m_scriptState->detachGlobalObject();
+  }
+
+  m_scriptState->disposePerContextData();
+
+  // It's likely that disposing the context has created a lot of
+  // garbage. Notify V8 about this so it'll have a chance of cleaning
+  // it up when idle.
+  V8GCForContextDispose::instance().notifyContextDisposed(
+      frame()->isMainFrame());
+
+  DCHECK(m_lifecycle == Lifecycle::ContextInitialized);
+  m_lifecycle = Lifecycle::ContextDetached;
 }
 
 void LocalWindowProxy::initialize() {