Diff
Modified: trunk/Source/WebCore/ChangeLog (95384 => 95385)
--- trunk/Source/WebCore/ChangeLog 2011-09-17 20:10:01 UTC (rev 95384)
+++ trunk/Source/WebCore/ChangeLog 2011-09-17 22:47:09 UTC (rev 95385)
@@ -1,3 +1,22 @@
+2011-09-17 Aaron Boodman <[email protected]>
+
+ 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-17 Ilya Tikhonovsky <[email protected]>
Web Inspector: file open dialog appears when user clicks on the timeline bar in timeline panel.
Modified: trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp (95384 => 95385)
--- trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp 2011-09-17 20:10:01 UTC (rev 95384)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp 2011-09-17 22:47:09 UTC (rev 95385)
@@ -173,7 +173,7 @@
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();
@@ -339,7 +339,7 @@
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.
Modified: trunk/Source/WebCore/bindings/v8/V8IsolatedContext.cpp (95384 => 95385)
--- trunk/Source/WebCore/bindings/v8/V8IsolatedContext.cpp 2011-09-17 20:10:01 UTC (rev 95384)
+++ trunk/Source/WebCore/bindings/v8/V8IsolatedContext.cpp 2011-09-17 22:47:09 UTC (rev 95385)
@@ -49,7 +49,7 @@
}
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.
@@ -64,7 +64,7 @@
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.
@@ -74,12 +74,14 @@
// 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()
Modified: trunk/Source/WebCore/bindings/v8/V8IsolatedContext.h (95384 => 95385)
--- trunk/Source/WebCore/bindings/v8/V8IsolatedContext.h 2011-09-17 20:10:01 UTC (rev 95384)
+++ trunk/Source/WebCore/bindings/v8/V8IsolatedContext.h 2011-09-17 22:47:09 UTC (rev 95385)
@@ -114,6 +114,8 @@
RefPtr<IsolatedWorld> m_world;
RefPtr<SecurityOrigin> m_securityOrigin;
+
+ Frame* m_frame;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/loader/EmptyClients.h (95384 => 95385)
--- trunk/Source/WebCore/loader/EmptyClients.h 2011-09-17 20:10:01 UTC (rev 95384)
+++ trunk/Source/WebCore/loader/EmptyClients.h 2011-09-17 22:47:09 UTC (rev 95385)
@@ -47,7 +47,7 @@
#include "SearchPopupMenu.h"
#if USE(V8)
-#include "V8IsolatedContext.h"
+#include <v8.h>
#endif
/*
@@ -397,9 +397,8 @@
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
Modified: trunk/Source/WebCore/loader/FrameLoaderClient.h (95384 => 95385)
--- trunk/Source/WebCore/loader/FrameLoaderClient.h 2011-09-17 20:10:01 UTC (rev 95384)
+++ trunk/Source/WebCore/loader/FrameLoaderClient.h 2011-09-17 22:47:09 UTC (rev 95385)
@@ -51,6 +51,13 @@
class NSView;
#endif
+#if USE(V8)
+namespace v8 {
+class Context;
+template<class T> class Handle;
+}
+#endif
+
namespace WebCore {
class AuthenticationChallenge;
@@ -87,9 +94,6 @@
class SharedBuffer;
class StringWithDirection;
class SubstituteData;
-#if USE(V8)
- class V8IsolatedContext;
-#endif
class Widget;
typedef void (PolicyChecker::*FramePolicyFunction)(PolicyAction);
@@ -274,9 +278,8 @@
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
Modified: trunk/Source/WebKit/chromium/ChangeLog (95384 => 95385)
--- trunk/Source/WebKit/chromium/ChangeLog 2011-09-17 20:10:01 UTC (rev 95384)
+++ trunk/Source/WebKit/chromium/ChangeLog 2011-09-17 22:47:09 UTC (rev 95385)
@@ -1,3 +1,19 @@
+2011-09-17 Aaron Boodman <[email protected]>
+
+ 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:
+
2011-09-17 chandra shekar vallala <[email protected]>
[chromium]The focus of an input field inside an Iframe doesn't get cleared even though clearFocusedNode is called.
Modified: trunk/Source/WebKit/chromium/public/WebFrameClient.h (95384 => 95385)
--- trunk/Source/WebKit/chromium/public/WebFrameClient.h 2011-09-17 20:10:01 UTC (rev 95384)
+++ trunk/Source/WebKit/chromium/public/WebFrameClient.h 2011-09-17 22:47:09 UTC (rev 95385)
@@ -298,17 +298,28 @@
// 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*) { }
+#if WEBKIT_USING_V8
+ virtual void didCreateScriptContext(WebFrame*, v8::Handle<v8::Context>, int worldId) { }
+#endif
- // Notifies that this frame's script context has been destroyed.
- virtual void didDestroyScriptContext(WebFrame*) { }
-
// 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.
Modified: trunk/Source/WebKit/chromium/src/FrameLoaderClientImpl.cpp (95384 => 95385)
--- trunk/Source/WebKit/chromium/src/FrameLoaderClientImpl.cpp 2011-09-17 20:10:01 UTC (rev 95384)
+++ trunk/Source/WebKit/chromium/src/FrameLoaderClientImpl.cpp 2011-09-17 22:47:09 UTC (rev 95385)
@@ -85,7 +85,7 @@
#include <wtf/text/CString.h>
#if USE(V8)
-#include "V8IsolatedContext.h"
+#include <v8.h>
#endif
using namespace WebCore;
@@ -140,24 +140,18 @@
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);
+ m_webFrame->client()->didCreateScriptContext(m_webFrame, context, worldId);
}
-void FrameLoaderClientImpl::didDestroyScriptContextForFrame()
+void FrameLoaderClientImpl::willReleaseScriptContext(v8::Handle<v8::Context> context, int worldId)
{
if (m_webFrame->client())
- m_webFrame->client()->didDestroyScriptContext(m_webFrame);
+ m_webFrame->client()->willReleaseScriptContext(m_webFrame, context, worldId);
}
-
-#if USE(V8)
-void FrameLoaderClientImpl::didCreateIsolatedScriptContext(V8IsolatedContext* isolatedContext)
-{
- if (m_webFrame->client())
- m_webFrame->client()->didCreateIsolatedScriptContext(m_webFrame, isolatedContext->world()->id(), isolatedContext->context());
-}
#endif
bool FrameLoaderClientImpl::allowScriptExtension(const String& extensionName,
Modified: trunk/Source/WebKit/chromium/src/FrameLoaderClientImpl.h (95384 => 95385)
--- trunk/Source/WebKit/chromium/src/FrameLoaderClientImpl.h 2011-09-17 20:10:01 UTC (rev 95384)
+++ trunk/Source/WebKit/chromium/src/FrameLoaderClientImpl.h 2011-09-17 22:47:09 UTC (rev 95385)
@@ -61,15 +61,9 @@
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
Modified: trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp (95384 => 95385)
--- trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp 2011-09-17 20:10:01 UTC (rev 95384)
+++ trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp 2011-09-17 22:47:09 UTC (rev 95385)
@@ -35,6 +35,7 @@
#include "WebFormElement.h"
#include "WebFrame.h"
#include "WebFrameClient.h"
+#include "WebScriptSource.h"
#include "WebSearchableFormData.h"
#include "WebSecurityPolicy.h"
#include "WebSettings.h"
@@ -157,7 +158,7 @@
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);
@@ -280,7 +281,197 @@
// 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
Added: trunk/Source/WebKit/chromium/tests/data/context_notifications_test.html (0 => 95385)
--- trunk/Source/WebKit/chromium/tests/data/context_notifications_test.html (rev 0)
+++ trunk/Source/WebKit/chromium/tests/data/context_notifications_test.html 2011-09-17 22:47:09 UTC (rev 95385)
@@ -0,0 +1,2 @@
+<script>console.log('A script is needed to create a script context.');</script>
+<iframe src=""
Property changes on: trunk/Source/WebKit/chromium/tests/data/context_notifications_test.html
___________________________________________________________________
Added: svn:eol-style
Added: trunk/Source/WebKit/chromium/tests/data/context_notifications_test_frame.html (0 => 95385)
--- trunk/Source/WebKit/chromium/tests/data/context_notifications_test_frame.html (rev 0)
+++ trunk/Source/WebKit/chromium/tests/data/context_notifications_test_frame.html 2011-09-17 22:47:09 UTC (rev 95385)
@@ -0,0 +1 @@
+<script>console.log('A script is needed to create a script context.');</script>
Property changes on: trunk/Source/WebKit/chromium/tests/data/context_notifications_test_frame.html
___________________________________________________________________
Added: svn:eol-style
