https://bugs.webkit.org/show_bug.cgi?id=67828
Reviewed by Adam Barth.
Source/WebCore:
* bindings/v8/V8DOMWindowShell.cpp:
(WebCore::V8DOMWindowShell::disposeContextHandles):
(WebCore::V8DOMWindowShell::initContextIfNeeded):
* bindings/v8/V8IsolatedContext.cpp:
(WebCore::V8IsolatedContext::V8IsolatedContext):
(WebCore::V8IsolatedContext::destroy):
* bindings/v8/V8IsolatedContext.h:
* loader/EmptyClients.h:
(WebCore::EmptyFrameLoaderClient::didCreateScriptContext):
(WebCore::EmptyFrameLoaderClient::willReleaseScriptContext):
* loader/FrameLoaderClient.h:
Source/WebKit/chromium:
* public/WebFrameClient.h:
(WebKit::WebFrameClient::didCreateScriptContext):
(WebKit::WebFrameClient::didDestroyScriptContext):
(WebKit::WebFrameClient::willReleaseScriptContext):
* src/FrameLoaderClientImpl.cpp:
(WebKit::FrameLoaderClientImpl::didCreateScriptContext):
(WebKit::FrameLoaderClientImpl::willReleaseScriptContext):
* src/FrameLoaderClientImpl.h:
* tests/WebFrameTest.cpp:
(WebKit::TEST_F):
(WebKit::ContextLifetimeTestWebFrameClient::Notification::Notification):
(WebKit::ContextLifetimeTestWebFrameClient::Notification::~Notification):
(WebKit::ContextLifetimeTestWebFrameClient::Notification::Equals):
(WebKit::ContextLifetimeTestWebFrameClient::~ContextLifetimeTestWebFrameClient):
(WebKit::ContextLifetimeTestWebFrameClient::reset):
(WebKit::ContextLifetimeTestWebFrameClient::didCreateScriptContext):
(WebKit::ContextLifetimeTestWebFrameClient::willReleaseScriptContext):
* tests/data/context_notifications_test.html: Added.
* tests/data/context_notifications_test_frame.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@95560
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2011-09-20 Aaron Boodman <aa@chromium.org>
+
+ Rework script context creation/release notifications
+ https://bugs.webkit.org/show_bug.cgi?id=67828
+
+ Reviewed by Adam Barth.
+
+ * bindings/v8/V8DOMWindowShell.cpp:
+ (WebCore::V8DOMWindowShell::disposeContextHandles):
+ (WebCore::V8DOMWindowShell::initContextIfNeeded):
+ * bindings/v8/V8IsolatedContext.cpp:
+ (WebCore::V8IsolatedContext::V8IsolatedContext):
+ (WebCore::V8IsolatedContext::destroy):
+ * bindings/v8/V8IsolatedContext.h:
+ * loader/EmptyClients.h:
+ (WebCore::EmptyFrameLoaderClient::didCreateScriptContext):
+ (WebCore::EmptyFrameLoaderClient::willReleaseScriptContext):
+ * loader/FrameLoaderClient.h:
+
2011-09-19 Oliver Hunt <oliver@apple.com>
Refactor Heap allocation logic into separate AllocationSpace class
void V8DOMWindowShell::disposeContextHandles()
{
if (!m_context.IsEmpty()) {
- m_frame->loader()->client()->didDestroyScriptContextForFrame();
+ m_frame->loader()->client()->willReleaseScriptContext(m_context, 0);
m_context.Dispose();
m_context.Clear();
setSecurityToken();
- m_frame->loader()->client()->didCreateScriptContextForFrame();
+ m_frame->loader()->client()->didCreateScriptContext(m_context, 0);
// FIXME: This is wrong. We should actually do this for the proper world once
// we do isolated worlds the WebCore way.
}
V8IsolatedContext::V8IsolatedContext(V8Proxy* proxy, int extensionGroup, int worldId)
- : m_world(IsolatedWorld::create(worldId))
+ : m_world(IsolatedWorld::create(worldId)),
+ m_frame(proxy->frame())
{
v8::HandleScope scope;
// FIXME: We should be creating a new V8DOMWindowShell here instead of riping out the context.
V8DOMWindowShell::installHiddenObjectPrototype(m_context->get());
// FIXME: This will go away once we have a windowShell for the isolated world.
- proxy->windowShell()->installDOMWindow(m_context->get(), proxy->frame()->domWindow());
+ proxy->windowShell()->installDOMWindow(m_context->get(), m_frame->domWindow());
// Using the default security token means that the canAccess is always
// called, which is slow.
// changes.
m_context->get()->UseDefaultSecurityToken();
- proxy->frame()->loader()->client()->didCreateIsolatedScriptContext(this);
+ m_frame->loader()->client()->didCreateScriptContext(context(), m_world->id());
}
void V8IsolatedContext::destroy()
{
+ m_frame->loader()->client()->willReleaseScriptContext(context(), m_world->id());
m_context->get().MakeWeak(this, &contextWeakReferenceCallback);
+ m_frame = 0;
}
V8IsolatedContext::~V8IsolatedContext()
RefPtr<IsolatedWorld> m_world;
RefPtr<SecurityOrigin> m_securityOrigin;
+
+ Frame* m_frame;
};
} // namespace WebCore
#include "SearchPopupMenu.h"
#if USE(V8)
-#include "V8IsolatedContext.h"
+#include <v8.h>
#endif
/*
virtual void registerForIconNotification(bool) { }
#if USE(V8)
- virtual void didCreateScriptContextForFrame() { }
- virtual void didDestroyScriptContextForFrame() { }
- virtual void didCreateIsolatedScriptContext(V8IsolatedContext*) { }
+ virtual void didCreateScriptContext(v8::Handle<v8::Context>, int worldId) { }
+ virtual void willReleaseScriptContext(v8::Handle<v8::Context>, int worldId) { }
virtual bool allowScriptExtension(const String& extensionName, int extensionGroup) { return false; }
#endif
class NSView;
#endif
+#if USE(V8)
+namespace v8 {
+class Context;
+template<class T> class Handle;
+}
+#endif
+
namespace WebCore {
class AuthenticationChallenge;
class SharedBuffer;
class StringWithDirection;
class SubstituteData;
-#if USE(V8)
- class V8IsolatedContext;
-#endif
class Widget;
typedef void (PolicyChecker::*FramePolicyFunction)(PolicyAction);
virtual void didPerformFirstNavigation() const = 0; // "Navigation" here means a transition from one page to another that ends up in the back/forward list.
#if USE(V8)
- virtual void didCreateScriptContextForFrame() = 0;
- virtual void didDestroyScriptContextForFrame() = 0;
- virtual void didCreateIsolatedScriptContext(V8IsolatedContext*) = 0;
+ virtual void didCreateScriptContext(v8::Handle<v8::Context>, int worldId) = 0;
+ virtual void willReleaseScriptContext(v8::Handle<v8::Context>, int worldId) = 0;
virtual bool allowScriptExtension(const String& extensionName, int extensionGroup) = 0;
#endif
+2011-09-20 Aaron Boodman <aa@chromium.org>
+
+ Rework script context creation/release notifications
+ https://bugs.webkit.org/show_bug.cgi?id=67828
+
+ Reviewed by Adam Barth.
+
+ * public/WebFrameClient.h:
+ (WebKit::WebFrameClient::didCreateScriptContext):
+ (WebKit::WebFrameClient::didDestroyScriptContext):
+ (WebKit::WebFrameClient::willReleaseScriptContext):
+ * src/FrameLoaderClientImpl.cpp:
+ (WebKit::FrameLoaderClientImpl::didCreateScriptContext):
+ (WebKit::FrameLoaderClientImpl::willReleaseScriptContext):
+ * src/FrameLoaderClientImpl.h:
+ * tests/WebFrameTest.cpp:
+ (WebKit::TEST_F):
+ (WebKit::ContextLifetimeTestWebFrameClient::Notification::Notification):
+ (WebKit::ContextLifetimeTestWebFrameClient::Notification::~Notification):
+ (WebKit::ContextLifetimeTestWebFrameClient::Notification::Equals):
+ (WebKit::ContextLifetimeTestWebFrameClient::~ContextLifetimeTestWebFrameClient):
+ (WebKit::ContextLifetimeTestWebFrameClient::reset):
+ (WebKit::ContextLifetimeTestWebFrameClient::didCreateScriptContext):
+ (WebKit::ContextLifetimeTestWebFrameClient::willReleaseScriptContext):
+ * tests/data/context_notifications_test.html: Added.
+ * tests/data/context_notifications_test_frame.html: Added.
+
2011-09-19 Geoffrey Garen <ggaren@apple.com>
Removed ENABLE_JSC_MULTIPLE_THREADS and related #ifdefs.
// Notifies that a new script context has been created for this frame.
// This is similar to didClearWindowObject but only called once per
// frame context.
+ // FIXME: Remove this when Chromium is updated to use the below version.
virtual void didCreateScriptContext(WebFrame*) { }
-
- // Notifies that this frame's script context has been destroyed.
- virtual void didDestroyScriptContext(WebFrame*) { }
+#if WEBKIT_USING_V8
+ virtual void didCreateScriptContext(WebFrame*, v8::Handle<v8::Context>, int worldId) { }
+#endif
// Notifies that a garbage-collected context was created - content
// scripts.
+ // FIXME: Remove this when Chromium is updated to use didCreateScriptContext().
#if WEBKIT_USING_V8
virtual void didCreateIsolatedScriptContext(WebFrame*, int worldID, v8::Handle<v8::Context>) { }
#endif
+ // Notifies that this frame's script context has been destroyed.
+ // FIXME: Remove this when Chromium is updated to use the below version.
+ virtual void didDestroyScriptContext(WebFrame*) { }
+
+#if WEBKIT_USING_V8
+ // WebKit is about to release its reference to a v8 context for a frame.
+ virtual void willReleaseScriptContext(WebFrame*, v8::Handle<v8::Context>, int worldId) { }
+#endif
+
// Geometry notifications ----------------------------------------------
// The frame's document finished the initial layout of a page.
#include <wtf/text/CString.h>
#if USE(V8)
-#include "V8IsolatedContext.h"
+#include <v8.h>
#endif
using namespace WebCore;
m_webFrame->client()->didCreateDocumentElement(m_webFrame);
}
-void FrameLoaderClientImpl::didCreateScriptContextForFrame()
+#if USE(V8)
+void FrameLoaderClientImpl::didCreateScriptContext(v8::Handle<v8::Context> context, int worldId)
{
- if (m_webFrame->client())
- m_webFrame->client()->didCreateScriptContext(m_webFrame);
-}
+ if (m_webFrame->client()) {
+ // FIXME: Remove these once Chromium is updated to use the new version of didCreateScriptContext().
+ if (worldId)
+ m_webFrame->client()->didCreateIsolatedScriptContext(m_webFrame, worldId, context);
+ else
+ m_webFrame->client()->didCreateScriptContext(m_webFrame);
-void FrameLoaderClientImpl::didDestroyScriptContextForFrame()
-{
- if (m_webFrame->client())
- m_webFrame->client()->didDestroyScriptContext(m_webFrame);
+ m_webFrame->client()->didCreateScriptContext(m_webFrame, context, worldId);
+ }
}
-#if USE(V8)
-void FrameLoaderClientImpl::didCreateIsolatedScriptContext(V8IsolatedContext* isolatedContext)
+void FrameLoaderClientImpl::willReleaseScriptContext(v8::Handle<v8::Context> context, int worldId)
{
- if (m_webFrame->client())
- m_webFrame->client()->didCreateIsolatedScriptContext(m_webFrame, isolatedContext->world()->id(), isolatedContext->context());
+ if (m_webFrame->client()) {
+ // FIXME: Remove this once Chromium is updated to use willReleaseScriptContext().
+ if (!worldId)
+ m_webFrame->client()->didDestroyScriptContext(m_webFrame);
+
+ m_webFrame->client()->willReleaseScriptContext(m_webFrame, context, worldId);
+ }
}
#endif
virtual void dispatchDidClearWindowObjectInWorld(WebCore::DOMWrapperWorld*);
virtual void documentElementAvailable();
- // A frame's V8 context was created or destroyed.
- virtual void didCreateScriptContextForFrame();
- virtual void didDestroyScriptContextForFrame();
-
#if USE(V8)
- // A context untied to a frame was created (through evaluateInIsolatedWorld).
- // This context is not tied to the lifetime of its frame, and is destroyed
- // in garbage collection.
- virtual void didCreateIsolatedScriptContext(WebCore::V8IsolatedContext*);
+ virtual void didCreateScriptContext(v8::Handle<v8::Context>, int worldId);
+ virtual void willReleaseScriptContext(v8::Handle<v8::Context>, int worldId);
#endif
// Returns true if we should allow the given V8 extension to be added to
#include "WebFormElement.h"
#include "WebFrame.h"
#include "WebFrameClient.h"
+#include "WebScriptSource.h"
#include "WebSearchableFormData.h"
#include "WebSecurityPolicy.h"
#include "WebSettings.h"
registerMockedHttpURLLoad("zero_sized_iframe.html");
// Create and initialize the WebView.
- TestWebFrameClient webFrameClient;
+ TestWebFrameClient webFrameClient;
WebView* webView = WebView::create(0);
webView->settings()->setJavaScriptEnabled(true);
webView->initializeMainFrame(&webFrameClient);
// Now retrieve the FocusedNode and test it should be null.
EXPECT_EQ(0, webViewImpl->focusedWebCoreNode());
- webViewImpl->close();
+ webViewImpl->close();
+}
+
+// Implementation of WebFrameClient that tracks the v8 contexts that are created
+// and destroyed for verification.
+class ContextLifetimeTestWebFrameClient : public WebFrameClient {
+public:
+ struct Notification {
+ public:
+ Notification(WebFrame* frame, v8::Handle<v8::Context> context, int worldId)
+ : frame(frame) ,
+ context(v8::Persistent<v8::Context>::New(context)),
+ worldId(worldId)
+ {
+ }
+
+ ~Notification()
+ {
+ context.Dispose();
+ }
+
+ bool Equals(Notification* other)
+ {
+ return other && frame == other->frame && context == other->context && worldId == other->worldId;
+ }
+
+ WebFrame* frame;
+ v8::Persistent<v8::Context> context;
+ int worldId;
+ };
+
+ ~ContextLifetimeTestWebFrameClient()
+ {
+ reset();
+ }
+
+ void reset()
+ {
+ for (size_t i = 0; i < createNotifications.size(); ++i)
+ delete createNotifications[i];
+
+ for (size_t i = 0; i < releaseNotifications.size(); ++i)
+ delete releaseNotifications[i];
+
+ createNotifications.clear();
+ releaseNotifications.clear();
+ }
+
+ std::vector<Notification*> createNotifications;
+ std::vector<Notification*> releaseNotifications;
+
+ private:
+ virtual void didCreateScriptContext(WebFrame* frame, v8::Handle<v8::Context> context, int worldId)
+ {
+ createNotifications.push_back(new Notification(frame, context, worldId));
+ }
+
+ virtual void willReleaseScriptContext(WebFrame* frame, v8::Handle<v8::Context> context, int worldId)
+ {
+ releaseNotifications.push_back(new Notification(frame, context, worldId));
+ }
+};
+
+TEST_F(WebFrameTest, ContextNotificationsLoadUnload)
+{
+ v8::HandleScope handleScope;
+
+ registerMockedHttpURLLoad("context_notifications_test.html");
+ registerMockedHttpURLLoad("context_notifications_test_frame.html");
+
+ // Load a frame with an iframe, make sure we get the right create notifications.
+ ContextLifetimeTestWebFrameClient webFrameClient;
+ WebView* webView = WebView::create(0);
+ webView->settings()->setJavaScriptEnabled(true);
+ webView->initializeMainFrame(&webFrameClient);
+ loadHttpFrame(webView->mainFrame(), "context_notifications_test.html");
+ serveRequests();
+
+ WebFrame* mainFrame = webView->mainFrame();
+ WebFrame* childFrame = mainFrame->firstChild();
+
+ ASSERT_EQ(2u, webFrameClient.createNotifications.size());
+ EXPECT_EQ(0u, webFrameClient.releaseNotifications.size());
+
+ ContextLifetimeTestWebFrameClient::Notification* firstCreateNotification = webFrameClient.createNotifications[0];
+ ContextLifetimeTestWebFrameClient::Notification* secondCreateNotification = webFrameClient.createNotifications[1];
+
+ EXPECT_EQ(mainFrame, firstCreateNotification->frame);
+ EXPECT_EQ(mainFrame->mainWorldScriptContext(), firstCreateNotification->context);
+ EXPECT_EQ(0, firstCreateNotification->worldId);
+
+ EXPECT_EQ(childFrame, secondCreateNotification->frame);
+ EXPECT_EQ(childFrame->mainWorldScriptContext(), secondCreateNotification->context);
+ EXPECT_EQ(0, secondCreateNotification->worldId);
+
+ // Close the view. We should get two release notifications that are exactly the same as the create ones, in reverse order.
+ webView->close();
+
+ ASSERT_EQ(2u, webFrameClient.releaseNotifications.size());
+ ContextLifetimeTestWebFrameClient::Notification* firstReleaseNotification = webFrameClient.releaseNotifications[0];
+ ContextLifetimeTestWebFrameClient::Notification* secondReleaseNotification = webFrameClient.releaseNotifications[1];
+
+ ASSERT_TRUE(firstCreateNotification->Equals(secondReleaseNotification));
+ ASSERT_TRUE(secondCreateNotification->Equals(firstReleaseNotification));
+}
+
+TEST_F(WebFrameTest, ContextNotificationsReload)
+{
+ v8::HandleScope handleScope;
+
+ registerMockedHttpURLLoad("context_notifications_test.html");
+ registerMockedHttpURLLoad("context_notifications_test_frame.html");
+
+ ContextLifetimeTestWebFrameClient webFrameClient;
+ WebView* webView = WebView::create(0);
+ webView->settings()->setJavaScriptEnabled(true);
+ webView->initializeMainFrame(&webFrameClient);
+ loadHttpFrame(webView->mainFrame(), "context_notifications_test.html");
+ serveRequests();
+
+ // Refresh, we should get two release notifications and two more create notifications.
+ webView->mainFrame()->reload(false);
+ serveRequests();
+ ASSERT_EQ(4u, webFrameClient.createNotifications.size());
+ ASSERT_EQ(2u, webFrameClient.releaseNotifications.size());
+
+ // The two release notifications we got should be exactly the same as the first two create notifications.
+ for (size_t i = 0; i < webFrameClient.releaseNotifications.size(); ++i) {
+ EXPECT_TRUE(webFrameClient.releaseNotifications[i]->Equals(
+ webFrameClient.createNotifications[webFrameClient.createNotifications.size() - 3 - i]));
+ }
+
+ // The last two create notifications should be for the current frames and context.
+ WebFrame* mainFrame = webView->mainFrame();
+ WebFrame* childFrame = mainFrame->firstChild();
+ ContextLifetimeTestWebFrameClient::Notification* firstRefreshNotification = webFrameClient.createNotifications[2];
+ ContextLifetimeTestWebFrameClient::Notification* secondRefreshNotification = webFrameClient.createNotifications[3];
+
+ EXPECT_EQ(mainFrame, firstRefreshNotification->frame);
+ EXPECT_EQ(mainFrame->mainWorldScriptContext(), firstRefreshNotification->context);
+ EXPECT_EQ(0, firstRefreshNotification->worldId);
+
+ EXPECT_EQ(childFrame, secondRefreshNotification->frame);
+ EXPECT_EQ(childFrame->mainWorldScriptContext(), secondRefreshNotification->context);
+ EXPECT_EQ(0, secondRefreshNotification->worldId);
+
+ webView->close();
+}
+
+TEST_F(WebFrameTest, ContextNotificationsIsolatedWorlds)
+{
+ v8::HandleScope handleScope;
+
+ registerMockedHttpURLLoad("context_notifications_test.html");
+ registerMockedHttpURLLoad("context_notifications_test_frame.html");
+
+ ContextLifetimeTestWebFrameClient webFrameClient;
+ WebView* webView = WebView::create(0);
+ webView->settings()->setJavaScriptEnabled(true);
+ webView->initializeMainFrame(&webFrameClient);
+ loadHttpFrame(webView->mainFrame(), "context_notifications_test.html");
+ serveRequests();
+
+ // Add an isolated world.
+ webFrameClient.reset();
+
+ int isolatedWorldId = 42;
+ WebScriptSource scriptSource("hi!");
+ int numSources = 1;
+ int extensionGroup = 0;
+ webView->mainFrame()->executeScriptInIsolatedWorld(isolatedWorldId, &scriptSource, numSources, extensionGroup);
+
+ // We should now have a new create notification.
+ ASSERT_EQ(1u, webFrameClient.createNotifications.size());
+ ContextLifetimeTestWebFrameClient::Notification* notification = webFrameClient.createNotifications[0];
+ ASSERT_EQ(isolatedWorldId, notification->worldId);
+ ASSERT_EQ(webView->mainFrame(), notification->frame);
+
+ // We don't have an API to enumarate isolated worlds for a frame, but we can at least assert that the context we got is *not* the main world's context.
+ ASSERT_NE(webView->mainFrame()->mainWorldScriptContext(), notification->context);
+
+ webView->close();
+
+ // We should have gotten three release notifications (one for each of the frames, plus one for the isolated context).
+ ASSERT_EQ(3u, webFrameClient.releaseNotifications.size());
+
+ // And one of them should be exactly the same as the create notification for the isolated context.
+ int matchCount = 0;
+ for (size_t i = 0; i < webFrameClient.releaseNotifications.size(); ++i) {
+ if (webFrameClient.releaseNotifications[i]->Equals(webFrameClient.createNotifications[0]))
+ ++matchCount;
+ }
+ EXPECT_EQ(1, matchCount);
}
} // namespace
--- /dev/null
+<script>console.log('A script is needed to create a script context.');</script>
+<iframe src="context_notifications_test_frame.html"></iframe>
--- /dev/null
+<script>console.log('A script is needed to create a script context.');</script>