Changeset 271514 in webkit


Ignore:
Timestamp:
Jan 15, 2021 2:51:27 AM (18 months ago)
Author:
commit-queue@webkit.org
Message:

Use event loop to set title
​https://bugs.webkit.org/show_bug.cgi?id=218496

Patch by Rob Buis <​rbuis@igalia.com> on 2021-01-15
Reviewed by Ryosuke Niwa.

Source/WebCore:

Use event loop to set title to avoid calling WebFrameLoaderClient
within HTMLTitleElement::insertedIntoAncestor.

  • dom/Document.cpp:

(WebCore::Document::updateTitle):

  • dom/Document.h:

Tools:

Adapt unit tests to wait for title change tasks
to be processed.

  • TestWebKitAPI/Tests/WebKit/PageLoadState.cpp:

(TestWebKitAPI::didChangeTitle):
(TestWebKitAPI::TEST):

  • TestWebKitAPI/Tests/WebKitCocoa/UIDelegate.mm:

(TEST):

LayoutTests:

Adapt tests to make sure pending title change tasks
are processed before the test is done.

  • fast/dom/title-text-property-2.html:
  • fast/dom/title-text-property-assigning-empty-string.html:
  • fast/dom/title-text-property.html:
  • http/tests/globalhistory/history-delegate-basic-title-expected.txt:
  • http/tests/globalhistory/history-delegate-basic-title.html:
  • http/tests/loading/basic-auth-load-URL-with-consecutive-slashes-expected.txt:
  • http/tests/loading/basic-auth-load-URL-with-consecutive-slashes.html:
  • http/tests/loading/redirect-with-no-location-crash-expected.txt:
  • http/tests/loading/redirect-with-no-location-crash.html:
  • platform/mac-wk2/TestExpectations:
  • platform/win/http/tests/loading/basic-auth-load-URL-with-consecutive-slashes-expected.txt: Copied from LayoutTests/http/tests/loading/basic-auth-load-URL-with-consecutive-slashes-expected.txt.
  • platform/wk2/http/tests/loading/basic-auth-load-URL-with-consecutive-slashes-expected.txt:
  • platform/wk2/http/tests/loading/redirect-with-no-location-crash-expected.txt:
Location:
trunk
Files:
19 edited
1 copied

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r271510 r271514  
     12021-01-15  Rob Buis  <rbuis@igalia.com>
     2
     3        Use event loop to set title
     4        https://bugs.webkit.org/show_bug.cgi?id=218496
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Adapt tests to make sure pending title change tasks
     9        are processed before the test is done.
     10
     11        * fast/dom/title-text-property-2.html:
     12        * fast/dom/title-text-property-assigning-empty-string.html:
     13        * fast/dom/title-text-property.html:
     14        * http/tests/globalhistory/history-delegate-basic-title-expected.txt:
     15        * http/tests/globalhistory/history-delegate-basic-title.html:
     16        * http/tests/loading/basic-auth-load-URL-with-consecutive-slashes-expected.txt:
     17        * http/tests/loading/basic-auth-load-URL-with-consecutive-slashes.html:
     18        * http/tests/loading/redirect-with-no-location-crash-expected.txt:
     19        * http/tests/loading/redirect-with-no-location-crash.html:
     20        * platform/mac-wk2/TestExpectations:
     21        * platform/win/http/tests/loading/basic-auth-load-URL-with-consecutive-slashes-expected.txt: Copied from LayoutTests/http/tests/loading/basic-auth-load-URL-with-consecutive-slashes-expected.txt.
     22        * platform/wk2/http/tests/loading/basic-auth-load-URL-with-consecutive-slashes-expected.txt:
     23        * platform/wk2/http/tests/loading/redirect-with-no-location-crash-expected.txt:
     24
    1252021-01-14  Julian Gonzalez  <julian_a_gonzalez@apple.com>
    226

  • trunk/LayoutTests/fast/dom/title-text-property-2.html

    r210833 r271514  
    22<head>
    33<script>
    4 function runTests() {
     4function startTest() {
    55    if (window.testRunner) {
     6        testRunner.waitUntilDone();
    67        testRunner.dumpAsText();
    78        testRunner.dumpTitleChanges();
    … …  
    1112    document.title = 'TITLE1';
    1213
     14    internals.queueTask("DOMManipulation", () => continueTest());
     15}
     16
     17function continueTest() {
    1318    title = document.getElementsByTagName('title').item(0);
    1419   
    … …  
    1621    title.text = 'TITLE2';
    1722   
     23    internals.queueTask("DOMManipulation", () => endTest());
     24}
     25
     26function endTest() {
    1827    newTitle = document.createElement('title');
    1928    console.log("Should not set title");
    … …  
    2938        titleElement.parentNode.removeChild(titleElement);
    3039    }
     40
     41    internals.queueTask("DOMManipulation", () => testRunner.notifyDone());
    3142}
    3243</script>
    3344<title>Initial title</title>
    3445</head>
    35 <body onload="runTests();" >
     46<body onload="startTest();" >
    3647</body>
    3748</html>

  • trunk/LayoutTests/fast/dom/title-text-property-assigning-empty-string.html

    r128943 r271514  
    22<head>
    33<script>
    4 function runTests() {
     4function startTest() {
    55    if (window.testRunner) {
     6        testRunner.waitUntilDone();
    67        testRunner.dumpAsText();
    78        testRunner.dumpTitleChanges();
    … …  
    910
    1011    document.title = 'New non-empty title';
     12    internals.queueTask("DOMManipulation", () => endTest())
     13}
     14
     15function endTest() {
    1116    document.title = '';
     17    internals.queueTask("DOMManipulation", () => testRunner.notifyDone())
    1218}
    1319</script>
    1420</head>
    15 <body onload='runTests();'>
     21<body onload='startTest();'>
    1622</body>
    1723</html>

  • trunk/LayoutTests/fast/dom/title-text-property.html

    r269896 r271514  
    1313function runTests() {
    1414    if (window.testRunner) {
     15        testRunner.waitUntilDone();
    1516        testRunner.dumpAsText();
    1617        testRunner.dumpTitleChanges();
    … …  
    2526
    2627    debugOutput('New title is: \'' + titleElem.text + '\'');
     28
     29    internals.queueTask("DOMManipulation", () => testRunner.notifyDone());
    2730}
    2831</script>

  • trunk/LayoutTests/http/tests/globalhistory/history-delegate-basic-title-expected.txt

    r210833 r271514  
    11WebView navigated to url "http://127.0.0.1:8000/globalhistory/history-delegate-basic-title.html" with title "" with HTTP equivalent method "GET".  The navigation was successful and was not a client redirect.
    2 WebView updated the title for history URL "http://127.0.0.1:8000/globalhistory/history-delegate-basic-title.html" to "Test Title 1".
    32WebView updated the title for history URL "http://127.0.0.1:8000/globalhistory/history-delegate-basic-title.html" to "Test Title 2".
    43This test sees if the history delegate is notified of title changes.

  • trunk/LayoutTests/http/tests/globalhistory/history-delegate-basic-title.html

    r120167 r271514  
    22<head>
    33<script>
    4 if (window.testRunner)
     4if (window.testRunner) {
     5    testRunner.waitUntilDone();
    56    testRunner.dumpAsText();
     7}
    68</script>
    79<title>Test Title 1</title>
    810</head>
    9 <body>
     11<body onload="runTest()">
    1012This test sees if the history delegate is notified of title changes.
     13<script>
     14function runTest() {
     15document.title = "Test Title 2";
     16internals.queueTask("DOMManipulation", () => testRunner.notifyDone());
     17}
     18</script>
    1119</body>
    12 <script>
    13 document.title = "Test Title 2";
    14 </script>
    1520</html>

  • trunk/LayoutTests/http/tests/loading/basic-auth-load-URL-with-consecutive-slashes-expected.txt

    r267644 r271514  
    66frame "<!--frame1-->" - didCommitLoadForFrame
    77frame "<!--frame1-->" - didFinishDocumentLoadForFrame
     8frame "<!--frame1-->" - didHandleOnloadEventsForFrame
    89frame "<!--frame1-->" - willPerformClientRedirectToURL: http://127.0.0.1:8000/a//b/non-existent-file.html
    9 frame "<!--frame1-->" - didHandleOnloadEventsForFrame
    1010main frame - didHandleOnloadEventsForFrame
    1111frame "<!--frame1-->" - didFinishLoadForFrame
    … …  
    1616frame "<!--frame1-->" - didReceiveTitle: 404 Not Found
    1717frame "<!--frame1-->" - didFinishDocumentLoadForFrame
    18 frame "<!--frame1-->" - didFailLoadWithError
     18frame "<!--frame1-->" - didHandleOnloadEventsForFrame
     19frame "<!--frame1-->" - didFinishLoadForFrame
    1920PASS did not cause assertion failure.

  • trunk/LayoutTests/http/tests/loading/basic-auth-load-URL-with-consecutive-slashes.html

    r195590 r271514  
    1313function done()
    1414{
    15     document.body.removeChild(document.getElementById("frame"));
    16     if (window.testRunner)
    17         testRunner.notifyDone();
     15    setTimeout(function() {
     16        document.body.removeChild(document.getElementById("frame"));
     17        if (window.testRunner)
     18            testRunner.notifyDone();
     19    }, 500);
    1820}
    1921
    … …  
    2527</script>
    2628</head>
    27 <body>
    28 <iframe id="frame" src="resources/basic-auth-testing.php?username=webkit&password=rocks" onload="notifyFrameDidLoad(this)"></iframe>
     29<body onload="notifyFrameDidLoad(frame)">
     30<iframe id="frame" src="resources/basic-auth-testing.php?username=webkit&password=rocks" onlad="notifyFrameDidLoad(this)"></iframe>
    2931<p>PASS did not cause assertion failure.</p>
    3032</body>

  • trunk/LayoutTests/http/tests/loading/redirect-with-no-location-crash-expected.txt

    r231450 r271514  
    11main frame - didStartProvisionalLoadForFrame
    22main frame - didCommitLoadForFrame
    3 main frame - didReceiveTitle: Test for https://bugs.webkit.org/show_bug.cgi?id=29293
    43frame "<!--frame1-->" - didStartProvisionalLoadForFrame
    54main frame - didFinishDocumentLoadForFrame
     5main frame - didReceiveTitle: Test for https://bugs.webkit.org/show_bug.cgi?id=29293
    66frame "<!--frame1-->" - didCommitLoadForFrame
    77frame "<!--frame1-->" - didFinishDocumentLoadForFrame

  • trunk/LayoutTests/http/tests/loading/redirect-with-no-location-crash.html

    r120167 r271514  
    33    <title>Test for https://bugs.webkit.org/show_bug.cgi?id=29293</title>
    44    <script>
    5         if (window.testRunner)
     5        if (window.testRunner) {
    66            testRunner.dumpAsText();
     7            testRunner.waitUntilDone();
     8        }
    79    </script>
    810</head>
    9 <body>
     11<body onload="setTimeout(function() { if (window.testRunner) testRunner.notifyDone(); })">
    1012  <iframe src="resources/redirect-with-no-location-crash.php"></iframe>
    1113</body>

  • trunk/LayoutTests/platform/mac-wk2/TestExpectations

    r271454 r271514  
    584584webkit.org/b/163136 http/tests/xmlhttprequest/auth-reject-protection-space.html [ Pass Failure ]
    585585
    586 webkit.org/b/163139 http/tests/loading/basic-auth-load-URL-with-consecutive-slashes.html [ Pass Failure ]
     586#webkit.org/b/163139 http/tests/loading/basic-auth-load-URL-with-consecutive-slashes.html [ Pass Failure ]
    587587
    588588webkit.org/b/162975 http/tests/cache/disk-cache/memory-cache-revalidation-updates-disk-cache.html [ Pass Failure ]

  • trunk/LayoutTests/platform/win/http/tests/loading/basic-auth-load-URL-with-consecutive-slashes-expected.txt

    r271513 r271514  
    66frame "<!--frame1-->" - didCommitLoadForFrame
    77frame "<!--frame1-->" - didFinishDocumentLoadForFrame
     8frame "<!--frame1-->" - didHandleOnloadEventsForFrame
    89frame "<!--frame1-->" - willPerformClientRedirectToURL: http://127.0.0.1:8000/a//b/non-existent-file.html
    9 frame "<!--frame1-->" - didHandleOnloadEventsForFrame
    1010main frame - didHandleOnloadEventsForFrame
    1111frame "<!--frame1-->" - didFinishLoadForFrame
    … …  
    1414frame "<!--frame1-->" - didCancelClientRedirectForFrame
    1515frame "<!--frame1-->" - didCommitLoadForFrame
     16frame "<!--frame1-->" - didFinishDocumentLoadForFrame
     17frame "<!--frame1-->" - didHandleOnloadEventsForFrame
     18frame "<!--frame1-->" - didFinishLoadForFrame
    1619frame "<!--frame1-->" - didReceiveTitle: 404 Not Found
    17 frame "<!--frame1-->" - didFinishDocumentLoadForFrame
    18 frame "<!--frame1-->" - didFailLoadWithError
    1920PASS did not cause assertion failure.

  • trunk/LayoutTests/platform/wk2/http/tests/loading/basic-auth-load-URL-with-consecutive-slashes-expected.txt

    r267644 r271514  
    66frame "<!--frame1-->" - didCommitLoadForFrame
    77frame "<!--frame1-->" - didFinishDocumentLoadForFrame
     8frame "<!--frame1-->" - didHandleOnloadEventsForFrame
    89frame "<!--frame1-->" - willPerformClientRedirectToURL: http://127.0.0.1:8000/a//b/non-existent-file.html
    9 frame "<!--frame1-->" - didHandleOnloadEventsForFrame
    1010main frame - didHandleOnloadEventsForFrame
    1111frame "<!--frame1-->" - didFinishLoadForFrame
    … …  
    1414frame "<!--frame1-->" - didCancelClientRedirectForFrame
    1515frame "<!--frame1-->" - didCommitLoadForFrame
     16frame "<!--frame1-->" - didFinishDocumentLoadForFrame
     17frame "<!--frame1-->" - didHandleOnloadEventsForFrame
     18frame "<!--frame1-->" - didFinishLoadForFrame
    1619frame "<!--frame1-->" - didReceiveTitle: 404 Not Found
    17 frame "<!--frame1-->" - didFinishDocumentLoadForFrame
    18 frame "<!--frame1-->" - didFailLoadWithError
    1920PASS did not cause assertion failure.

  • trunk/LayoutTests/platform/wk2/http/tests/loading/redirect-with-no-location-crash-expected.txt

    r231450 r271514  
    11main frame - didStartProvisionalLoadForFrame
    22main frame - didCommitLoadForFrame
     3main frame - didFinishDocumentLoadForFrame
    34main frame - didReceiveTitle: Test for https://bugs.webkit.org/show_bug.cgi?id=29293
    4 main frame - didFinishDocumentLoadForFrame
    55frame "<!--frame1-->" - didStartProvisionalLoadForFrame
    66frame "<!--frame1-->" - didCommitLoadForFrame

  • trunk/Source/WebCore/ChangeLog

    r271513 r271514  
     12021-01-15  Rob Buis  <rbuis@igalia.com>
     2
     3        Use event loop to set title
     4        https://bugs.webkit.org/show_bug.cgi?id=218496
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Use event loop to set title to avoid calling WebFrameLoaderClient
     9        within HTMLTitleElement::insertedIntoAncestor.
     10
     11        * dom/Document.cpp:
     12        (WebCore::Document::updateTitle):
     13        * dom/Document.h:
     14
    1152021-01-15  Philippe Normand  <pnormand@igalia.com>
    216

  • trunk/Source/WebCore/dom/Document.cpp

    r271440 r271514  
    16791679    m_title.direction = title.direction;
    16801680
    1681     if (auto* loader = this->loader())
    1682         loader->setTitle(m_title);
     1681    if (!m_updateTitleTaskScheduled) {
     1682        eventLoop().queueTask(TaskSource::DOMManipulation, [protectedThis = makeRef(*this), this]() mutable {
     1683            m_updateTitleTaskScheduled = false;
     1684            if (auto documentLoader = makeRefPtr(loader()))
     1685                documentLoader->setTitle(m_title);
     1686        });
     1687        m_updateTitleTaskScheduled = true;
     1688    }
    16831689}
    16841690

  • trunk/Source/WebCore/dom/Document.h

    r271418 r271514  
    21152115#endif
    21162116
     2117    bool m_updateTitleTaskScheduled { false };
     2118
    21172119    OrientationNotifier m_orientationNotifier;
    21182120    mutable RefPtr<Logger> m_logger;

  • trunk/Tools/ChangeLog

    r271512 r271514  
     12021-01-15  Rob Buis  <rbuis@igalia.com>
     2
     3        Use event loop to set title
     4        https://bugs.webkit.org/show_bug.cgi?id=218496
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Adapt unit tests to wait for title change tasks
     9        to be processed.
     10
     11        * TestWebKitAPI/Tests/WebKit/PageLoadState.cpp:
     12        (TestWebKitAPI::didChangeTitle):
     13        (TestWebKitAPI::TEST):
     14        * TestWebKitAPI/Tests/WebKitCocoa/UIDelegate.mm:
     15        (TEST):
     16
    1172021-01-15  Philippe Normand  <pnormand@igalia.com>
    218

  • trunk/Tools/TestWebKitAPI/Tests/WebKit/PageLoadState.cpp

    r248336 r271514  
    3636
    3737static bool test1Done;
     38static bool titleChangeDone;
    3839
    3940struct PageLoadTestState {
    … …  
    113114    PageLoadTestState* state = reinterpret_cast<PageLoadTestState*>(const_cast<void*>(clientInfo));
    114115    state->didChangeTitle++;
     116    titleChangeDone = true;
    115117}
    116118
    … …  
    279281    EXPECT_EQ(state.willChangeCanGoForward, 1);
    280282
    281     test1Done = false;
     283    titleChangeDone = false;
    282284    url = adoptWK(Util::createURLForResource("set-long-title", "html"));
    283285    WKPageLoadURL(webView.page(), url.get());
    284     Util::run(&test1Done);
     286    Util::run(&titleChangeDone);
    285287
    286288    EXPECT_EQ(state.didChangeActiveURL, 4);
    287     EXPECT_EQ(state.didChangeTitle, 2);
    288     EXPECT_EQ(state.willChangeTitle, 2);
     289    EXPECT_EQ(state.didChangeTitle, 1);
     290    EXPECT_EQ(state.willChangeTitle, 1);
    289291
    290292    WKPageSetPageStateClient(webView.page(), nullptr);

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/UIDelegate.mm

    r263196 r271514  
    413413    auto delegate = adoptNS([[PrintDelegate alloc] init]);
    414414    [webView setUIDelegate:delegate.get()];
    415     [webView loadHTMLString:@"<head><title>test_title</title></head><body onload='print()'>hello world!</body>" baseURL:[NSURL URLWithString:@"http://example.com/"]];
     415    [webView loadHTMLString:@"<head><title>test_title</title></head><body onload='setTimeout(function() { print() });'>hello world!</body>" baseURL:[NSURL URLWithString:@"http://example.com/"]];
    416416    TestWebKitAPI::Util::run(&done);
    417417
Note: See TracChangeset for help on using the changeset viewer.