Changeset 95560 in webkit


Ignore:
Timestamp:
Sep 20, 2011 11:58:04 AM (13 years ago)
Author:
aa@chromium.org
Message:

Rework script context creation/release notifications
​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.
Location:
trunk/Source
Files:
2 added
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r95559 r95560  
     12011-09-20  Aaron Boodman  <aa@chromium.org>
     2
     3        Rework script context creation/release notifications
     4        https://bugs.webkit.org/show_bug.cgi?id=67828
     5
     6        Reviewed by Adam Barth.
     7
     8        * bindings/v8/V8DOMWindowShell.cpp:
     9        (WebCore::V8DOMWindowShell::disposeContextHandles):
     10        (WebCore::V8DOMWindowShell::initContextIfNeeded):
     11        * bindings/v8/V8IsolatedContext.cpp:
     12        (WebCore::V8IsolatedContext::V8IsolatedContext):
     13        (WebCore::V8IsolatedContext::destroy):
     14        * bindings/v8/V8IsolatedContext.h:
     15        * loader/EmptyClients.h:
     16        (WebCore::EmptyFrameLoaderClient::didCreateScriptContext):
     17        (WebCore::EmptyFrameLoaderClient::willReleaseScriptContext):
     18        * loader/FrameLoaderClient.h:
     19
    1202011-09-19  Oliver Hunt  <oliver@apple.com>
    221

  • trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp

    r95483 r95560  
    174174{
    175175    if (!m_context.IsEmpty()) {
    176         m_frame->loader()->client()->didDestroyScriptContextForFrame();
     176        m_frame->loader()->client()->willReleaseScriptContext(m_context, 0);
    177177        m_context.Dispose();
    178178        m_context.Clear();
    … …  
    340340    setSecurityToken();
    341341
    342     m_frame->loader()->client()->didCreateScriptContextForFrame();
     342    m_frame->loader()->client()->didCreateScriptContext(m_context, 0);
    343343
    344344    // FIXME: This is wrong. We should actually do this for the proper world once

  • trunk/Source/WebCore/bindings/v8/V8IsolatedContext.cpp

    r95483 r95560  
    5050
    5151V8IsolatedContext::V8IsolatedContext(V8Proxy* proxy, int extensionGroup, int worldId)
    52     : m_world(IsolatedWorld::create(worldId))
     52    : m_world(IsolatedWorld::create(worldId)),
     53      m_frame(proxy->frame())
    5354{
    5455    v8::HandleScope scope;
    … …  
    6566    V8DOMWindowShell::installHiddenObjectPrototype(m_context->get());
    6667    // FIXME: This will go away once we have a windowShell for the isolated world.
    67     proxy->windowShell()->installDOMWindow(m_context->get(), proxy->frame()->domWindow());
     68    proxy->windowShell()->installDOMWindow(m_context->get(), m_frame->domWindow());
    6869
    6970    // Using the default security token means that the canAccess is always
    … …  
    7576    m_context->get()->UseDefaultSecurityToken();
    7677
    77     proxy->frame()->loader()->client()->didCreateIsolatedScriptContext(this);
     78    m_frame->loader()->client()->didCreateScriptContext(context(), m_world->id());
    7879}
    7980
    8081void V8IsolatedContext::destroy()
    8182{
     83    m_frame->loader()->client()->willReleaseScriptContext(context(), m_world->id());
    8284    m_context->get().MakeWeak(this, &contextWeakReferenceCallback);
     85    m_frame = 0;
    8386}
    8487

  • trunk/Source/WebCore/bindings/v8/V8IsolatedContext.h

    r95483 r95560  
    115115
    116116    RefPtr<SecurityOrigin> m_securityOrigin;
     117
     118    Frame* m_frame;
    117119};
    118120

  • trunk/Source/WebCore/loader/EmptyClients.h

    r95483 r95560  
    4848
    4949#if USE(V8)
    50 #include "V8IsolatedContext.h"
     50#include <v8.h>
    5151#endif
    5252
    … …  
    395395
    396396#if USE(V8)
    397     virtual void didCreateScriptContextForFrame() { }
    398     virtual void didDestroyScriptContextForFrame() { }
    399     virtual void didCreateIsolatedScriptContext(V8IsolatedContext*) { }
     397    virtual void didCreateScriptContext(v8::Handle<v8::Context>, int worldId) { }
     398    virtual void willReleaseScriptContext(v8::Handle<v8::Context>, int worldId) { }
    400399    virtual bool allowScriptExtension(const String& extensionName, int extensionGroup) { return false; }
    401400#endif

  • trunk/Source/WebCore/loader/FrameLoaderClient.h

    r95483 r95560  
    5151#endif
    5252
     53#if USE(V8)
     54namespace v8 {
     55class Context;
     56template<class T> class Handle;
     57}
     58#endif
     59
    5360namespace WebCore {
    5461
    … …  
    8794    class StringWithDirection;
    8895    class SubstituteData;
    89 #if USE(V8)
    90     class V8IsolatedContext;
    91 #endif
    9296    class Widget;
    9397
    … …  
    271275
    272276#if USE(V8)
    273         virtual void didCreateScriptContextForFrame() = 0;
    274         virtual void didDestroyScriptContextForFrame() = 0;
    275         virtual void didCreateIsolatedScriptContext(V8IsolatedContext*) = 0;
     277        virtual void didCreateScriptContext(v8::Handle<v8::Context>, int worldId) = 0;
     278        virtual void willReleaseScriptContext(v8::Handle<v8::Context>, int worldId) = 0;
    276279        virtual bool allowScriptExtension(const String& extensionName, int extensionGroup) = 0;
    277280#endif

  • trunk/Source/WebKit/chromium/ChangeLog

    r95514 r95560  
     12011-09-20  Aaron Boodman  <aa@chromium.org>
     2
     3        Rework script context creation/release notifications
     4        https://bugs.webkit.org/show_bug.cgi?id=67828
     5
     6        Reviewed by Adam Barth.
     7
     8        * public/WebFrameClient.h:
     9        (WebKit::WebFrameClient::didCreateScriptContext):
     10        (WebKit::WebFrameClient::didDestroyScriptContext):
     11        (WebKit::WebFrameClient::willReleaseScriptContext):
     12        * src/FrameLoaderClientImpl.cpp:
     13        (WebKit::FrameLoaderClientImpl::didCreateScriptContext):
     14        (WebKit::FrameLoaderClientImpl::willReleaseScriptContext):
     15        * src/FrameLoaderClientImpl.h:
     16        * tests/WebFrameTest.cpp:
     17        (WebKit::TEST_F):
     18        (WebKit::ContextLifetimeTestWebFrameClient::Notification::Notification):
     19        (WebKit::ContextLifetimeTestWebFrameClient::Notification::~Notification):
     20        (WebKit::ContextLifetimeTestWebFrameClient::Notification::Equals):
     21        (WebKit::ContextLifetimeTestWebFrameClient::~ContextLifetimeTestWebFrameClient):
     22        (WebKit::ContextLifetimeTestWebFrameClient::reset):
     23        (WebKit::ContextLifetimeTestWebFrameClient::didCreateScriptContext):
     24        (WebKit::ContextLifetimeTestWebFrameClient::willReleaseScriptContext):
     25        * tests/data/context_notifications_test.html: Added.
     26        * tests/data/context_notifications_test_frame.html: Added.
     27
    1282011-09-19  Geoffrey Garen  <ggaren@apple.com>
    229

  • trunk/Source/WebKit/chromium/public/WebFrameClient.h

    r95483 r95560  
    299299    // This is similar to didClearWindowObject but only called once per
    300300    // frame context.
     301    // FIXME: Remove this when Chromium is updated to use the below version.
    301302    virtual void didCreateScriptContext(WebFrame*) { }
    302 
    303     // Notifies that this frame's script context has been destroyed.
    304     virtual void didDestroyScriptContext(WebFrame*) { }
     303#if WEBKIT_USING_V8
     304    virtual void didCreateScriptContext(WebFrame*, v8::Handle<v8::Context>, int worldId) { }
     305#endif
    305306
    306307    // Notifies that a garbage-collected context was created - content
    307308    // scripts.
     309    // FIXME: Remove this when Chromium is updated to use didCreateScriptContext().
    308310#if WEBKIT_USING_V8
    309311    virtual void didCreateIsolatedScriptContext(WebFrame*, int worldID, v8::Handle<v8::Context>) { }
     312#endif
     313
     314    // Notifies that this frame's script context has been destroyed.
     315    // FIXME: Remove this when Chromium is updated to use the below version.
     316    virtual void didDestroyScriptContext(WebFrame*) { }
     317
     318#if WEBKIT_USING_V8
     319    // WebKit is about to release its reference to a v8 context for a frame.
     320    virtual void willReleaseScriptContext(WebFrame*, v8::Handle<v8::Context>, int worldId) { }
    310321#endif
    311322

  • trunk/Source/WebKit/chromium/src/FrameLoaderClientImpl.cpp

    r95483 r95560  
    8686
    8787#if USE(V8)
    88 #include "V8IsolatedContext.h"
     88#include <v8.h>
    8989#endif
    9090
    … …  
    141141}
    142142
    143 void FrameLoaderClientImpl::didCreateScriptContextForFrame()
    144 {
    145     if (m_webFrame->client())
    146         m_webFrame->client()->didCreateScriptContext(m_webFrame);
    147 }
    148 
    149 void FrameLoaderClientImpl::didDestroyScriptContextForFrame()
    150 {
    151     if (m_webFrame->client())
    152         m_webFrame->client()->didDestroyScriptContext(m_webFrame);
    153 }
    154 
    155143#if USE(V8)
    156 void FrameLoaderClientImpl::didCreateIsolatedScriptContext(V8IsolatedContext* isolatedContext)
    157 {
    158     if (m_webFrame->client())
    159         m_webFrame->client()->didCreateIsolatedScriptContext(m_webFrame, isolatedContext->world()->id(), isolatedContext->context());
     144void FrameLoaderClientImpl::didCreateScriptContext(v8::Handle<v8::Context> context, int worldId)
     145{
     146    if (m_webFrame->client()) {
     147        // FIXME: Remove these once Chromium is updated to use the new version of didCreateScriptContext().
     148        if (worldId)
     149            m_webFrame->client()->didCreateIsolatedScriptContext(m_webFrame, worldId, context);
     150        else
     151            m_webFrame->client()->didCreateScriptContext(m_webFrame);
     152
     153        m_webFrame->client()->didCreateScriptContext(m_webFrame, context, worldId);
     154    }
     155}
     156
     157void FrameLoaderClientImpl::willReleaseScriptContext(v8::Handle<v8::Context> context, int worldId)
     158{
     159    if (m_webFrame->client()) {
     160        // FIXME: Remove this once Chromium is updated to use willReleaseScriptContext().
     161        if (!worldId)
     162            m_webFrame->client()->didDestroyScriptContext(m_webFrame);
     163
     164        m_webFrame->client()->willReleaseScriptContext(m_webFrame, context, worldId);
     165    }
    160166}
    161167#endif

  • trunk/Source/WebKit/chromium/src/FrameLoaderClientImpl.h

    r95483 r95560  
    6262    virtual void documentElementAvailable();
    6363
    64     // A frame's V8 context was created or destroyed.
    65     virtual void didCreateScriptContextForFrame();
    66     virtual void didDestroyScriptContextForFrame();
    67 
    6864#if USE(V8)
    69     // A context untied to a frame was created (through evaluateInIsolatedWorld).
    70     // This context is not tied to the lifetime of its frame, and is destroyed
    71     // in garbage collection.
    72     virtual void didCreateIsolatedScriptContext(WebCore::V8IsolatedContext*);
     65    virtual void didCreateScriptContext(v8::Handle<v8::Context>, int worldId);
     66    virtual void willReleaseScriptContext(v8::Handle<v8::Context>, int worldId);
    7367#endif
    7468

  • trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp

    r95483 r95560  
    3636#include "WebFrame.h"
    3737#include "WebFrameClient.h"
     38#include "WebScriptSource.h"
    3839#include "WebSearchableFormData.h"
    3940#include "WebSecurityPolicy.h"
    … …  
    158159
    159160    // Create and initialize the WebView.
    160      TestWebFrameClient webFrameClient;
     161    TestWebFrameClient webFrameClient;
    161162    WebView* webView = WebView::create(0);
    162163    webView->settings()->setJavaScriptEnabled(true);
    … …  
    281282    EXPECT_EQ(0, webViewImpl->focusedWebCoreNode());
    282283
    283     webViewImpl->close();
     284    webViewImpl->close();
     285}
     286
     287// Implementation of WebFrameClient that tracks the v8 contexts that are created
     288// and destroyed for verification.
     289class ContextLifetimeTestWebFrameClient : public WebFrameClient {
     290public:
     291    struct Notification {
     292    public:
     293        Notification(WebFrame* frame, v8::Handle<v8::Context> context, int worldId)
     294            : frame(frame) ,
     295              context(v8::Persistent<v8::Context>::New(context)),
     296              worldId(worldId)
     297        {
     298        }
     299
     300        ~Notification()
     301        {
     302            context.Dispose();
     303        }
     304
     305        bool Equals(Notification* other)
     306        {
     307            return other && frame == other->frame && context == other->context && worldId == other->worldId;
     308        }
     309
     310        WebFrame* frame;
     311        v8::Persistent<v8::Context> context;
     312        int worldId;
     313    };
     314
     315    ~ContextLifetimeTestWebFrameClient()
     316    {
     317        reset();
     318    }
     319
     320    void reset()
     321    {
     322        for (size_t i = 0; i < createNotifications.size(); ++i)
     323            delete createNotifications[i];
     324
     325        for (size_t i = 0; i < releaseNotifications.size(); ++i)
     326            delete releaseNotifications[i];
     327
     328        createNotifications.clear();
     329        releaseNotifications.clear();
     330    }
     331
     332    std::vector<Notification*> createNotifications;
     333    std::vector<Notification*> releaseNotifications;
     334
     335 private:
     336    virtual void didCreateScriptContext(WebFrame* frame, v8::Handle<v8::Context> context, int worldId)
     337    {
     338        createNotifications.push_back(new Notification(frame, context, worldId));
     339    }
     340
     341    virtual void willReleaseScriptContext(WebFrame* frame, v8::Handle<v8::Context> context, int worldId)
     342    {
     343        releaseNotifications.push_back(new Notification(frame, context, worldId));
     344    }
     345};
     346
     347TEST_F(WebFrameTest, ContextNotificationsLoadUnload)
     348{
     349    v8::HandleScope handleScope;
     350
     351    registerMockedHttpURLLoad("context_notifications_test.html");
     352    registerMockedHttpURLLoad("context_notifications_test_frame.html");
     353
     354    // Load a frame with an iframe, make sure we get the right create notifications.
     355    ContextLifetimeTestWebFrameClient webFrameClient;
     356    WebView* webView = WebView::create(0);
     357    webView->settings()->setJavaScriptEnabled(true);
     358    webView->initializeMainFrame(&webFrameClient);
     359    loadHttpFrame(webView->mainFrame(), "context_notifications_test.html");
     360    serveRequests();
     361
     362    WebFrame* mainFrame = webView->mainFrame();
     363    WebFrame* childFrame = mainFrame->firstChild();
     364
     365    ASSERT_EQ(2u, webFrameClient.createNotifications.size());
     366    EXPECT_EQ(0u, webFrameClient.releaseNotifications.size());
     367
     368    ContextLifetimeTestWebFrameClient::Notification* firstCreateNotification = webFrameClient.createNotifications[0];
     369    ContextLifetimeTestWebFrameClient::Notification* secondCreateNotification = webFrameClient.createNotifications[1];
     370
     371    EXPECT_EQ(mainFrame, firstCreateNotification->frame);
     372    EXPECT_EQ(mainFrame->mainWorldScriptContext(), firstCreateNotification->context);
     373    EXPECT_EQ(0, firstCreateNotification->worldId);
     374
     375    EXPECT_EQ(childFrame, secondCreateNotification->frame);
     376    EXPECT_EQ(childFrame->mainWorldScriptContext(), secondCreateNotification->context);
     377    EXPECT_EQ(0, secondCreateNotification->worldId);
     378
     379    // Close the view. We should get two release notifications that are exactly the same as the create ones, in reverse order.
     380    webView->close();
     381
     382    ASSERT_EQ(2u, webFrameClient.releaseNotifications.size());
     383    ContextLifetimeTestWebFrameClient::Notification* firstReleaseNotification = webFrameClient.releaseNotifications[0];
     384    ContextLifetimeTestWebFrameClient::Notification* secondReleaseNotification = webFrameClient.releaseNotifications[1];
     385
     386    ASSERT_TRUE(firstCreateNotification->Equals(secondReleaseNotification));
     387    ASSERT_TRUE(secondCreateNotification->Equals(firstReleaseNotification));
     388}
     389
     390TEST_F(WebFrameTest, ContextNotificationsReload)
     391{
     392    v8::HandleScope handleScope;
     393
     394    registerMockedHttpURLLoad("context_notifications_test.html");
     395    registerMockedHttpURLLoad("context_notifications_test_frame.html");
     396
     397    ContextLifetimeTestWebFrameClient webFrameClient;
     398    WebView* webView = WebView::create(0);
     399    webView->settings()->setJavaScriptEnabled(true);
     400    webView->initializeMainFrame(&webFrameClient);
     401    loadHttpFrame(webView->mainFrame(), "context_notifications_test.html");
     402    serveRequests();
     403
     404    // Refresh, we should get two release notifications and two more create notifications.
     405    webView->mainFrame()->reload(false);
     406    serveRequests();
     407    ASSERT_EQ(4u, webFrameClient.createNotifications.size());
     408    ASSERT_EQ(2u, webFrameClient.releaseNotifications.size());
     409
     410    // The two release notifications we got should be exactly the same as the first two create notifications.
     411    for (size_t i = 0; i < webFrameClient.releaseNotifications.size(); ++i) {
     412      EXPECT_TRUE(webFrameClient.releaseNotifications[i]->Equals(
     413          webFrameClient.createNotifications[webFrameClient.createNotifications.size() - 3 - i]));
     414    }
     415
     416    // The last two create notifications should be for the current frames and context.
     417    WebFrame* mainFrame = webView->mainFrame();
     418    WebFrame* childFrame = mainFrame->firstChild();
     419    ContextLifetimeTestWebFrameClient::Notification* firstRefreshNotification = webFrameClient.createNotifications[2];
     420    ContextLifetimeTestWebFrameClient::Notification* secondRefreshNotification = webFrameClient.createNotifications[3];
     421
     422    EXPECT_EQ(mainFrame, firstRefreshNotification->frame);
     423    EXPECT_EQ(mainFrame->mainWorldScriptContext(), firstRefreshNotification->context);
     424    EXPECT_EQ(0, firstRefreshNotification->worldId);
     425
     426    EXPECT_EQ(childFrame, secondRefreshNotification->frame);
     427    EXPECT_EQ(childFrame->mainWorldScriptContext(), secondRefreshNotification->context);
     428    EXPECT_EQ(0, secondRefreshNotification->worldId);
     429
     430    webView->close();
     431}
     432
     433TEST_F(WebFrameTest, ContextNotificationsIsolatedWorlds)
     434{
     435    v8::HandleScope handleScope;
     436
     437    registerMockedHttpURLLoad("context_notifications_test.html");
     438    registerMockedHttpURLLoad("context_notifications_test_frame.html");
     439
     440    ContextLifetimeTestWebFrameClient webFrameClient;
     441    WebView* webView = WebView::create(0);
     442    webView->settings()->setJavaScriptEnabled(true);
     443    webView->initializeMainFrame(&webFrameClient);
     444    loadHttpFrame(webView->mainFrame(), "context_notifications_test.html");
     445    serveRequests();
     446
     447    // Add an isolated world.
     448    webFrameClient.reset();
     449
     450    int isolatedWorldId = 42;
     451    WebScriptSource scriptSource("hi!");
     452    int numSources = 1;
     453    int extensionGroup = 0;
     454    webView->mainFrame()->executeScriptInIsolatedWorld(isolatedWorldId, &scriptSource, numSources, extensionGroup);
     455
     456    // We should now have a new create notification.
     457    ASSERT_EQ(1u, webFrameClient.createNotifications.size());
     458    ContextLifetimeTestWebFrameClient::Notification* notification = webFrameClient.createNotifications[0];
     459    ASSERT_EQ(isolatedWorldId, notification->worldId);
     460    ASSERT_EQ(webView->mainFrame(), notification->frame);
     461
     462    // 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.
     463    ASSERT_NE(webView->mainFrame()->mainWorldScriptContext(), notification->context);
     464
     465    webView->close();
     466
     467    // We should have gotten three release notifications (one for each of the frames, plus one for the isolated context).
     468    ASSERT_EQ(3u, webFrameClient.releaseNotifications.size());
     469
     470    // And one of them should be exactly the same as the create notification for the isolated context.
     471    int matchCount = 0;
     472    for (size_t i = 0; i < webFrameClient.releaseNotifications.size(); ++i) {
     473      if (webFrameClient.releaseNotifications[i]->Equals(webFrameClient.createNotifications[0]))
     474        ++matchCount;
     475    }
     476    EXPECT_EQ(1, matchCount);
    284477}
    285478
Note: See TracChangeset for help on using the changeset viewer.