Changeset 243767 in webkit


Ignore:
Timestamp:
Apr 2, 2019 4:11:43 PM (5 years ago)
Author:
achristensen@apple.com
Message:

Fix assertion in http/tests/adClickAttribution/store-ad-click-attribution.html
​https://bugs.webkit.org/show_bug.cgi?id=196503

Reviewed by Chris Dumez.

Source/WebKit:

In WebPageProxy::didDestroyNavigation we try to ignore a request to destroy a navigation from a page being navigated from
during a cross-site navigation, but if the old web process sends the message after WebPageProxy::commitProvisionalPage
has been called, we can still destroy a navigation when we are continuing a navigation in another process. To prevent this,
have the process not send the message when it knows the navigation is continuing in another process.
Also make the use of unchecked navigation pointers more robust by checking it for nullity.

  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::didCommitLoadForFrame):

  • WebProcess/WebPage/WebFrame.cpp:

(WebKit::WebFrame::didReceivePolicyDecision):
(WebKit::WebFrame::documentLoaderDetached):

  • WebProcess/WebPage/WebFrame.h:

LayoutTests:

  • http/tests/adClickAttribution/store-ad-click-attribution-expected.txt:
  • http/tests/adClickAttribution/store-ad-click-attribution.html:
  • platform/wk2/TestExpectations:
Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r243763 r243767  
     12019-04-02  Alex Christensen  <achristensen@webkit.org>
     2
     3        Fix assertion in http/tests/adClickAttribution/store-ad-click-attribution.html
     4        https://bugs.webkit.org/show_bug.cgi?id=196503
     5
     6        Reviewed by Chris Dumez.
     7
     8        * http/tests/adClickAttribution/store-ad-click-attribution-expected.txt:
     9        * http/tests/adClickAttribution/store-ad-click-attribution.html:
     10        * platform/wk2/TestExpectations:
     11
    1122019-04-02  Devin Rousso  <drousso@apple.com>
    213

  • trunk/LayoutTests/http/tests/adClickAttribution/store-ad-click-attribution-expected.txt

    r241451 r243767  
    22
    33
    4 WebCore::AdClickAttribution 1
    5 Source: 127.0.0.1
    6 Destination: localhost
    7 Campaign ID: 3
    8 No conversion data.
     4No stored Ad Click Attribution data.

  • trunk/LayoutTests/http/tests/adClickAttribution/store-ad-click-attribution.html

    r241451 r243767  
    1 <!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true, internal:AdClickAttributionEnabled=true ] -->
     1<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AdClickAttributionEnabled=true ] -->
    22<html lang="en">
    33<head>

  • trunk/LayoutTests/platform/wk2/TestExpectations

    r243762 r243767  
    754754
    755755http/tests/adClickAttribution [ Pass ]
    756 http/tests/adClickAttribution/store-ad-click-attribution.html [ Skip ]
    757756
    758757### END OF (5) Progressions, expected successes that are expected failures in WebKit1.

  • trunk/Source/WebKit/ChangeLog

    r243764 r243767  
     12019-04-02  Alex Christensen  <achristensen@webkit.org>
     2
     3        Fix assertion in http/tests/adClickAttribution/store-ad-click-attribution.html
     4        https://bugs.webkit.org/show_bug.cgi?id=196503
     5
     6        Reviewed by Chris Dumez.
     7
     8        In WebPageProxy::didDestroyNavigation we try to ignore a request to destroy a navigation from a page being navigated from
     9        during a cross-site navigation, but if the old web process sends the message after WebPageProxy::commitProvisionalPage
     10        has been called, we can still destroy a navigation when we are continuing a navigation in another process.  To prevent this,
     11        have the process not send the message when it knows the navigation is continuing in another process.
     12        Also make the use of unchecked navigation pointers more robust by checking it for nullity.
     13
     14        * UIProcess/WebPageProxy.cpp:
     15        (WebKit::WebPageProxy::didCommitLoadForFrame):
     16        * WebProcess/WebPage/WebFrame.cpp:
     17        (WebKit::WebFrame::didReceivePolicyDecision):
     18        (WebKit::WebFrame::documentLoaderDetached):
     19        * WebProcess/WebPage/WebFrame.h:
     20
    1212019-04-02  Per Arne Vollan  <pvollan@apple.com>
    222

  • trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

    r243733 r243767  
    38743874    PageClientProtector protector(pageClient());
    38753875
    3876     // On process-swap, the previous process tries to destroy the navigation but the provisional process is actually taking over the navigation.
    3877     if (m_provisionalPage && m_provisionalPage->navigationID() == navigationID)
    3878         return;
    3879 
    38803876    // FIXME: Message check the navigationID.
    38813877    m_navigationState->didDestroyNavigation(navigationID);
    … …  
    41074103    // FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
    41084104    RefPtr<API::Navigation> navigation;
    4109     if (frame->isMainFrame() && navigationID) {
    4110         navigation = navigationState().navigation(navigationID);
     4105    if (frame->isMainFrame() && navigationID && (navigation = navigationState().navigation(navigationID))) {
    41114106#if ENABLE(RESOURCE_LOAD_STATISTICS)
    41124107        auto requesterOrigin = navigation->lastNavigationAction().requesterOrigin;

  • trunk/Source/WebKit/WebProcess/WebPage/WebFrame.cpp

    r241500 r243767  
    279279    }
    280280
     281    if (action == PolicyAction::StopAllLoads)
     282        m_navigationIsContinuingInAnotherProcess = true;
     283
    281284    function(action, identifier);
    282285}
    … …  
    813816void WebFrame::documentLoaderDetached(uint64_t navigationID)
    814817{
    815     if (auto * page = this->page())
     818    if (m_navigationIsContinuingInAnotherProcess)
     819        return;
     820    if (auto* page = this->page())
    816821        page->send(Messages::WebPageProxy::DidDestroyNavigation(navigationID));
    817822}

  • trunk/Source/WebKit/WebProcess/WebPage/WebFrame.h

    r240909 r243767  
    189189   
    190190    uint64_t m_frameID { 0 };
     191    bool m_navigationIsContinuingInAnotherProcess { false };
    191192
    192193#if PLATFORM(IOS_FAMILY)
Note: See TracChangeset for help on using the changeset viewer.