Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-17 23:35:34 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1649127 - Make sure we only set AddrHostRecord::mTRRUsed = true when TRRServiceChannel::AsyncOpen succeeds r=dragana,necko-reviewers

Browse Source 
mTRRUsed is a variable that we check to gate several telemetry probes, and to
decide if TRR really failed and we should add a domain to the TRR blocklist.
The problem with setting this too early is that when this is true but we
don't actually send the TRR request, then we will report that we fell back
to Do53 and potentially skip next TRR requests in the future.

The solution here is to only set mTRRUsed if TRRServiceChannel::AsyncOpen
succeeds.

Differential Revision: https://phabricator.services.mozilla.com/D81517
This commit is contained in:
Valentin Gosu 2020-07-07 10:08:28 +00:00
parent ac4982144d
commit 9b25e934ea
3 changed files with 24 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
netwerk/dns/TRR.cpp
View File

@ -410,6 +410,13 @@ nsresult TRR::SendHTTPRequest() {
return rv;
}
// If the asyncOpen succeeded we can say that we actually attempted to
// use the TRR connection.
RefPtr<AddrHostRecord> addrRec = do_QueryObject(mRec);
if (addrRec) {
addrRec->mTRRUsed = true;
}
NS_NewTimerWithCallback(getter_AddRefs(mTimeout), this,
gTRRService->GetRequestTimeout(),
nsITimer::TYPE_ONE_SHOT);

12
netwerk/dns/nsHostResolver.cpp
View File

@ -281,10 +281,10 @@ AddrHostRecord::AddrHostRecord(const nsHostKey& key)
addr_info(nullptr),
addr(nullptr),
mFirstTRRresult(NS_OK),
mTRRUsed(false),
mTRRSuccess(0),
mNativeSuccess(0),
mNative(false),
mTRRUsed(false),
mNativeUsed(false),
onQueue(false),
usingAnyThread(false),
@ -1309,7 +1309,6 @@ nsresult nsHostResolver::TrrLookup(nsHostRecord* aRec, TRR* pushedTRR) {
if (addrRec) {
addrRec->mTRRSuccess = 0; // bump for each successful TRR response
addrRec->mTrrStart = TimeStamp::Now();
addrRec->mTRRUsed = true; // this record gets TRR treatment
addrRec->mTrrAUsed = AddrHostRecord::INIT;
addrRec->mTrrAAAAUsed = AddrHostRecord::INIT;
@ -1568,6 +1567,15 @@ nsresult nsHostResolver::NameLookup(nsHostRecord* rec) {
if (!rec->IsAddrRecord()) {
return rv;
}
#ifdef DEBUG
// If we use this branch then the mTRRUsed flag should not be set
// Even if we did call TrrLookup above, the fact that it failed sync-ly
// means that we didn't actually succeed in opening the channel.
RefPtr<AddrHostRecord> addrRec = do_QueryObject(rec);
MOZ_ASSERT(addrRec && !addrRec->mTRRUsed);
#endif
rv = NativeLookup(rec);
}

15
netwerk/dns/nsHostResolver.h
View File

@ -206,10 +206,9 @@ class AddrHostRecord final : public nsHostRecord {
size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const override;
bool IsTRR() { return mTRRUsed; }
private:
friend class nsHostResolver;
friend class mozilla::net::TRR;
explicit AddrHostRecord(const nsHostKey& key);
~AddrHostRecord();
@ -240,13 +239,13 @@ class AddrHostRecord final : public nsHostRecord {
RefPtr<mozilla::net::AddrInfo> mFirstTRR; // partial TRR storage
nsresult mFirstTRRresult;
uint8_t mTRRSuccess; // number of successful TRR responses
uint8_t mNativeSuccess; // number of native lookup responses
mozilla::Atomic<bool> mTRRUsed; // TRR was used on this record
uint8_t mTRRSuccess; // number of successful TRR responses
uint8_t mNativeSuccess; // number of native lookup responses
uint16_t mNative : 1; // true if this record is being resolved "natively",
// which means that it is either on the pending queue
// or owned by one of the worker threads. */
uint16_t mTRRUsed : 1; // TRR was used on this record
uint16_t mNative : 1; // true if this record is being resolved "natively",
// which means that it is either on the pending queue
// or owned by one of the worker threads. */
uint16_t mNativeUsed : 1;
uint16_t onQueue : 1; // true if pending and on the queue (not yet
// given to getaddrinfo())