Bug 1599160 - Stop synthesizing performance timing entries for cache sheets. r=mayhemer

This basically undoes D77842, but it was better done on top than just
removing the patch from the stack. I could squash them if desired.

The previous patch to respect caching headers makes tests much much more
happy, to the point where I'm not sure whether we really need this or
not. Your call whether we should keep it or not.

Differential Revision: https://phabricator.services.mozilla.com/D78660
This commit is contained in:
Emilio Cobos Álvarez 2020-06-11 11:42:08 +00:00
parent 5048e0ec84
commit d1db9f246c
7 changed files with 15 additions and 78 deletions

View File

@ -205,14 +205,6 @@ PerformanceTimingData::PerformanceTimingData(nsITimedChannel* aChannel,
}
}
void PerformanceTimingData::InitializeForMemoryCacheHit() {
auto now = TimeStamp::Now();
mInitialized = true;
mAsyncOpen = mRequestStart = mResponseStart = mResponseEnd =
mDomainLookupStart = mDomainLookupEnd = mConnectStart = mConnectEnd =
mCacheReadStart = mCacheReadEnd = now;
}
void PerformanceTimingData::SetPropertiesFromHttpChannel(
nsIHttpChannel* aHttpChannel, nsITimedChannel* aChannel) {
MOZ_ASSERT(aHttpChannel);

View File

@ -112,18 +112,6 @@ class PerformanceTimingData final {
return duration.ToMilliseconds() + mZeroTime;
}
/**
* Initializes the timestamps to represent a synchronous cache hit in an
* in-memory cache.
*
* This is used right now for the shared stylesheet cache, which doesn't
* always go through necko and thus there isn't an HTTP channel, etc.
*
* We fill the values with the current timestamp to represent an "instant"
* resource fetch.
*/
void InitializeForMemoryCacheHit();
// The last channel's AsyncOpen time. This may occur before the FetchStart
// in some cases.
DOMHighResTimeStamp AsyncOpenHighRes(Performance* aPerformance);

View File

@ -6,7 +6,7 @@
<!DOCTYPE html>
<html>
<head>
<link rel="stylesheet" type="text/css" href="http://mochi.test:8888/tests/SimpleTest/test.css"/>
<link rel="stylesheet" type="text/css" href="http://mochi.test:8888/tests/SimpleTest/test.css?resource-timing-nocors"/>
<!--
This document has the origin http://mochi.test:8888.
@ -109,7 +109,7 @@ function isnot(received, notExpected, message) {
}
var allResources = {
"http://mochi.test:8888/tests/SimpleTest/test.css" : "link",
"http://mochi.test:8888/tests/SimpleTest/test.css?resource-timing-nocors" : "link",
// 1
"http://mochi.test:8888/tests/dom/tests/mochitest/general/generateCss.sjs?A" : "link",

View File

@ -6,7 +6,7 @@
<!DOCTYPE html>
<html>
<head>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
<link rel="stylesheet" href="/tests/SimpleTest/test.css?performance-timeline-main-test"/>
<script type="application/javascript">
function ok(cond, message) {

View File

@ -45,7 +45,7 @@
<!DOCTYPE html>
<html>
<head>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
<link rel="stylesheet" href="/tests/SimpleTest/test.css?resource-timing-main-test"/>
<script type="application/javascript">
var mainWindowTestsDone = false;
@ -67,7 +67,7 @@ var bufferFullCounter = 0;
const expectedBufferFullEvents = 1;
var allResources = {
"http://mochi.test:8888/tests/SimpleTest/test.css": "link",
"http://mochi.test:8888/tests/SimpleTest/test.css?resource-timing-main-test": "link",
"http://mochi.test:8888/tests/image/test/mochitest/blue.png" : "img",
"http://mochi.test:8888/tests/image/test/mochitest/red.png" : "object",
"http://mochi.test:8888/tests/image/test/mochitest/big.png" : "embed",

View File

@ -29,8 +29,6 @@
#include "nsIContent.h"
#include "nsIContentInlines.h"
#include "mozilla/dom/Document.h"
#include "mozilla/dom/PerformanceTiming.h"
#include "mozilla/dom/PerformanceMainThread.h"
#include "nsIURI.h"
#include "nsNetUtil.h"
#include "nsContentUtils.h"
@ -824,48 +822,10 @@ nsresult Loader::CheckContentPolicy(nsIPrincipal* aLoadingPrincipal,
return NS_OK;
}
void Loader::MaybeNotifyOfResourceTiming(SheetLoadData& aData) {
SheetLoadDataHashKey key(aData);
if (!mLoadsPerformed.EnsureInserted(key)) {
// We've already reported this subresource load.
return;
}
if (!mDocument) {
// No document means no performance object to report to.
return;
}
nsPIDOMWindowInner* win = mDocument->GetInnerWindow();
if (!win) {
return;
}
auto* perf = static_cast<dom::PerformanceMainThread*>(win->GetPerformance());
if (!perf) {
return;
}
// If any ancestor is cross-origin, then we don't report it, see
// mIsCrossOriginNoCORS and so.
for (auto* parent = aData.mSheet->GetParentSheet(); parent;
parent = parent->GetParentSheet()) {
if (parent->GetCORSMode() == CORS_NONE &&
!aData.mTriggeringPrincipal->Subsumes(parent->Principal())) {
return;
}
}
nsLiteralString initiatorType = aData.mSheet->GetParentSheet()
? NS_LITERAL_STRING("css")
: NS_LITERAL_STRING("link");
nsAutoCString entryName;
aData.mSheet->GetOriginalURI()->GetSpec(entryName);
auto data = MakeUnique<dom::PerformanceTimingData>(nullptr, nullptr, 0);
data->InitializeForMemoryCacheHit();
perf->AddRawEntry(std::move(data), initiatorType,
NS_ConvertUTF8toUTF16(entryName));
void Loader::DidHitCompleteSheetCache(SheetLoadData& aData) {
mLoadsPerformed.PutEntry(SheetLoadDataHashKey(aData));
}
/**
* CreateSheet() creates a StyleSheet object for the given URI.
*
@ -1345,9 +1305,6 @@ nsresult Loader::LoadSheet(SheetLoadData& aLoadData, SheetState aSheetState) {
// mBlockResourceTiming. If that is set then we too are such a
// sub-resource and so we set the flag on ourself too to propagate it
// on down.
//
// If you add more conditions here make sure to add them to
// MaybeNotifyResourceTiming too.
if (aLoadData.mParentData->mIsCrossOriginNoCORS ||
aLoadData.mParentData->mBlockResourceTiming) {
// Set a flag so any other stylesheet triggered by this one will
@ -1729,7 +1686,7 @@ Result<Loader::LoadSheetResult, nsresult> Loader::LoadStyleLink(
aInfo.mReferrerInfo, context);
if (state == SheetState::Complete) {
LOG((" Sheet already complete: 0x%p", sheet.get()));
MaybeNotifyOfResourceTiming(*data);
DidHitCompleteSheetCache(*data);
if (aObserver || !mObservers.IsEmpty() || aInfo.mContent) {
rv = PostLoadEvent(std::move(data));
if (NS_FAILED(rv)) {
@ -1885,7 +1842,7 @@ nsresult Loader::LoadChildSheet(StyleSheet& aParentSheet,
if (state == SheetState::Complete) {
LOG((" Sheet already complete"));
MaybeNotifyOfResourceTiming(*data);
DidHitCompleteSheetCache(*data);
// We're completely done. No need to notify, even, since the
// @import rule addition/modification will trigger the right style
// changes automatically.
@ -1975,7 +1932,7 @@ Result<RefPtr<StyleSheet>, nsresult> Loader::InternalLoadNonDocumentSheet(
mDocument);
if (state == SheetState::Complete) {
LOG((" Sheet already complete"));
MaybeNotifyOfResourceTiming(*data);
DidHitCompleteSheetCache(*data);
if (aObserver || !mObservers.IsEmpty()) {
rv = PostLoadEvent(std::move(data));
if (NS_FAILED(rv)) {

View File

@ -358,10 +358,10 @@ class Loader final {
nsIURI* aURL, dom::MediaList* aMedia,
LoaderReusableStyleSheets* aSavedSheets);
// If we hit the shared cache and this is the first load for a given resource,
// we still post a resource timing entry, because tests expect this, and other
// browsers behave like this too.
void MaybeNotifyOfResourceTiming(SheetLoadData&);
/**
* Called when we hit the internal memory cache with a complete stylesheet.
*/
void DidHitCompleteSheetCache(SheetLoadData&);
enum class UseSystemPrincipal { No, Yes };