Modified: trunk/Source/WebCore/ChangeLog (166263 => 166264)
--- trunk/Source/WebCore/ChangeLog 2014-03-25 22:38:52 UTC (rev 166263)
+++ trunk/Source/WebCore/ChangeLog 2014-03-25 22:56:58 UTC (rev 166264)
@@ -1,3 +1,26 @@
+2014-03-25 Brian Burg <bb...@apple.com>
+
+ 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:
+
2014-03-25 Jer Noble <jer.no...@apple.com>
[Mac] Pause the media element during system sleep.
Modified: trunk/Source/WebCore/loader/ProgressTracker.cpp (166263 => 166264)
--- trunk/Source/WebCore/loader/ProgressTracker.cpp 2014-03-25 22:38:52 UTC (rev 166263)
+++ trunk/Source/WebCore/loader/ProgressTracker.cpp 2014-03-25 22:56:58 UTC (rev 166264)
@@ -73,8 +73,6 @@
long long estimatedLength;
};
-unsigned long ProgressTracker::s_uniqueIdentifier = 0;
-
ProgressTracker::ProgressTracker(ProgressTrackerClient& client)
: m_client(client)
, m_totalPageAndResourceBytesToLoad(0)
@@ -84,6 +82,7 @@
, m_progressNotificationTimeInterval(std::chrono::milliseconds(100))
, m_finalProgressChangedSent(false)
, m_progressValue(0)
+ , m_nextUniqueIdentifier(1)
, m_numProgressTrackedFrames(0)
, m_progressHeartbeatTimer(this, &ProgressTracker::progressHeartbeatTimerFired)
, m_heartbeatsWithNoProgress(0)
@@ -114,6 +113,7 @@
m_finalProgressChangedSent = false;
m_numProgressTrackedFrames = 0;
m_originatingProgressFrame = 0;
+ // Don't reset m_nextUniqueIdentifier. More loads could start after reset() is called.
m_heartbeatsWithNoProgress = 0;
m_totalBytesReceivedBeforePreviousHeartbeat = 0;
@@ -294,7 +294,7 @@
unsigned long ProgressTracker::createUniqueIdentifier()
{
- return ++s_uniqueIdentifier;
+ return m_nextUniqueIdentifier++;
}
bool ProgressTracker::isMainLoadProgressing() const
Modified: trunk/Source/WebCore/loader/ProgressTracker.h (166263 => 166264)
--- trunk/Source/WebCore/loader/ProgressTracker.h 2014-03-25 22:38:52 UTC (rev 166263)
+++ trunk/Source/WebCore/loader/ProgressTracker.h 2014-03-25 22:56:58 UTC (rev 166264)
@@ -46,7 +46,7 @@
explicit ProgressTracker(ProgressTrackerClient&);
~ProgressTracker();
- static unsigned long createUniqueIdentifier();
+ unsigned long createUniqueIdentifier();
double estimatedProgress() const;
@@ -68,8 +68,6 @@
void progressHeartbeatTimerFired(Timer<ProgressTracker>&);
- static unsigned long s_uniqueIdentifier;
-
ProgressTrackerClient& m_client;
long long m_totalPageAndResourceBytesToLoad;
long long m_totalBytesReceived;
@@ -80,6 +78,7 @@
bool m_finalProgressChangedSent;
double m_progressValue;
RefPtr<Frame> m_originatingProgressFrame;
+ unsigned long m_nextUniqueIdentifier;
int m_numProgressTrackedFrames;
HashMap<unsigned long, std::unique_ptr<ProgressItem>> m_progressItems;
Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp (166263 => 166264)
--- trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp 2014-03-25 22:38:52 UTC (rev 166263)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp 2014-03-25 22:56:58 UTC (rev 166264)
@@ -254,12 +254,13 @@
return toWTFString(adoptWK(WKURLCopyLastPathComponent(fileUrl))); // We lose some information here, but it's better than exposing a full path, which is always machine specific.
}
-static HashMap<uint64_t, String> assignedUrlsCache;
+static HashMap<std::pair<WKBundleFrameRef, uint64_t>, String> assignedUrlsCache;
-static inline void dumpResourceURL(uint64_t identifier, StringBuilder& stringBuilder)
+static inline void dumpResourceURL(WKBundleFrameRef frame, uint64_t identifier, StringBuilder& stringBuilder)
{
- if (assignedUrlsCache.contains(identifier))
- stringBuilder.append(assignedUrlsCache.get(identifier));
+ std::pair<WKBundleFrameRef, uint64_t> key = std::make_pair(frame, identifier);
+ if (assignedUrlsCache.contains(key))
+ stringBuilder.append(assignedUrlsCache.get(key));
else
stringBuilder.appendLiteral("<unknown>");
}
@@ -1027,13 +1028,15 @@
InjectedBundle::shared().outputText("didDetectXSS\n");
}
-void InjectedBundlePage::didInitiateLoadForResource(WKBundlePageRef page, WKBundleFrameRef, uint64_t identifier, WKURLRequestRef request, bool)
+void InjectedBundlePage::didInitiateLoadForResource(WKBundlePageRef page, WKBundleFrameRef frame, uint64_t identifier, WKURLRequestRef request, bool)
{
if (!InjectedBundle::shared().isTestRunning())
return;
WKRetainPtr<WKURLRef> url = ""
- assignedUrlsCache.add(identifier, pathSuitableForTestResult(url.get()));
+ auto result = assignedUrlsCache.add(std::make_pair(frame, identifier), pathSuitableForTestResult(url.get()));
+ // It is a bug in WebCore if multiple resources had the same frame/identifier pair.
+ ASSERT_UNUSED(result, result.isNewEntry);
}
// Resource Load Client Callbacks
@@ -1053,7 +1056,7 @@
if (InjectedBundle::shared().isTestRunning()
&& InjectedBundle::shared().testRunner()->shouldDumpResourceLoadCallbacks()) {
StringBuilder stringBuilder;
- dumpResourceURL(identifier, stringBuilder);
+ dumpResourceURL(frame, identifier, stringBuilder);
stringBuilder.appendLiteral(" - willSendRequest ");
dumpRequestDescriptionSuitableForTestResult(request, stringBuilder);
stringBuilder.appendLiteral(" redirectResponse ");
@@ -1104,14 +1107,14 @@
return request;
}
-void InjectedBundlePage::didReceiveResponseForResource(WKBundlePageRef page, WKBundleFrameRef, uint64_t identifier, WKURLResponseRef response)
+void InjectedBundlePage::didReceiveResponseForResource(WKBundlePageRef page, WKBundleFrameRef frame, uint64_t identifier, WKURLResponseRef response)
{
if (!InjectedBundle::shared().isTestRunning())
return;
if (InjectedBundle::shared().testRunner()->shouldDumpResourceLoadCallbacks()) {
StringBuilder stringBuilder;
- dumpResourceURL(identifier, stringBuilder);
+ dumpResourceURL(frame, identifier, stringBuilder);
stringBuilder.appendLiteral(" - didReceiveResponse ");
dumpResponseDescriptionSuitableForTestResult(response, stringBuilder);
stringBuilder.append('\n');
@@ -1138,7 +1141,7 @@
{
}
-void InjectedBundlePage::didFinishLoadForResource(WKBundlePageRef, WKBundleFrameRef, uint64_t identifier)
+void InjectedBundlePage::didFinishLoadForResource(WKBundlePageRef, WKBundleFrameRef frame, uint64_t identifier)
{
if (!InjectedBundle::shared().isTestRunning())
return;
@@ -1147,12 +1150,12 @@
return;
StringBuilder stringBuilder;
- dumpResourceURL(identifier, stringBuilder);
+ dumpResourceURL(frame, identifier, stringBuilder);
stringBuilder.appendLiteral(" - didFinishLoading\n");
InjectedBundle::shared().outputText(stringBuilder.toString());
}
-void InjectedBundlePage::didFailLoadForResource(WKBundlePageRef, WKBundleFrameRef, uint64_t identifier, WKErrorRef error)
+void InjectedBundlePage::didFailLoadForResource(WKBundlePageRef, WKBundleFrameRef frame, uint64_t identifier, WKErrorRef error)
{
if (!InjectedBundle::shared().isTestRunning())
return;
@@ -1161,7 +1164,7 @@
return;
StringBuilder stringBuilder;
- dumpResourceURL(identifier, stringBuilder);
+ dumpResourceURL(frame, identifier, stringBuilder);
stringBuilder.appendLiteral(" - didFailLoadingWithError: ");
dumpErrorDescriptionSuitableForTestResult(error, stringBuilder);
