Move Frame to the Oilpan heap.

Source/web/WebLocalFrameImpl.cpp

 /*
  * Copyright (C) 2009 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 480 matching lines @@
 WebRemoteFrame* WebLocalFrameImpl::toWebRemoteFrame()
 {
     ASSERT_NOT_REACHED();
     return 0;
 }
 
 void WebLocalFrameImpl::close()
 {
     m_client = 0;
 
+    if (m_frame)
+        m_frame->setHasBeenClosed();
+
+#if ENABLE(OILPAN)
+    if (m_frame && m_frame->isMainFrame() && viewImpl())

[dcheng, 2014/09/17 10:58:08]

I'm a little concerned about how it's much harder to reason about memory
correctness in the Oilpan version of this code.

Before, the ownership model was very simple--Source/web internally holds a ref
on behalf of the embedder, which it drops once close() is called. The embedder
knows that as long as it doesn't call close(), the WebFrame remains live, no
matter what it does.

With Oilpan, the only reference the Blink API holds on behalf of the embedder is
for the main frame. As part of frame detach, the embedder (RenderFrameImpl)
removes child WebFrames from the frame tree. Today, the code for
RenderFrameImpl::frameDetached(WebFrame* frame) looks like this:

1798  if (is_subframe)
1799    frame->parent()->removeChild(frame);
1800
1801  // |frame| is invalid after here.
1802  frame->close();

Imagine that |frame| wasn't passed as a parameter to this method, and it was
using |frame_| instead (something that's likely to happen in the future). After
the call to removeChild(), I don't think there would actually be any references
to keep this child WebFrame alive. While this isn't technically an issue
(because I don't think a GC can run between line 1799 and 1802), it seems
conceptually wrong.

+        viewImpl()->clearMainFrame(this);
+#else
     deref(); // Balances ref() acquired in WebFrame::create
+#endif
 }
 
 WebString WebLocalFrameImpl::uniqueName() const
 {
     return frame()->tree().uniqueName();
 }
 
 WebString WebLocalFrameImpl::assignedName() const
 {
     return frame()->tree().name();
@@ skipping 1008 matching lines @@
 
 // WebLocalFrameImpl public ---------------------------------------------------------
 
 WebLocalFrame* WebLocalFrame::create(WebFrameClient* client)
 {
     return WebLocalFrameImpl::create(client);
 }
 
 WebLocalFrameImpl* WebLocalFrameImpl::create(WebFrameClient* client)
 {
-    return adoptRef(new WebLocalFrameImpl(client)).leakRef();
+    WebLocalFrameImpl* frame = new WebLocalFrameImpl(client);
+#if ENABLE(OILPAN)
+    return frame;
+#else
+    return adoptRef(frame).leakRef();
+#endif
 }
 
 WebLocalFrameImpl::WebLocalFrameImpl(WebFrameClient* client)
     : m_frameLoaderClientImpl(this)
     , m_client(client)
     , m_permissionClient(0)
     , m_inputEventsScaleFactorForEmulation(1)
     , m_userMediaClientImpl(this)
     , m_geolocationClientProxy(GeolocationClientProxy::create(client ? client->geolocationClient() : 0))
 {
     Platform::current()->incrementStatsCounter(webFrameActiveCount);
     frameCount++;
 }
 
 WebLocalFrameImpl::~WebLocalFrameImpl()
 {
     Platform::current()->decrementStatsCounter(webFrameActiveCount);
     frameCount--;
 
     cancelPendingScopingEffort();
 }
 
-void WebLocalFrameImpl::setCoreFrame(PassRefPtr<LocalFrame> frame)
+void WebLocalFrameImpl::trace(Visitor* visitor)
+{
+#if ENABLE(OILPAN)
+    visitor->trace(m_frame);
+    visitor->trace(m_printContext);
+    visitor->trace(m_geolocationClientProxy);
+
+    WebFrame::traceChildren(visitor, this);
+#endif
+}
+
+void WebLocalFrameImpl::setCoreFrame(PassRefPtrWillBeRawPtr<LocalFrame> frame)
 {
     m_frame = frame;
 
     // FIXME: we shouldn't add overhead to every frame by registering these objects when they're not used.
     if (m_frame) {
         OwnPtr<NotificationPresenterImpl> notificationPresenter = adoptPtr(new NotificationPresenterImpl());
         if (m_client)
             notificationPresenter->initialize(m_client->notificationPresenter());
 
         provideNotification(*m_frame, notificationPresenter.release());
         provideNotificationPermissionClientTo(*m_frame, NotificationPermissionClientImpl::create());
         provideUserMediaTo(*m_frame, &m_userMediaClientImpl);
         provideGeolocationTo(*m_frame, m_geolocationClientProxy.get());
         m_geolocationClientProxy->setController(GeolocationController::from(m_frame.get()));
         provideMIDITo(*m_frame, MIDIClientProxy::create(m_client ? m_client->webMIDIClient() : 0));
         provideLocalFileSystemTo(*m_frame, LocalFileSystemClient::create());
 
         if (RuntimeEnabledFeatures::screenOrientationEnabled())
             ScreenOrientationController::provideTo(*m_frame, m_client ? m_client->webScreenOrientationClient() : 0);
     }
 }
 
-PassRefPtr<LocalFrame> WebLocalFrameImpl::initializeCoreFrame(FrameHost* host, FrameOwner* owner, const AtomicString& name, const AtomicString& fallbackName)
+PassRefPtrWillBeRawPtr<LocalFrame> WebLocalFrameImpl::initializeCoreFrame(FrameHost* host, FrameOwner* owner, const AtomicString& name, const AtomicString& fallbackName)
 {
-    RefPtr<LocalFrame> frame = LocalFrame::create(&m_frameLoaderClientImpl, host, owner);
+    RefPtrWillBeRawPtr<LocalFrame> frame = LocalFrame::create(&m_frameLoaderClientImpl, host, owner);
     setCoreFrame(frame);
     frame->tree().setName(name, fallbackName);
     // We must call init() after m_frame is assigned because it is referenced
     // during init(). Note that this may dispatch JS events; the frame may be
     // detached after init() returns.
     frame->init();
     return frame;
 }
 
-PassRefPtr<LocalFrame> WebLocalFrameImpl::createChildFrame(const FrameLoadRequest& request, HTMLFrameOwnerElement* ownerElement)
+PassRefPtrWillBeRawPtr<LocalFrame> WebLocalFrameImpl::createChildFrame(const FrameLoadRequest& request, HTMLFrameOwnerElement* ownerElement)
 {
     ASSERT(m_client);
     WebLocalFrameImpl* webframeChild = toWebLocalFrameImpl(m_client->createChildFrame(this, request.frameName()));
     if (!webframeChild)
         return nullptr;
 
     // FIXME: Using subResourceAttributeName as fallback is not a perfect
     // solution. subResourceAttributeName returns just one attribute name. The
     // element might not have the attribute, and there might be other attributes
     // which can identify the element.
-    RefPtr<LocalFrame> child = webframeChild->initializeCoreFrame(frame()->host(), ownerElement, request.frameName(), ownerElement->getAttribute(ownerElement->subResourceAttributeName()));
+    RefPtrWillBeRawPtr<LocalFrame> child = webframeChild->initializeCoreFrame(frame()->host(), ownerElement, request.frameName(), ownerElement->getAttribute(ownerElement->subResourceAttributeName()));
     // Initializing the core frame may cause the new child to be detached, since
     // it may dispatch a load event in the parent.
     if (!child->tree().parent())
         return nullptr;
 
     // If we're moving in the back/forward list, we might want to replace the content
     // of this child frame with whatever was there at that point.
     RefPtr<HistoryItem> childItem;
     if (isBackForwardLoadType(frame()->loader().loadType()) && !frame()->document()->loadEventFinished())
         childItem = PassRefPtr<HistoryItem>(webframeChild->client()->historyItemForNewChildFrame(webframeChild));
@@ skipping 267 matching lines @@
 
 void WebLocalFrameImpl::invalidateAll() const
 {
     ASSERT(frame() && frame()->view());
     FrameView* view = frame()->view();
     view->invalidateRect(view->frameRect());
     invalidateScrollbar();
 }
 
 } // namespace blink