Changeset 166264 in webkit


Ignore:
Timestamp:
Mar 25, 2014 3:56:58 PM (10 years ago)
Author:
BJ Burg
Message:

Source/WebCore: Web Replay: resource unique identifiers should be unique-per-frame, not globally
​https://bugs.webkit.org/show_bug.cgi?id=130632

Reviewed by Timothy Hatcher.

For replay purposes, we want to deterministically assign resource load identifiers
to resource loaders, provided that the resource loaders are created in the same
order.

To do this, we convert unique identifiers from being globally-unique to being
frame-unique. When a new frame is being loaded, unique identifiers for
subresources of that frame begin counting from 1.

No new tests. Identifier invariants are exercised by existing assertions and tests.

  • loader/ProgressTracker.cpp:

(WebCore::ProgressTracker::ProgressTracker):
(WebCore::ProgressTracker::reset):
(WebCore::ProgressTracker::createUniqueIdentifier):

  • loader/ProgressTracker.h:

Tools: Web Replay: resource unique identifiers should be unique-per-frame, not globally
​https://bugs.webkit.org/show_bug.cgi?id=130623

Reviewed by Timothy Hatcher.

The resource loader callback dumping routines assumed that resource identifiers
were globally unique. Its map of resource identifiers to URLs must also track the
frame associated with the resource.

  • WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:

(WTR::dumpResourceURL): Additionally take a WKBundleFrameRef argument, and use the
opaque pointer as part of the key for assignedUrlsCache. The frame pointer is
stable as long as the frame is valid.
(WTR::InjectedBundlePage::didInitiateLoadForResource):
(WTR::InjectedBundlePage::willSendRequestForFrame):
(WTR::InjectedBundlePage::didReceiveResponseForResource):
(WTR::InjectedBundlePage::didFinishLoadForResource):
(WTR::InjectedBundlePage::didFailLoadForResource):

Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r166261 r166264  
     12014-03-25  Brian Burg  <bburg@apple.com>
     2
     3        Web Replay: resource unique identifiers should be unique-per-frame, not globally
     4        https://bugs.webkit.org/show_bug.cgi?id=130632
     5
     6        Reviewed by Timothy Hatcher.
     7
     8        For replay purposes, we want to deterministically assign resource load identifiers
     9        to resource loaders, provided that the resource loaders are created in the same
     10        order.
     11
     12        To do this, we convert unique identifiers from being globally-unique to being
     13        frame-unique. When a new frame is being loaded, unique identifiers for
     14        subresources of that frame begin counting from 1.
     15
     16        No new tests. Identifier invariants are exercised by existing assertions and tests.
     17
     18        * loader/ProgressTracker.cpp:
     19        (WebCore::ProgressTracker::ProgressTracker):
     20        (WebCore::ProgressTracker::reset):
     21        (WebCore::ProgressTracker::createUniqueIdentifier):
     22        * loader/ProgressTracker.h:
     23
    1242014-03-25  Jer Noble  <jer.noble@apple.com>
    225

  • trunk/Source/WebCore/loader/ProgressTracker.cpp

    r165676 r166264  
    7474};
    7575
    76 unsigned long ProgressTracker::s_uniqueIdentifier = 0;
    77 
    7876ProgressTracker::ProgressTracker(ProgressTrackerClient& client)
    7977    : m_client(client)
    … …  
    8583    , m_finalProgressChangedSent(false)
    8684    , m_progressValue(0)
     85    , m_nextUniqueIdentifier(1)
    8786    , m_numProgressTrackedFrames(0)
    8887    , m_progressHeartbeatTimer(this, &ProgressTracker::progressHeartbeatTimerFired)
    … …  
    115114    m_numProgressTrackedFrames = 0;
    116115    m_originatingProgressFrame = 0;
     116    // Don't reset m_nextUniqueIdentifier. More loads could start after reset() is called.
    117117
    118118    m_heartbeatsWithNoProgress = 0;
    … …  
    295295unsigned long ProgressTracker::createUniqueIdentifier()
    296296{
    297     return ++s_uniqueIdentifier;
     297    return m_nextUniqueIdentifier++;
    298298}
    299299

  • trunk/Source/WebCore/loader/ProgressTracker.h

    r165676 r166264  
    4747    ~ProgressTracker();
    4848
    49     static unsigned long createUniqueIdentifier();
     49    unsigned long createUniqueIdentifier();
    5050
    5151    double estimatedProgress() const;
    … …  
    6969    void progressHeartbeatTimerFired(Timer<ProgressTracker>&);
    7070   
    71     static unsigned long s_uniqueIdentifier;
    72    
    7371    ProgressTrackerClient& m_client;
    7472    long long m_totalPageAndResourceBytesToLoad;
    … …  
    8179    double m_progressValue;
    8280    RefPtr<Frame> m_originatingProgressFrame;
     81    unsigned long m_nextUniqueIdentifier;
    8382   
    8483    int m_numProgressTrackedFrames;

  • trunk/Tools/ChangeLog

    r166260 r166264  
     12014-03-25  Brian Burg  <bburg@apple.com>
     2
     3        Web Replay: resource unique identifiers should be unique-per-frame, not globally
     4        https://bugs.webkit.org/show_bug.cgi?id=130623
     5
     6        Reviewed by Timothy Hatcher.
     7
     8        The resource loader callback dumping routines assumed that resource identifiers
     9        were globally unique. Its map of resource identifiers to URLs must also track the
     10        frame associated with the resource.
     11
     12        * WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:
     13        (WTR::dumpResourceURL): Additionally take a WKBundleFrameRef argument, and use the
     14        opaque pointer as part of the key for assignedUrlsCache. The frame pointer is
     15        stable as long as the frame is valid.
     16        (WTR::InjectedBundlePage::didInitiateLoadForResource):
     17        (WTR::InjectedBundlePage::willSendRequestForFrame):
     18        (WTR::InjectedBundlePage::didReceiveResponseForResource):
     19        (WTR::InjectedBundlePage::didFinishLoadForResource):
     20        (WTR::InjectedBundlePage::didFailLoadForResource):
     21
    1222014-03-25  Andy Estes  <aestes@apple.com>
    223

  • trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp

    r164295 r166264  
    255255}
    256256
    257 static HashMap<uint64_t, String> assignedUrlsCache;
    258 
    259 static inline void dumpResourceURL(uint64_t identifier, StringBuilder& stringBuilder)
    260 {
    261     if (assignedUrlsCache.contains(identifier))
    262         stringBuilder.append(assignedUrlsCache.get(identifier));
     257static HashMap<std::pair<WKBundleFrameRef, uint64_t>, String> assignedUrlsCache;
     258
     259static inline void dumpResourceURL(WKBundleFrameRef frame, uint64_t identifier, StringBuilder& stringBuilder)
     260{
     261    std::pair<WKBundleFrameRef, uint64_t> key = std::make_pair(frame, identifier);
     262    if (assignedUrlsCache.contains(key))
     263        stringBuilder.append(assignedUrlsCache.get(key));
    263264    else
    264265        stringBuilder.appendLiteral("<unknown>");
    … …  
    10281029}
    10291030
    1030 void InjectedBundlePage::didInitiateLoadForResource(WKBundlePageRef page, WKBundleFrameRef, uint64_t identifier, WKURLRequestRef request, bool)
     1031void InjectedBundlePage::didInitiateLoadForResource(WKBundlePageRef page, WKBundleFrameRef frame, uint64_t identifier, WKURLRequestRef request, bool)
    10311032{
    10321033    if (!InjectedBundle::shared().isTestRunning())
    … …  
    10341035
    10351036    WKRetainPtr<WKURLRef> url = adoptWK(WKURLRequestCopyURL(request));
    1036     assignedUrlsCache.add(identifier, pathSuitableForTestResult(url.get()));
     1037    auto result = assignedUrlsCache.add(std::make_pair(frame, identifier), pathSuitableForTestResult(url.get()));
     1038    // It is a bug in WebCore if multiple resources had the same frame/identifier pair.
     1039    ASSERT_UNUSED(result, result.isNewEntry);
    10371040}
    10381041
    … …  
    10541057        && InjectedBundle::shared().testRunner()->shouldDumpResourceLoadCallbacks()) {
    10551058        StringBuilder stringBuilder;
    1056         dumpResourceURL(identifier, stringBuilder);
     1059        dumpResourceURL(frame, identifier, stringBuilder);
    10571060        stringBuilder.appendLiteral(" - willSendRequest ");
    10581061        dumpRequestDescriptionSuitableForTestResult(request, stringBuilder);
    … …  
    11051108}
    11061109
    1107 void InjectedBundlePage::didReceiveResponseForResource(WKBundlePageRef page, WKBundleFrameRef, uint64_t identifier, WKURLResponseRef response)
     1110void InjectedBundlePage::didReceiveResponseForResource(WKBundlePageRef page, WKBundleFrameRef frame, uint64_t identifier, WKURLResponseRef response)
    11081111{
    11091112    if (!InjectedBundle::shared().isTestRunning())
    … …  
    11121115    if (InjectedBundle::shared().testRunner()->shouldDumpResourceLoadCallbacks()) {
    11131116        StringBuilder stringBuilder;
    1114         dumpResourceURL(identifier, stringBuilder);
     1117        dumpResourceURL(frame, identifier, stringBuilder);
    11151118        stringBuilder.appendLiteral(" - didReceiveResponse ");
    11161119        dumpResponseDescriptionSuitableForTestResult(response, stringBuilder);
    … …  
    11391142}
    11401143
    1141 void InjectedBundlePage::didFinishLoadForResource(WKBundlePageRef, WKBundleFrameRef, uint64_t identifier)
     1144void InjectedBundlePage::didFinishLoadForResource(WKBundlePageRef, WKBundleFrameRef frame, uint64_t identifier)
    11421145{
    11431146    if (!InjectedBundle::shared().isTestRunning())
    … …  
    11481151
    11491152    StringBuilder stringBuilder;
    1150     dumpResourceURL(identifier, stringBuilder);
     1153    dumpResourceURL(frame, identifier, stringBuilder);
    11511154    stringBuilder.appendLiteral(" - didFinishLoading\n");
    11521155    InjectedBundle::shared().outputText(stringBuilder.toString());
    11531156}
    11541157
    1155 void InjectedBundlePage::didFailLoadForResource(WKBundlePageRef, WKBundleFrameRef, uint64_t identifier, WKErrorRef error)
     1158void InjectedBundlePage::didFailLoadForResource(WKBundlePageRef, WKBundleFrameRef frame, uint64_t identifier, WKErrorRef error)
    11561159{
    11571160    if (!InjectedBundle::shared().isTestRunning())
    … …  
    11621165
    11631166    StringBuilder stringBuilder;
    1164     dumpResourceURL(identifier, stringBuilder);
     1167    dumpResourceURL(frame, identifier, stringBuilder);
    11651168    stringBuilder.appendLiteral(" - didFailLoadingWithError: ");
    11661169
Note: See TracChangeset for help on using the changeset viewer.