Changeset 73373 in webkit

Timestamp:

Dec 6, 2010 9:35:05 AM (13 years ago)

Author:

tony@chromium.org

Message:

2010-12-03 Tony Chang <​tony@chromium.org>

Reviewed by Kent Tamura.

[chromium] fix 2 bugs with inspector tests in DRT
​https://bugs.webkit.org/show_bug.cgi?id=50492

Tasks can outlive the tasklist (even when canceled) so this was
causing a crash when ~WebTask() ran. Avoid this by unregistering
when a task is canceled.

Also fix an assert when closing devtool windows by copying some
logic from test_shell.

• DumpRenderTree/chromium/Task.cpp: (WebTask::~WebTask): (TaskList::revokeAll):
• DumpRenderTree/chromium/Task.h: Canceling a task now removes it from

the tasklist (since the task can outlive the tasklist).

• DumpRenderTree/chromium/TestShell.h: (TestShell::devToolsWebView):
• DumpRenderTree/chromium/WebViewHost.cpp: (WebViewHost::~WebViewHost): Don't load about:blank when closing

a window if the window has devtools loaded. This avoids an
ASSERT and matches test_shell.

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• DumpRenderTree/chromium/Task.cpp (modified) (2 diffs)
• DumpRenderTree/chromium/Task.h (modified) (2 diffs)
• DumpRenderTree/chromium/TestShell.h (modified) (1 diff)
• DumpRenderTree/chromium/WebViewHost.cpp (modified) (1 diff)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2010-12-03  Tony Chang  <tony@chromium.org>
+
+        Reviewed by Kent Tamura.
+
+        [chromium] fix 2 bugs with inspector tests in DRT
+        https://bugs.webkit.org/show_bug.cgi?id=50492
+
+        Tasks can outlive the tasklist (even when canceled) so this was
+        causing a crash when ~WebTask() ran.  Avoid this by unregistering
+        when a task is canceled.
+
+        Also fix an assert when closing devtool windows by copying some
+        logic from test_shell.
+
+        * DumpRenderTree/chromium/Task.cpp:
+        (WebTask::~WebTask):
+        (TaskList::revokeAll):
+        * DumpRenderTree/chromium/Task.h: Canceling a task now removes it from
+            the tasklist (since the task can outlive the tasklist).
+        * DumpRenderTree/chromium/TestShell.h:
+        (TestShell::devToolsWebView):
+        * DumpRenderTree/chromium/WebViewHost.cpp:
+        (WebViewHost::~WebViewHost): Don't load about:blank when closing
+            a window if the window has devtools loaded.  This avoids an
+            ASSERT and matches test_shell.
+
 2010-12-06  Alejandro G. Castro  <alex@igalia.com>
 

trunk/WebKitTools/DumpRenderTree/chromium/Task.cpp

@@ -37 +37 @@
 
 WebTask::WebTask(TaskList* list): m_taskList(list) { m_taskList->registerTask(this); }
-WebTask::~WebTask() { m_taskList->unregisterTask(this); }
+WebTask::~WebTask()
+{
+    if (m_taskList)
+        m_taskList->unregisterTask(this);
+}
 
 void TaskList::unregisterTask(WebTask* task)
@@ -48 +52 @@
 void TaskList::revokeAll()
 {
-    for (unsigned i = 0; i < m_tasks.size(); ++i)
-        m_tasks[i]->cancel();
-    m_tasks.clear();
+    while (!m_tasks.isEmpty())
+        m_tasks[0]->cancel();
 }
 

trunk/WebKitTools/DumpRenderTree/chromium/Task.h

@@ -46 +46 @@
     virtual void cancel() = 0;
     virtual ~WebTask();
-private:
+protected:
     TaskList* m_taskList;
 };
@@ -72 +72 @@
             runIfValid();
     }
-    virtual void cancel() { m_object = 0; }
+    virtual void cancel()
+    {
+        m_object = 0;
+        m_taskList->unregisterTask(this);
+        m_taskList = 0;
+    }
     virtual void runIfValid() = 0;
 protected:

trunk/WebKitTools/DumpRenderTree/chromium/TestShell.h

@@ -164 +164 @@
     DRTDevToolsAgent* drtDevToolsAgent() { return m_drtDevToolsAgent.get(); }
     DRTDevToolsClient* drtDevToolsClient() { return m_drtDevToolsClient.get(); }
+    WebViewHost* devToolsWebView() { return m_devTools; }
 
     static const int virtualWindowBorder = 3;

trunk/WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp

@@ -1112 +1112 @@
 WebViewHost::~WebViewHost()
 {
-    // Navigate to an empty page to fire all the destruction logic for the
-    // current page.
-    loadURLForFrame(GURL("about:blank"), WebString());
+    // DevTools frontend page is supposed to be navigated only once and
+    // loading another URL in that Page is an error.
+    if (m_shell->devToolsWebView() != this) {
+        // Navigate to an empty page to fire all the destruction logic for the
+        // current page.
+        loadURLForFrame(GURL("about:blank"), WebString());
+    }
 
     // Call GC twice to clean up garbage.