Changeset 62774 in webkit

Timestamp:

Jul 8, 2010 2:37:32 AM (14 years ago)

Author:

apavlov@chromium.org

Message:

2010-07-07 Alexander Pavlov <​apavlov@chromium.org>

Reviewed by Darin Fisher.

[Chromium] Crash when re-entering message loop

Allow re-entrancy of the message loop by tracking all suspended pages
through a PageGroupLoadDeferrer stack.
​https://bugs.webkit.org/show_bug.cgi?id=41697

WebCore:

• page/PageGroupLoadDeferrer.cpp: (WebCore::PageGroupLoadDeferrer::PageGroupLoadDeferrer):
• page/PageGroupLoadDeferrer.h: (WebCore::PageGroupLoadDeferrer::nextDeferrer):

WebKit/chromium:

• src/WebViewImpl.cpp: (WebKit::WebView::willEnterModalLoop): (WebKit::WebView::didExitModalLoop):
• src/js/Tests.js: (.TestSuite.prototype.evaluateInConsole_): (.TestSuite.prototype.testCompletionOnPause): (.TestSuite.prototype.testMessageLoopReentrant.this):

Location:

trunk

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/PageGroupLoadDeferrer.cpp (modified) (1 diff)
• WebCore/page/PageGroupLoadDeferrer.h (modified) (1 diff)
• WebKit/chromium/ChangeLog (modified) (1 diff)
• WebKit/chromium/src/WebViewImpl.cpp (modified) (2 diffs)
• WebKit/chromium/src/js/Tests.js (modified) (5 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-07-07  Alexander Pavlov  <apavlov@chromium.org>
+
+        Reviewed by Darin Fisher.
+
+        [Chromium] Crash when re-entering message loop
+        https://bugs.webkit.org/show_bug.cgi?id=41697
+
+        A Chromium-specific test case will be submitted into Chromium shortly.
+
+        * page/PageGroupLoadDeferrer.cpp:
+        (WebCore::PageGroupLoadDeferrer::PageGroupLoadDeferrer):
+        * page/PageGroupLoadDeferrer.h:
+        (WebCore::PageGroupLoadDeferrer::nextDeferrer):
+
 2010-07-07  Yury Semikhatsky  <yurys@chromium.org>
 

trunk/WebCore/page/PageGroupLoadDeferrer.cpp

@@ -31 +31 @@
 using namespace std;
 
-PageGroupLoadDeferrer::PageGroupLoadDeferrer(Page* page, bool deferSelf)
+PageGroupLoadDeferrer::PageGroupLoadDeferrer(Page* page, bool deferSelf, PageGroupLoadDeferrer* nextDeferrer)
+    : m_nextDeferrer(nextDeferrer)
 {
     const HashSet<Page*>& pages = page->group().pages();

trunk/WebCore/page/PageGroupLoadDeferrer.h

@@ -31 +31 @@
     class PageGroupLoadDeferrer : public Noncopyable {
     public:
-        PageGroupLoadDeferrer(Page*, bool deferSelf);
+        PageGroupLoadDeferrer(Page*, bool deferSelf, PageGroupLoadDeferrer* nextDeferrer = 0);
         ~PageGroupLoadDeferrer();
+        PageGroupLoadDeferrer* nextDeferrer() { return m_nextDeferrer; }
 
     private:
         Vector<RefPtr<Frame>, 16> m_deferredFrames;
+        PageGroupLoadDeferrer* m_nextDeferrer;
     };
 }

trunk/WebKit/chromium/ChangeLog

@@ +1 @@
+2010-07-07  Alexander Pavlov  <apavlov@chromium.org>
+
+        Reviewed by Darin Fisher.
+
+        [Chromium] Crash when re-entering message loop
+
+        Allow re-entrancy of the message loop by tracking all suspended pages
+        through a PageGroupLoadDeferrer stack.
+        https://bugs.webkit.org/show_bug.cgi?id=41697
+
+        * src/WebViewImpl.cpp:
+        (WebKit::WebView::willEnterModalLoop):
+        (WebKit::WebView::didExitModalLoop):
+        * src/js/Tests.js:
+        (.TestSuite.prototype.evaluateInConsole_):
+        (.TestSuite.prototype.testCompletionOnPause):
+        (.TestSuite.prototype.testMessageLoopReentrant.this):
+
 2010-07-08  David Holloway  <dhollowa@chromium.org>
 

trunk/WebKit/chromium/src/WebViewImpl.cpp

@@ -186 +186 @@
 void WebView::willEnterModalLoop()
 {
-    // It is not valid to nest more than once.
-    ASSERT(!pageGroupLoadDeferrer);
-
     PageGroup* pageGroup = PageGroup::pageGroup(pageGroupName);
     ASSERT(pageGroup);
@@ -196 +193 @@
 
     // Pick any page in the page group since we are deferring all pages.
-    pageGroupLoadDeferrer = new PageGroupLoadDeferrer(*pageGroup->pages().begin(), true);
+    pageGroupLoadDeferrer = new PageGroupLoadDeferrer(*pageGroup->pages().begin(), true, pageGroupLoadDeferrer);
 }
 
 void WebView::didExitModalLoop()
 {
+    ASSERT(pageGroupLoadDeferrer);
+    PageGroupLoadDeferrer* nextDeferrer = pageGroupLoadDeferrer->nextDeferrer();
+
     delete pageGroupLoadDeferrer;
-    pageGroupLoadDeferrer = 0;
+    pageGroupLoadDeferrer = nextDeferrer;
 }
 

trunk/WebKit/chromium/src/js/Tests.js

@@ -853 +853 @@
 TestSuite.prototype.evaluateInConsole_ = function(code, callback)
 {
-    WebInspector.console.visible = true;
+    WebInspector.showConsole();
     WebInspector.console.prompt.text = code;
     WebInspector.console.promptElement.dispatchEvent( TestSuite.createKeyEvent("Enter"));
@@ -953 +953 @@
 
     function showConsole() {
-        test.addSniffer(WebInspector.console, "afterShow", testLocalsCompletion);
-        WebInspector.showConsole();
+        if (WebInspector.currentFocusElement === WebInspector.console.promptElement)
+            testLocalsCompletion();
+        else {
+            test.addSniffer(WebInspector.console, "afterShow", testLocalsCompletion);
+            WebInspector.showConsole();
+        }
     }
 
@@ -1776 +1780 @@
 TestSuite.prototype.testConsoleLog = function()
 {
-    WebInspector.console.visible = true;
+    WebInspector.showConsole();
     var messages = WebInspector.console.messages;
     var index = 0;
@@ -1818 +1822 @@
 TestSuite.prototype.testEvalGlobal = function()
 {
-    WebInspector.console.visible = true;
+    WebInspector.showConsole();
 
     var inputs = ["foo", "foobar"];
@@ -1851 +1855 @@
 
 /**
+ * Tests the message loop re-entrancy.
+ */
+TestSuite.prototype.testMessageLoopReentrant = function()
+{
+    var test = this;
+    this.showPanel("scripts");
+
+    var breakpointLine = 16;
+
+    WebInspector.showConsole();
+
+    this._waitUntilScriptsAreParsed(["debugger_test_page.html"],
+        function() {
+          test.showMainPageScriptSource_(
+              "debugger_test_page.html",
+              function(view, url) {
+                view._addBreakpoint(breakpointLine);
+
+                test.evaluateInConsole_(
+                    'setTimeout("calculate()", 0)',
+                    function(resultText) {
+                      test.assertTrue(!isNaN(resultText), "Failed to get timer id: " + resultText);
+                    });
+
+              });
+        });
+
+    // Wait until script is paused.
+    this.addSniffer(
+        WebInspector,
+        "pausedScript",
+        function(callFrames) {
+            test.evaluateInConsole_(
+                'document.cookie',
+                test.releaseControl.bind(test)); // This callback will be invoked only if the test succeeds (i.e. no crash).
+        });
+
+    this.takeControl();
+};
+
+
+/**
  * Tests that Storage panel can be open and that local DOM storage is added
  * to the panel.