Changeset 148083 in webkit


Ignore:
Timestamp:
Apr 10, 2013 12:39:18 AM (11 years ago)
Author:
zandobersek@gmail.com
Message:

REGRESSION (r146518): WebKit2APITests/TestInspector is failing
​https://bugs.webkit.org/show_bug.cgi?id=113281

Reviewed by Darin Adler.

Source/WebKit2:

Changes to the WebInspectorProxy opening processing in r146518 caused the change in how the GTK-specific
WebInspectorProxy code operates, specifically the 'bring-to-front' signal is not emitted anymore when opening the
inspector due to the WebInspectorProxy::bringToFront method now only bringing the inspector window to front if it exists
and opening it (and thus unable to bring it to front) otherwise.

Closing of the inspector through the didClose method is now done immediately after sending the WebInspector::Close()
message to the WebProcess rather than waiting for the WebProcess to communicate back the order of closing. Due to this
the relevant code in the test cases had to be changed to not run the loop but rather just check that the closing was
successful.

(InspectorTest::InspectorTest): Remove the initialization of the now redundant m_quitOnBringToFront member variable.
(InspectorTest::bringToFront): Quit the loop without checking the removed member variable.
(InspectorTest::closed): Do not quit the loop as it is not run anymore.
(InspectorTest::showIdle): A helper method that asynchronously calls the webkit_web_inspector_show method, removing some
unnecessary complexity about under what exact circumstances to quit the loop in open-window/bring-to-front callbacks.
(InspectorTest::show): Replaces the showAndWaitUntilFinished method, adding an idle invocation of the showIdle helper
method and running the loop.
(InspectorTest::close): Formerly closeAndWaitUntilClosed, now does not run the loop anymore as there's no need for it.
(testInspectorDefault): Only the window opening event is now expected upon showing the inspector for the first time.
Adjusting callsites to use InspectorTest::show and InspectorTest::close instead of
InspectorTest::showAndWaitUntilFinished and InspectorTest::showAndWaitUntilFinished.
(CustomInspectorTest::destroyWindow): Formerly destroyWindowAndWaitUntilClosed, the closing is not asynchronous anymore
so the loop is not run.
(testInspectorManualAttachDetach): Only the window opening event is now expected upon showing the inspector for the first time.
Adjusting callsites to use InspectorTest::show and InspectorTest::close instead of
InspectorTest::showAndWaitUntilFinished and InspectorTest::showAndWaitUntilFinished.
(testInspectorCustomContainerDestroyed): Adjusting callsites to use InspectorTest::show and CustomInspectorTest::destroyWindow
instead of InspectorTest::showAndWaitUntilFinished and CustomInspectorTest::destroyWindowAndWaitUntilClosed.

  • UIProcess/WebInspectorProxy.cpp:

(WebKit::WebInspectorProxy::show): When showing a connected WebInspectorProxy, call the bringToFront method which will
open the inspector if it's not yet visible or bring it to the front otherwise.

Tools:

  • Scripts/run-gtk-tests:

(TestRunner): Remove the skip entry for the WebKit2APITests/TestInspector unit test.

Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit2/ChangeLog

    r148082 r148083  
     12013-04-10  Zan Dobersek  <zdobersek@igalia.com>
     2
     3        REGRESSION (r146518): WebKit2APITests/TestInspector is failing
     4        https://bugs.webkit.org/show_bug.cgi?id=113281
     5
     6        Reviewed by Darin Adler.
     7
     8        Changes to the WebInspectorProxy opening processing in r146518 caused the change in how the GTK-specific
     9        WebInspectorProxy code operates, specifically the 'bring-to-front' signal is not emitted anymore when opening the
     10        inspector due to the WebInspectorProxy::bringToFront method now only bringing the inspector window to front if it exists
     11        and opening it (and thus unable to bring it to front) otherwise.
     12
     13        Closing of the inspector through the didClose method is now done immediately after sending the WebInspector::Close()
     14        message to the WebProcess rather than waiting for the WebProcess to communicate back the order of closing. Due to this
     15        the relevant code in the test cases had to be changed to not run the loop but rather just check that the closing was
     16        successful.
     17
     18        (InspectorTest::InspectorTest): Remove the initialization of the now redundant m_quitOnBringToFront member variable.
     19        (InspectorTest::bringToFront): Quit the loop without checking the removed member variable.
     20        (InspectorTest::closed): Do not quit the loop as it is not run anymore.
     21        (InspectorTest::showIdle): A helper method that asynchronously calls the webkit_web_inspector_show method, removing some
     22        unnecessary complexity about under what exact circumstances to quit the loop in open-window/bring-to-front callbacks.
     23        (InspectorTest::show): Replaces the showAndWaitUntilFinished method, adding an idle invocation of the showIdle helper
     24        method and running the loop.
     25        (InspectorTest::close): Formerly closeAndWaitUntilClosed, now does not run the loop anymore as there's no need for it.
     26        (testInspectorDefault): Only the window opening event is now expected upon showing the inspector for the first time.
     27        Adjusting callsites to use InspectorTest::show and InspectorTest::close instead of
     28        InspectorTest::showAndWaitUntilFinished and InspectorTest::showAndWaitUntilFinished.
     29        (CustomInspectorTest::destroyWindow): Formerly destroyWindowAndWaitUntilClosed, the closing is not asynchronous anymore
     30        so the loop is not run.
     31        (testInspectorManualAttachDetach): Only the window opening event is now expected upon showing the inspector for the first time.
     32        Adjusting callsites to use InspectorTest::show and InspectorTest::close instead of
     33        InspectorTest::showAndWaitUntilFinished and InspectorTest::showAndWaitUntilFinished.
     34        (testInspectorCustomContainerDestroyed): Adjusting callsites to use InspectorTest::show and CustomInspectorTest::destroyWindow
     35        instead of InspectorTest::showAndWaitUntilFinished and CustomInspectorTest::destroyWindowAndWaitUntilClosed.
     36        * UIProcess/WebInspectorProxy.cpp:
     37        (WebKit::WebInspectorProxy::show): When showing a connected WebInspectorProxy, call the bringToFront method which will
     38        open the inspector if it's not yet visible or bring it to the front otherwise.
     39
    1402013-04-10  Zan Dobersek  <zandobersek@gmail.com>
    241

  • trunk/Source/WebKit2/UIProcess/API/gtk/tests/TestInspector.cpp

    r148082 r148083  
    6666        : WebViewTest()
    6767        , m_inspector(webkit_web_view_get_inspector(m_webView))
    68         , m_quitOnBringToFront(false)
    6968    {
    7069        webkit_settings_set_enable_developer_extras(webkit_web_view_get_settings(m_webView), TRUE);
    … …  
    9291    {
    9392        m_events.append(BringToFront);
    94         if (m_quitOnBringToFront)
    95             g_main_loop_quit(m_mainLoop);
     93        g_main_loop_quit(m_mainLoop);
    9694        return FALSE;
    9795    }
    … …  
    10098    {
    10199        m_events.append(Closed);
    102         g_main_loop_quit(m_mainLoop);
    103100    }
    104101
    … …  
    115112    }
    116113
    117     void showAndWaitUntilFinished(bool quitOnBringToFront)
    118     {
    119         m_quitOnBringToFront = quitOnBringToFront;
    120         webkit_web_inspector_show(m_inspector);
     114
     115    static gboolean showIdle(InspectorTest* test)
     116    {
     117        webkit_web_inspector_show(test->m_inspector);
     118        return FALSE;
     119    }
     120
     121    void show()
     122    {
     123        g_idle_add(reinterpret_cast<GSourceFunc>(showIdle), this);
    121124        g_main_loop_run(m_mainLoop);
    122         m_quitOnBringToFront = false;
    123125    }
    124126
    … …  
    142144    }
    143145
    144     void closeAndWaitUntilClosed()
     146    void close()
    145147    {
    146148        webkit_web_inspector_close(m_inspector);
    147         g_main_loop_run(m_mainLoop);
    148149    }
    149150
    150151    WebKitWebInspector* m_inspector;
    151     bool m_quitOnBringToFront;
    152152    Vector<InspectorEvents> m_events;
    153153};
    … …  
    160160    test->waitUntilLoadFinished();
    161161
    162     test->showAndWaitUntilFinished(false);
     162    test->show();
    163163    // We don't add the view to a container, so consume the weak ref with GRefPtr.
    164164    GRefPtr<WebKitWebViewBase> inspectorView = webkit_web_inspector_get_web_view(test->m_inspector);
    … …  
    168168    g_assert_cmpuint(webkit_web_inspector_get_attached_height(test->m_inspector), ==, 0);
    169169    Vector<InspectorTest::InspectorEvents>& events = test->m_events;
    170     g_assert_cmpint(events.size(), ==, 2);
    171     g_assert_cmpint(events[0], ==, InspectorTest::BringToFront);
    172     g_assert_cmpint(events[1], ==, InspectorTest::OpenWindow);
    173     test->m_events.clear();
    174 
    175     test->showAndWaitUntilFinished(true);
     170    g_assert_cmpint(events.size(), ==, 1);
     171    g_assert_cmpint(events[0], ==, InspectorTest::OpenWindow);
     172    test->m_events.clear();
     173
     174    test->show();
    176175    events = test->m_events;
    177176    g_assert_cmpint(events.size(), ==, 1);
    … …  
    195194    test->m_events.clear();
    196195
    197     test->closeAndWaitUntilClosed();
     196    test->close();
    198197    events = test->m_events;
    199198    g_assert_cmpint(events.size(), ==, 1);
    … …  
    274273    }
    275274
    276     void destroyWindowAndWaitUntilClosed()
     275    void destroyWindow()
    277276    {
    278277        g_assert(m_inspectorWindow);
    279278        gtk_widget_destroy(m_inspectorWindow);
    280279        m_inspectorWindow = 0;
    281         g_main_loop_run(m_mainLoop);
    282280    }
    283281
    … …  
    292290    test->waitUntilLoadFinished();
    293291
    294     test->showAndWaitUntilFinished(false);
     292    test->show();
    295293    test->assertObjectIsDeletedWhenTestFinishes(G_OBJECT(webkit_web_inspector_get_web_view(test->m_inspector)));
    296294    g_assert(!webkit_web_inspector_is_attached(test->m_inspector));
    297295    Vector<InspectorTest::InspectorEvents>& events = test->m_events;
    298     g_assert_cmpint(events.size(), ==, 2);
    299     g_assert_cmpint(events[0], ==, InspectorTest::BringToFront);
    300     g_assert_cmpint(events[1], ==, InspectorTest::OpenWindow);
     296    g_assert_cmpint(events.size(), ==, 1);
     297    g_assert_cmpint(events[0], ==, InspectorTest::OpenWindow);
    301298    test->m_events.clear();
    302299
    … …  
    320317    g_assert(webkit_web_inspector_is_attached(test->m_inspector));
    321318    test->m_events.clear();
    322     test->closeAndWaitUntilClosed();
     319    test->close();
    323320    events = test->m_events;
    324321    g_assert_cmpint(events.size(), ==, 2);
    … …  
    335332    test->waitUntilLoadFinished();
    336333
    337     test->showAndWaitUntilFinished(false);
     334    test->show();
    338335    test->assertObjectIsDeletedWhenTestFinishes(G_OBJECT(webkit_web_inspector_get_web_view(test->m_inspector)));
    339336    g_assert(!webkit_web_inspector_is_attached(test->m_inspector));
    340337
    341338    test->m_events.clear();
    342     test->destroyWindowAndWaitUntilClosed();
     339    test->destroyWindow();
    343340    Vector<InspectorTest::InspectorEvents>& events = test->m_events;
    344341    g_assert_cmpint(events.size(), ==, 1);

  • trunk/Source/WebKit2/UIProcess/WebInspectorProxy.cpp

    r147362 r148083  
    154154
    155155    if (isConnected()) {
    156         open();
     156        bringToFront();
    157157        return;
    158158    }

  • trunk/Tools/ChangeLog

    r148077 r148083  
     12013-04-10  Zan Dobersek  <zdobersek@igalia.com>
     2
     3        REGRESSION (r146518): WebKit2APITests/TestInspector is failing
     4        https://bugs.webkit.org/show_bug.cgi?id=113281
     5
     6        Reviewed by Darin Adler.
     7
     8        * Scripts/run-gtk-tests:
     9        (TestRunner): Remove the skip entry for the WebKit2APITests/TestInspector unit test.
     10
    1112013-04-09  Glenn Adams  <glenn@skynav.com>
    212

  • trunk/Tools/Scripts/run-gtk-tests

    r147559 r148083  
    6868        SkippedTest("unittests/testwebview", "/webkit/webview/icon-uri", "Test times out in GTK Linux 64-bit Release bot", 82328),
    6969        SkippedTest("unittests/testatk", "/webkit/atk/getTextInParagraphAndBodyModerate", "Test fails", 105538),
    70         SkippedTest("WebKit2APITests/TestInspector", SkippedTest.ENTIRE_SUITE, "Test fails", 113281),
    7170        SkippedTest("WebKit2APITests/TestInspectorServer", SkippedTest.ENTIRE_SUITE, "Test times out", 105866),
    7271        SkippedTest("WebKit2APITests/TestResources", "/webkit2/WebKitWebView/resources", "Test is flaky in GTK Linux 32-bit Release bot", 82868),
Note: See TracChangeset for help on using the changeset viewer.