Changeset 95385 in webkit


Ignore:
Timestamp:
Sep 17, 2011 3:47:09 PM (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:
Location:
trunk/Source
Files:
2 added
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r95383 r95385  
     12011-09-17  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-17  Ilya Tikhonovsky  <loislo@chromium.org>
    221

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

    r95354 r95385  
    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

    r93055 r95385  
    5050
    5151V8IsolatedContext::V8IsolatedContext(V8Proxy* proxy, int extensionGroup, int worldId)
    52     : m_world(IsolatedWorld::create(worldId))
     52    : m_world(IsolatedWorld::create(worldId)), m_frame(proxy->frame())
    5353{
    5454    v8::HandleScope scope;
    … …  
    6565    V8DOMWindowShell::installHiddenObjectPrototype(m_context->get());
    6666    // FIXME: This will go away once we have a windowShell for the isolated world.
    67     proxy->windowShell()->installDOMWindow(m_context->get(), proxy->frame()->domWindow());
     67    proxy->windowShell()->installDOMWindow(m_context->get(), m_frame->domWindow());
    6868
    6969    // Using the default security token means that the canAccess is always
    … …  
    7575    m_context->get()->UseDefaultSecurityToken();
    7676
    77     proxy->frame()->loader()->client()->didCreateIsolatedScriptContext(this);
     77    m_frame->loader()->client()->didCreateScriptContext(context(), m_world->id());
    7878}
    7979
    8080void V8IsolatedContext::destroy()
    8181{
     82    m_frame->loader()->client()->willReleaseScriptContext(context(), m_world->id());
    8283    m_context->get().MakeWeak(this, &contextWeakReferenceCallback);
     84    m_frame = 0;
    8385}
    8486

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

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

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

    r95271 r95385  
    4848
    4949#if USE(V8)
    50 #include "V8IsolatedContext.h"
     50#include <v8.h>
    5151#endif
    5252
    … …  
    398398
    399399#if USE(V8)
    400     virtual void didCreateScriptContextForFrame() { }
    401     virtual void didDestroyScriptContextForFrame() { }
    402     virtual void didCreateIsolatedScriptContext(V8IsolatedContext*) { }
     400    virtual void didCreateScriptContext(v8::Handle<v8::Context>, int worldId) { }
     401    virtual void willReleaseScriptContext(v8::Handle<v8::Context>, int worldId) { }
    403402    virtual bool allowScriptExtension(const String& extensionName, int extensionGroup) { return false; }
    404403#endif

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

    r95369 r95385  
    5252#endif
    5353
     54#if USE(V8)
     55namespace v8 {
     56class Context;
     57template<class T> class Handle;
     58}
     59#endif
     60
    5461namespace WebCore {
    5562
    … …  
    8895    class StringWithDirection;
    8996    class SubstituteData;
    90 #if USE(V8)
    91     class V8IsolatedContext;
    92 #endif
    9397    class Widget;
    9498
    … …  
    275279
    276280#if USE(V8)
    277         virtual void didCreateScriptContextForFrame() = 0;
    278         virtual void didDestroyScriptContextForFrame() = 0;
    279         virtual void didCreateIsolatedScriptContext(V8IsolatedContext*) = 0;
     281        virtual void didCreateScriptContext(v8::Handle<v8::Context>, int worldId) = 0;
     282        virtual void willReleaseScriptContext(v8::Handle<v8::Context>, int worldId) = 0;
    280283        virtual bool allowScriptExtension(const String& extensionName, int extensionGroup) = 0;
    281284#endif

  • trunk/Source/WebKit/chromium/ChangeLog

    r95380 r95385  
     12011-09-17  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
    1172011-09-17  chandra shekar vallala  <chandra.vallala@motorola.com>
    218

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

    r94231 r95385  
    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

    r95369 r95385  
    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        m_webFrame->client()->didCreateScriptContext(m_webFrame, context, worldId);
     148}
     149
     150void FrameLoaderClientImpl::willReleaseScriptContext(v8::Handle<v8::Context> context, int worldId)
     151{
     152    if (m_webFrame->client())
     153        m_webFrame->client()->willReleaseScriptContext(m_webFrame, context, worldId);
    160154}
    161155#endif

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

    r95369 r95385  
    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

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