From 5568e27ddbde0d5d0b7b1982e6ccde769be8d736 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Thu, 14 Mar 2019 11:23:39 +0100 Subject: [PATCH 01/13] Bug 1535273 - Remove webrender::util::rect_is_empty. r=kats --- gfx/wr/webrender/src/texture_allocator.rs | 5 ++--- gfx/wr/webrender/src/util.rs | 7 ------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/gfx/wr/webrender/src/texture_allocator.rs b/gfx/wr/webrender/src/texture_allocator.rs index 5584338ec80e..1f7902b2191e 100644 --- a/gfx/wr/webrender/src/texture_allocator.rs +++ b/gfx/wr/webrender/src/texture_allocator.rs @@ -3,7 +3,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use api::units::{DeviceIntPoint, DeviceIntRect, DeviceIntSize}; -use util; //TODO: gather real-world statistics on the bin usage in order to assist the decision // on where to place the size thresholds. @@ -167,10 +166,10 @@ impl ArrayAllocationTracker { } // Add the guillotined rects back to the free list. - if !util::rect_is_empty(&new_free_rect_to_right) { + if !new_free_rect_to_right.is_empty() { self.push(chosen.slice, new_free_rect_to_right); } - if !util::rect_is_empty(&new_free_rect_to_bottom) { + if !new_free_rect_to_bottom.is_empty() { self.push(chosen.slice, new_free_rect_to_bottom); } } diff --git a/gfx/wr/webrender/src/util.rs b/gfx/wr/webrender/src/util.rs index f64811a30f38..8e952e690e80 100644 --- a/gfx/wr/webrender/src/util.rs +++ b/gfx/wr/webrender/src/util.rs @@ -7,7 +7,6 @@ use api::units::*; use euclid::{Point2D, Rect, Size2D, TypedPoint2D, TypedRect, TypedSize2D, Vector2D}; use euclid::{TypedTransform2D, TypedTransform3D, TypedVector2D, TypedScale}; use malloc_size_of::{MallocShallowSizeOf, MallocSizeOf, MallocSizeOfOps}; -use num_traits::Zero; use plane_split::{Clipper, Polygon}; use std::{i32, f32, fmt, ptr}; use std::borrow::Cow; @@ -416,12 +415,6 @@ impl RectHelpers for TypedRect { } } -// Don't use `euclid`'s `is_empty` because that has effectively has an "and" in the conditional -// below instead of an "or". -pub fn rect_is_empty(rect: &TypedRect) -> bool { - rect.size.width == Zero::zero() || rect.size.height == Zero::zero() -} - #[allow(dead_code)] #[inline] pub fn rect_from_points_f(x0: f32, y0: f32, x1: f32, y1: f32) -> Rect { From c341b17328a9fe6b4c3dc52682a47f986f7d94f5 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Fri, 12 Apr 2019 16:08:39 +0200 Subject: [PATCH 02/13] Bug 1535273 - Remove webrender::util::rect_from_points. r=kats --- gfx/wr/webrender/src/util.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/gfx/wr/webrender/src/util.rs b/gfx/wr/webrender/src/util.rs index 8e952e690e80..dbb7dd31a607 100644 --- a/gfx/wr/webrender/src/util.rs +++ b/gfx/wr/webrender/src/util.rs @@ -4,7 +4,7 @@ use api::BorderRadius; use api::units::*; -use euclid::{Point2D, Rect, Size2D, TypedPoint2D, TypedRect, TypedSize2D, Vector2D}; +use euclid::{TypedPoint2D, TypedRect, TypedSize2D, Vector2D}; use euclid::{TypedTransform2D, TypedTransform3D, TypedVector2D, TypedScale}; use malloc_size_of::{MallocShallowSizeOf, MallocSizeOf, MallocSizeOfOps}; use plane_split::{Clipper, Polygon}; @@ -415,12 +415,6 @@ impl RectHelpers for TypedRect { } } -#[allow(dead_code)] -#[inline] -pub fn rect_from_points_f(x0: f32, y0: f32, x1: f32, y1: f32) -> Rect { - Rect::new(Point2D::new(x0, y0), Size2D::new(x1 - x0, y1 - y0)) -} - pub fn lerp(a: f32, b: f32, t: f32) -> f32 { (b - a) * t + a } From c0b14e4c16833fc282a71692e940ad12ddba5174 Mon Sep 17 00:00:00 2001 From: longsonr Date: Fri, 12 Apr 2019 15:44:20 +0100 Subject: [PATCH 03/13] Bug 1542646 Part 3 - forward declaration of DOMSVGAnimatedNumberList is in the wrong namespace r=dholbert --- dom/svg/SVGComponentTransferFunctionElement.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dom/svg/SVGComponentTransferFunctionElement.h b/dom/svg/SVGComponentTransferFunctionElement.h index cf3ed14cc444..517a20639d40 100644 --- a/dom/svg/SVGComponentTransferFunctionElement.h +++ b/dom/svg/SVGComponentTransferFunctionElement.h @@ -21,10 +21,10 @@ namespace mozilla { -class DOMSVGAnimatedNumberList; - namespace dom { +class DOMSVGAnimatedNumberList; + typedef SVGFEUnstyledElement SVGComponentTransferFunctionElementBase; class SVGComponentTransferFunctionElement From 4928e9ca911922273e5752a6ae1fe87bf061ded8 Mon Sep 17 00:00:00 2001 From: longsonr Date: Fri, 12 Apr 2019 15:47:56 +0100 Subject: [PATCH 04/13] Bug 1542646 Part 4 - use nullptr rather than NULL r=dholbert --- dom/svg/SVGContentUtils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dom/svg/SVGContentUtils.cpp b/dom/svg/SVGContentUtils.cpp index c976cc53cc5a..a1494d130704 100644 --- a/dom/svg/SVGContentUtils.cpp +++ b/dom/svg/SVGContentUtils.cpp @@ -784,7 +784,7 @@ already_AddRefed SVGContentUtils::GetPath( SVGPathData pathData; SVGPathDataParser parser(aPathString, &pathData); if (!parser.Parse()) { - return NULL; + return nullptr; } RefPtr drawTarget = From 07bf678276c2899c2e904dacfde82da89c873356 Mon Sep 17 00:00:00 2001 From: longsonr Date: Fri, 12 Apr 2019 17:28:13 +0100 Subject: [PATCH 05/13] Bug 1542646 Part 5 - don't use instance variables to access static methods r=dholbert --- layout/svg/SVGTextFrame.cpp | 2 +- layout/svg/nsSVGForeignObjectFrame.cpp | 2 +- layout/svg/nsSVGIntegrationUtils.cpp | 2 +- layout/svg/nsSVGOuterSVGFrame.cpp | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/layout/svg/SVGTextFrame.cpp b/layout/svg/SVGTextFrame.cpp index 1803f3b9a7e6..a6e3d2e72cec 100644 --- a/layout/svg/SVGTextFrame.cpp +++ b/layout/svg/SVGTextFrame.cpp @@ -3677,7 +3677,7 @@ uint32_t SVGTextFrame::GetNumberOfChars(nsIContent* aContent) { float SVGTextFrame::GetComputedTextLength(nsIContent* aContent) { UpdateGlyphPositioning(); - float cssPxPerDevPx = PresContext()->AppUnitsToFloatCSSPixels( + float cssPxPerDevPx = nsPresContext::AppUnitsToFloatCSSPixels( PresContext()->AppUnitsPerDevPixel()); nscoord length = 0; diff --git a/layout/svg/nsSVGForeignObjectFrame.cpp b/layout/svg/nsSVGForeignObjectFrame.cpp index 77bc551b2314..3ab6b93758a9 100644 --- a/layout/svg/nsSVGForeignObjectFrame.cpp +++ b/layout/svg/nsSVGForeignObjectFrame.cpp @@ -253,7 +253,7 @@ void nsSVGForeignObjectFrame::PaintSVG(gfxContext& aContext, // SVG paints in CSS px, but normally frames paint in dev pixels. Here we // multiply a CSS-px-to-dev-pixel factor onto aTransform so our children // paint correctly. - float cssPxPerDevPx = PresContext()->AppUnitsToFloatCSSPixels( + float cssPxPerDevPx = nsPresContext::AppUnitsToFloatCSSPixels( PresContext()->AppUnitsPerDevPixel()); gfxMatrix canvasTMForChildren = aTransform; canvasTMForChildren.PreScale(cssPxPerDevPx, cssPxPerDevPx); diff --git a/layout/svg/nsSVGIntegrationUtils.cpp b/layout/svg/nsSVGIntegrationUtils.cpp index d35bfb876b86..fdfb390f9afd 100644 --- a/layout/svg/nsSVGIntegrationUtils.cpp +++ b/layout/svg/nsSVGIntegrationUtils.cpp @@ -95,7 +95,7 @@ class PreEffectsVisualOverflowCollector : public nsLayoutUtils::BoxCallback { if (nsSVGIntegrationUtils::UsingOverflowAffectingEffects(aFrame) && !aInReflow) { nsOverflowAreas* preTransformOverflows = - aFrame->GetProperty(aFrame->PreTransformOverflowAreasProperty()); + aFrame->GetProperty(nsIFrame::PreTransformOverflowAreasProperty()); MOZ_ASSERT(!preTransformOverflows, "GetVisualOverflowRect() won't return the pre-effects rect!"); diff --git a/layout/svg/nsSVGOuterSVGFrame.cpp b/layout/svg/nsSVGOuterSVGFrame.cpp index da2ad0620d01..8f7f7dcad717 100644 --- a/layout/svg/nsSVGOuterSVGFrame.cpp +++ b/layout/svg/nsSVGOuterSVGFrame.cpp @@ -860,7 +860,7 @@ gfxMatrix nsSVGOuterSVGFrame::GetCanvasTM() { if (!mCanvasTM) { SVGSVGElement* content = static_cast(GetContent()); - float devPxPerCSSPx = 1.0f / PresContext()->AppUnitsToFloatCSSPixels( + float devPxPerCSSPx = 1.0f / nsPresContext::AppUnitsToFloatCSSPixels( PresContext()->AppUnitsPerDevPixel()); gfxMatrix tm = content->PrependLocalTransformsTo( From eff829f752c7cf1f8b026a51da54ff784455208c Mon Sep 17 00:00:00 2001 From: longsonr Date: Fri, 12 Apr 2019 17:29:32 +0100 Subject: [PATCH 06/13] Bug 1542646 Part 6 - pass by reference when it's more efficient r=dholbert --- layout/svg/nsCSSClipPathInstance.h | 2 +- layout/svg/nsSVGPatternFrame.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/layout/svg/nsCSSClipPathInstance.h b/layout/svg/nsCSSClipPathInstance.h index 87a84b3912bc..73b72e276789 100644 --- a/layout/svg/nsCSSClipPathInstance.h +++ b/layout/svg/nsCSSClipPathInstance.h @@ -33,7 +33,7 @@ class nsCSSClipPathInstance { private: explicit nsCSSClipPathInstance(nsIFrame* aFrame, - const StyleShapeSource aClipPathStyle) + const StyleShapeSource& aClipPathStyle) : mTargetFrame(aFrame), mClipPathStyle(aClipPathStyle) {} already_AddRefed CreateClipPath(DrawTarget* aDrawTarget); diff --git a/layout/svg/nsSVGPatternFrame.cpp b/layout/svg/nsSVGPatternFrame.cpp index a9a5c50fb770..e778286184ed 100644 --- a/layout/svg/nsSVGPatternFrame.cpp +++ b/layout/svg/nsSVGPatternFrame.cpp @@ -636,7 +636,7 @@ gfxMatrix nsSVGPatternFrame::ConstructCTM(const SVGAnimatedViewBox &aViewBox, if (!aViewBox.IsExplicitlySet()) { return gfxMatrix(scaleX, 0.0, 0.0, scaleY, 0.0, 0.0); } - const SVGViewBox viewBox = aViewBox.GetAnimValue(); + const SVGViewBox& viewBox = aViewBox.GetAnimValue(); if (viewBox.height <= 0.0f || viewBox.width <= 0.0f) { return gfxMatrix(0.0, 0.0, 0.0, 0.0, 0.0, 0.0); // singular From 67a7d1d576db98ec015271b55a455d184645c70b Mon Sep 17 00:00:00 2001 From: Brendan Dahl Date: Fri, 5 Apr 2019 08:44:56 -0700 Subject: [PATCH 07/13] Bug 1543245 - Use prototype document for all chrome XHTML pages. r=smaug Let all chrome privileged XHTML take advantage of the cache and faster document creation with the prototype document. Differential Revision: https://phabricator.services.mozilla.com/D26822 --- dom/html/nsHTMLDocument.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/dom/html/nsHTMLDocument.cpp b/dom/html/nsHTMLDocument.cpp index ba48cd947729..e1eb0293ca3e 100644 --- a/dom/html/nsHTMLDocument.cpp +++ b/dom/html/nsHTMLDocument.cpp @@ -444,7 +444,7 @@ void nsHTMLDocument::TryFallback(int32_t& aCharsetSource, aEncoding = FallbackEncoding::FromLocale(); } -// Using a prototype document is currently only allowed with browser.xhtml. +// Using a prototype document is only allowed with chrome privilege. bool ShouldUsePrototypeDocument(nsIChannel* aChannel, nsIDocShell* aDocShell) { if (!aChannel || !aDocShell || !StaticPrefs::dom_prototype_document_cache_enabled()) { @@ -455,9 +455,7 @@ bool ShouldUsePrototypeDocument(nsIChannel* aChannel, nsIDocShell* aDocShell) { } nsCOMPtr originalURI; aChannel->GetOriginalURI(getter_AddRefs(originalURI)); - return IsChromeURI(originalURI) && - originalURI->GetSpecOrDefault().EqualsLiteral( - BROWSER_CHROME_URL_QUOTED); + return IsChromeURI(originalURI); } nsresult nsHTMLDocument::StartDocumentLoad(const char* aCommand, @@ -561,7 +559,7 @@ nsresult nsHTMLDocument::StartDocumentLoad(const char* aCommand, } else { mParser->MarkAsNotScriptCreated(aCommand); } - } else if (ShouldUsePrototypeDocument(aChannel, docShell)) { + } else if (xhtml && ShouldUsePrototypeDocument(aChannel, docShell)) { loadWithPrototype = true; nsCOMPtr originalURI; aChannel->GetOriginalURI(getter_AddRefs(originalURI)); From 4ac9a7a0eb5bbb6018c27f3f21df4e3c9664f36c Mon Sep 17 00:00:00 2001 From: Brendan Dahl Date: Mon, 8 Apr 2019 14:01:29 -0700 Subject: [PATCH 08/13] Bug 1543245 - Prevent HTML script tags from loading twice when using PrototypeDocument. r=smaug HTML script tags were being loaded once by the element when it was bound to the tree and a second time by the PrototypeDocumentContentSink. This patch disables the script element from loading itself. Differential Revision: https://phabricator.services.mozilla.com/D26823 --- dom/prototype/PrototypeDocumentContentSink.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/dom/prototype/PrototypeDocumentContentSink.cpp b/dom/prototype/PrototypeDocumentContentSink.cpp index f27985550167..c7bce8f278a7 100644 --- a/dom/prototype/PrototypeDocumentContentSink.cpp +++ b/dom/prototype/PrototypeDocumentContentSink.cpp @@ -1034,6 +1034,15 @@ nsresult PrototypeDocumentContentSink::CreateElementFromPrototype( rv = AddAttributes(aPrototype, result); if (NS_FAILED(rv)) return rv; + + if (xtfNi->Equals(nsGkAtoms::script, kNameSpaceID_XHTML) || + xtfNi->Equals(nsGkAtoms::script, kNameSpaceID_SVG)) { + nsCOMPtr sele = do_QueryInterface(result); + MOZ_ASSERT(sele, "Node didn't QI to script."); + // Script loading is handled by the this content sink, so prevent the + // script from loading when it is bound to the document. + sele->PreventExecution(); + } } result.forget(aResult); From 9409810d68661ddfdb44f071ddd930edbb1a4c91 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 29 Jan 2019 11:38:56 -0500 Subject: [PATCH 09/13] Bug 1543786 - Ensure that we revoke a top frame's storage access when it is navigated away; r=baku Differential Revision: https://phabricator.services.mozilla.com/D27155 --- docshell/base/nsDocShell.cpp | 20 +++++++- docshell/base/nsDocShell.h | 3 ++ dom/base/nsFrameLoader.cpp | 8 ++++ dom/ipc/TabChild.cpp | 9 +++- .../test/browser/antitracking_head.js | 47 ++++++++++++++++++- .../antitracking/test/browser/browser.ini | 2 + ...er_storageAccessRemovalNavigateTopframe.js | 40 ++++++++++++++++ 7 files changed, 124 insertions(+), 5 deletions(-) create mode 100644 toolkit/components/antitracking/test/browser/browser_storageAccessRemovalNavigateTopframe.js diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index ececb52169fb..c8a8f1f9fe7a 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -2644,8 +2644,8 @@ nsresult nsDocShell::SetDocLoaderParent(nsDocLoader* aParent) { RecomputeCanExecuteScripts(); // Inform windows when they're being removed from their parent. - if (!aParent && mScriptGlobal) { - mScriptGlobal->ParentWindowChanged(); + if (!aParent) { + MaybeClearStorageAccessFlag(); } NS_ASSERTION(mInheritPrivateBrowsingId || wasPrivate == UsePrivateBrowsing(), @@ -2654,6 +2654,22 @@ nsresult nsDocShell::SetDocLoaderParent(nsDocLoader* aParent) { return NS_OK; } +void nsDocShell::MaybeClearStorageAccessFlag() { + if (mScriptGlobal) { + // Tell our window that the parent has now changed. + mScriptGlobal->ParentWindowChanged(); + + // Tell all of our children about the change recursively as well. + nsTObserverArray::ForwardIterator iter(mChildList); + while (iter.HasMore()) { + nsCOMPtr child = do_QueryObject(iter.GetNext()); + if (child) { + static_cast(child.get())->MaybeClearStorageAccessFlag(); + } + } + } +} + NS_IMETHODIMP nsDocShell::GetSameTypeParent(nsIDocShellTreeItem** aParent) { NS_ENSURE_ARG_POINTER(aParent); diff --git a/docshell/base/nsDocShell.h b/docshell/base/nsDocShell.h index 1b9b10153db3..e6e355d6c0a3 100644 --- a/docshell/base/nsDocShell.h +++ b/docshell/base/nsDocShell.h @@ -399,6 +399,9 @@ class nsDocShell final : public nsDocLoader, nsresult InternalLoad(nsDocShellLoadState* aLoadState, nsIDocShell** aDocShell, nsIRequest** aRequest); + // Clear the document's storage access flag if needed. + void MaybeClearStorageAccessFlag(); + private: // member functions friend class nsDSURIContentListener; friend class FramingChecker; diff --git a/dom/base/nsFrameLoader.cpp b/dom/base/nsFrameLoader.cpp index 70df167f011e..62db863ef8f2 100644 --- a/dom/base/nsFrameLoader.cpp +++ b/dom/base/nsFrameLoader.cpp @@ -425,6 +425,12 @@ nsresult nsFrameLoader::ReallyStartLoadingInternal() { return NS_OK; } + if (mDocShell) { + // If we already have a docshell, ensure that the docshell's storage access + // flag is cleared. + mDocShell->MaybeClearStorageAccessFlag(); + } + nsresult rv = MaybeCreateDocShell(); if (NS_FAILED(rv)) { return rv; @@ -942,6 +948,8 @@ void nsFrameLoader::Hide() { if (!mDocShell) return; + docShell->MaybeClearStorageAccessFlag(); + nsCOMPtr contentViewer; mDocShell->GetContentViewer(getter_AddRefs(contentViewer)); if (contentViewer) contentViewer->SetSticky(false); diff --git a/dom/ipc/TabChild.cpp b/dom/ipc/TabChild.cpp index 159c8e6fcf37..d3e0e22caff9 100644 --- a/dom/ipc/TabChild.cpp +++ b/dom/ipc/TabChild.cpp @@ -1051,14 +1051,19 @@ mozilla::ipc::IPCResult TabChild::RecvLoadURL(const nsCString& aURI, nsIWebNavigation::LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP | nsIWebNavigation::LOAD_FLAGS_DISALLOW_INHERIT_PRINCIPAL; - nsresult rv = - WebNavigation()->LoadURI(NS_ConvertUTF8toUTF16(aURI), loadURIOptions); + nsIWebNavigation* webNav = WebNavigation(); + nsresult rv = webNav->LoadURI(NS_ConvertUTF8toUTF16(aURI), loadURIOptions); if (NS_FAILED(rv)) { NS_WARNING( "WebNavigation()->LoadURI failed. Eating exception, what else can I " "do?"); } + nsCOMPtr docShell = do_GetInterface(WebNavigation()); + if (docShell) { + nsDocShell::Cast(docShell)->MaybeClearStorageAccessFlag(); + } + CrashReporter::AnnotateCrashReport(CrashReporter::Annotation::URL, aURI); return IPC_OK(); diff --git a/toolkit/components/antitracking/test/browser/antitracking_head.js b/toolkit/components/antitracking/test/browser/antitracking_head.js index 76463b613440..dec8f072fed4 100644 --- a/toolkit/components/antitracking/test/browser/antitracking_head.js +++ b/toolkit/components/antitracking/test/browser/antitracking_head.js @@ -335,7 +335,7 @@ this.AntiTracking = { } else { thirdPartyPage = TEST_3RD_PARTY_PAGE; } - await ContentTask.spawn(browser, + let id = await ContentTask.spawn(browser, { page: thirdPartyPage, nextPage: TEST_4TH_PARTY_PAGE, callback: options.callback.toString(), @@ -420,14 +420,59 @@ this.AntiTracking = { ifr.src = obj.nextPage; }); + case "navigate-topframe": + // pass-through break; default: ok(false, "Unexpected accessRemoval code passed: " + obj.accessRemoval); break; } } + + return id; }); + if (doAccessRemovalChecks && + options.accessRemoval == "navigate-topframe") { + await BrowserTestUtils.loadURI(browser, TEST_4TH_PARTY_PAGE); + await BrowserTestUtils.browserLoaded(browser); + + let pageshow = BrowserTestUtils.waitForContentEvent(tab.linkedBrowser, "pageshow"); + gBrowser.goBack(); + await pageshow; + + await ContentTask.spawn(browser, + { id, + callbackAfterRemoval: options.callbackAfterRemoval ? + options.callbackAfterRemoval.toString() : null, + }, + async function(obj) { + let ifr = content.document.getElementById(obj.id); + content.alert(ifr); + ifr.contentWindow.postMessage(obj.callbackAfterRemoval, "*"); + + content.addEventListener("message", function msg(event) { + if (event.data.type == "finish") { + content.removeEventListener("message", msg); + resolve(); + return; + } + + if (event.data.type == "ok") { + ok(event.data.what, event.data.msg); + return; + } + + if (event.data.type == "info") { + info(event.data.msg); + return; + } + + ok(false, "Unknown message"); + }); + }); + } + if (options.allowList) { info("Enabling content blocking for this page"); win.ContentBlocking.enableForCurrentPage(); diff --git a/toolkit/components/antitracking/test/browser/browser.ini b/toolkit/components/antitracking/test/browser/browser.ini index b2fe8e22f93f..83483da67cfa 100644 --- a/toolkit/components/antitracking/test/browser/browser.ini +++ b/toolkit/components/antitracking/test/browser/browser.ini @@ -79,6 +79,8 @@ skip-if = serviceworker_e10s [browser_storageAccessPromiseResolveHandlerUserInteraction.js] [browser_storageAccessRemovalNavigateSubframe.js] skip-if = serviceworker_e10s +[browser_storageAccessRemovalNavigateTopframe.js] +skip-if = serviceworker_e10s [browser_storageAccessSandboxed.js] skip-if = serviceworker_e10s [browser_storageAccessWithHeuristics.js] diff --git a/toolkit/components/antitracking/test/browser/browser_storageAccessRemovalNavigateTopframe.js b/toolkit/components/antitracking/test/browser/browser_storageAccessRemovalNavigateTopframe.js new file mode 100644 index 000000000000..71b1340c7e0a --- /dev/null +++ b/toolkit/components/antitracking/test/browser/browser_storageAccessRemovalNavigateTopframe.js @@ -0,0 +1,40 @@ +AntiTracking.runTest("Storage Access is removed when topframe navigates", + // blocking callback + async _ => { + /* import-globals-from storageAccessAPIHelpers.js */ + await noStorageAccessInitially(); + }, + + // non-blocking callback + async _ => { + /* import-globals-from storageAccessAPIHelpers.js */ + if (allowListed) { + await hasStorageAccessInitially(); + } else { + await noStorageAccessInitially(); + } + + /* import-globals-from storageAccessAPIHelpers.js */ + let [threw, rejected] = await callRequestStorageAccess(); + ok(!threw, "requestStorageAccess should not throw"); + ok(!rejected, "requestStorageAccess should be available"); + }, + // cleanup function + async _ => { + await new Promise(resolve => { + Services.clearData.deleteData(Ci.nsIClearDataService.CLEAR_ALL, value => resolve()); + }); + }, + null, // extra prefs + false, // no window open test + false, // no user-interaction test + 0, // no blocking notifications + false, // run in normal window + null, // no iframe sandbox + "navigate-topframe", // access removal type + // after-removal callback + async _ => { + /* import-globals-from storageAccessAPIHelpers.js */ + await noStorageAccessInitially(); + } +); From decc40afd52152ae63c889dafaf559da17a3df9e Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Fri, 12 Apr 2019 19:08:36 -0400 Subject: [PATCH 10/13] Bug 1543786 follow-up: Remove a call to a non-existent function --- .../components/antitracking/test/browser/antitracking_head.js | 1 - 1 file changed, 1 deletion(-) diff --git a/toolkit/components/antitracking/test/browser/antitracking_head.js b/toolkit/components/antitracking/test/browser/antitracking_head.js index dec8f072fed4..41514b494d6b 100644 --- a/toolkit/components/antitracking/test/browser/antitracking_head.js +++ b/toolkit/components/antitracking/test/browser/antitracking_head.js @@ -454,7 +454,6 @@ this.AntiTracking = { content.addEventListener("message", function msg(event) { if (event.data.type == "finish") { content.removeEventListener("message", msg); - resolve(); return; } From a9e2a9b857804983bebb21b55f54f9b8c1d195d1 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Fri, 12 Apr 2019 19:30:25 -0400 Subject: [PATCH 11/13] Bug 1543786 - Fix build failure (landed on a CLOSED TREE) --- dom/base/nsFrameLoader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dom/base/nsFrameLoader.cpp b/dom/base/nsFrameLoader.cpp index 62db863ef8f2..a30e5353692b 100644 --- a/dom/base/nsFrameLoader.cpp +++ b/dom/base/nsFrameLoader.cpp @@ -948,7 +948,7 @@ void nsFrameLoader::Hide() { if (!mDocShell) return; - docShell->MaybeClearStorageAccessFlag(); + mDocShell->MaybeClearStorageAccessFlag(); nsCOMPtr contentViewer; mDocShell->GetContentViewer(getter_AddRefs(contentViewer)); From 8fa5eb9862eca91cee9d807d51879b33e1712810 Mon Sep 17 00:00:00 2001 From: Bogdan Tara Date: Sat, 13 Apr 2019 05:14:11 +0300 Subject: [PATCH 12/13] Backed out 3 changesets (bug 1543786) for browser_storageAccessRemovalNavigateTopframe.js failures CLOSED TREE Backed out changeset 4f63311e6f00 (bug 1543786) Backed out changeset 757b729752e0 (bug 1543786) Backed out changeset 6aabad91d980 (bug 1543786) --- docshell/base/nsDocShell.cpp | 20 +------- docshell/base/nsDocShell.h | 3 -- dom/base/nsFrameLoader.cpp | 8 ---- dom/ipc/TabChild.cpp | 9 +--- .../test/browser/antitracking_head.js | 46 +------------------ .../antitracking/test/browser/browser.ini | 2 - ...er_storageAccessRemovalNavigateTopframe.js | 40 ---------------- 7 files changed, 5 insertions(+), 123 deletions(-) delete mode 100644 toolkit/components/antitracking/test/browser/browser_storageAccessRemovalNavigateTopframe.js diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index c8a8f1f9fe7a..ececb52169fb 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -2644,8 +2644,8 @@ nsresult nsDocShell::SetDocLoaderParent(nsDocLoader* aParent) { RecomputeCanExecuteScripts(); // Inform windows when they're being removed from their parent. - if (!aParent) { - MaybeClearStorageAccessFlag(); + if (!aParent && mScriptGlobal) { + mScriptGlobal->ParentWindowChanged(); } NS_ASSERTION(mInheritPrivateBrowsingId || wasPrivate == UsePrivateBrowsing(), @@ -2654,22 +2654,6 @@ nsresult nsDocShell::SetDocLoaderParent(nsDocLoader* aParent) { return NS_OK; } -void nsDocShell::MaybeClearStorageAccessFlag() { - if (mScriptGlobal) { - // Tell our window that the parent has now changed. - mScriptGlobal->ParentWindowChanged(); - - // Tell all of our children about the change recursively as well. - nsTObserverArray::ForwardIterator iter(mChildList); - while (iter.HasMore()) { - nsCOMPtr child = do_QueryObject(iter.GetNext()); - if (child) { - static_cast(child.get())->MaybeClearStorageAccessFlag(); - } - } - } -} - NS_IMETHODIMP nsDocShell::GetSameTypeParent(nsIDocShellTreeItem** aParent) { NS_ENSURE_ARG_POINTER(aParent); diff --git a/docshell/base/nsDocShell.h b/docshell/base/nsDocShell.h index e6e355d6c0a3..1b9b10153db3 100644 --- a/docshell/base/nsDocShell.h +++ b/docshell/base/nsDocShell.h @@ -399,9 +399,6 @@ class nsDocShell final : public nsDocLoader, nsresult InternalLoad(nsDocShellLoadState* aLoadState, nsIDocShell** aDocShell, nsIRequest** aRequest); - // Clear the document's storage access flag if needed. - void MaybeClearStorageAccessFlag(); - private: // member functions friend class nsDSURIContentListener; friend class FramingChecker; diff --git a/dom/base/nsFrameLoader.cpp b/dom/base/nsFrameLoader.cpp index a30e5353692b..70df167f011e 100644 --- a/dom/base/nsFrameLoader.cpp +++ b/dom/base/nsFrameLoader.cpp @@ -425,12 +425,6 @@ nsresult nsFrameLoader::ReallyStartLoadingInternal() { return NS_OK; } - if (mDocShell) { - // If we already have a docshell, ensure that the docshell's storage access - // flag is cleared. - mDocShell->MaybeClearStorageAccessFlag(); - } - nsresult rv = MaybeCreateDocShell(); if (NS_FAILED(rv)) { return rv; @@ -948,8 +942,6 @@ void nsFrameLoader::Hide() { if (!mDocShell) return; - mDocShell->MaybeClearStorageAccessFlag(); - nsCOMPtr contentViewer; mDocShell->GetContentViewer(getter_AddRefs(contentViewer)); if (contentViewer) contentViewer->SetSticky(false); diff --git a/dom/ipc/TabChild.cpp b/dom/ipc/TabChild.cpp index d3e0e22caff9..159c8e6fcf37 100644 --- a/dom/ipc/TabChild.cpp +++ b/dom/ipc/TabChild.cpp @@ -1051,19 +1051,14 @@ mozilla::ipc::IPCResult TabChild::RecvLoadURL(const nsCString& aURI, nsIWebNavigation::LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP | nsIWebNavigation::LOAD_FLAGS_DISALLOW_INHERIT_PRINCIPAL; - nsIWebNavigation* webNav = WebNavigation(); - nsresult rv = webNav->LoadURI(NS_ConvertUTF8toUTF16(aURI), loadURIOptions); + nsresult rv = + WebNavigation()->LoadURI(NS_ConvertUTF8toUTF16(aURI), loadURIOptions); if (NS_FAILED(rv)) { NS_WARNING( "WebNavigation()->LoadURI failed. Eating exception, what else can I " "do?"); } - nsCOMPtr docShell = do_GetInterface(WebNavigation()); - if (docShell) { - nsDocShell::Cast(docShell)->MaybeClearStorageAccessFlag(); - } - CrashReporter::AnnotateCrashReport(CrashReporter::Annotation::URL, aURI); return IPC_OK(); diff --git a/toolkit/components/antitracking/test/browser/antitracking_head.js b/toolkit/components/antitracking/test/browser/antitracking_head.js index 41514b494d6b..76463b613440 100644 --- a/toolkit/components/antitracking/test/browser/antitracking_head.js +++ b/toolkit/components/antitracking/test/browser/antitracking_head.js @@ -335,7 +335,7 @@ this.AntiTracking = { } else { thirdPartyPage = TEST_3RD_PARTY_PAGE; } - let id = await ContentTask.spawn(browser, + await ContentTask.spawn(browser, { page: thirdPartyPage, nextPage: TEST_4TH_PARTY_PAGE, callback: options.callback.toString(), @@ -420,58 +420,14 @@ this.AntiTracking = { ifr.src = obj.nextPage; }); - case "navigate-topframe": - // pass-through break; default: ok(false, "Unexpected accessRemoval code passed: " + obj.accessRemoval); break; } } - - return id; }); - if (doAccessRemovalChecks && - options.accessRemoval == "navigate-topframe") { - await BrowserTestUtils.loadURI(browser, TEST_4TH_PARTY_PAGE); - await BrowserTestUtils.browserLoaded(browser); - - let pageshow = BrowserTestUtils.waitForContentEvent(tab.linkedBrowser, "pageshow"); - gBrowser.goBack(); - await pageshow; - - await ContentTask.spawn(browser, - { id, - callbackAfterRemoval: options.callbackAfterRemoval ? - options.callbackAfterRemoval.toString() : null, - }, - async function(obj) { - let ifr = content.document.getElementById(obj.id); - content.alert(ifr); - ifr.contentWindow.postMessage(obj.callbackAfterRemoval, "*"); - - content.addEventListener("message", function msg(event) { - if (event.data.type == "finish") { - content.removeEventListener("message", msg); - return; - } - - if (event.data.type == "ok") { - ok(event.data.what, event.data.msg); - return; - } - - if (event.data.type == "info") { - info(event.data.msg); - return; - } - - ok(false, "Unknown message"); - }); - }); - } - if (options.allowList) { info("Enabling content blocking for this page"); win.ContentBlocking.enableForCurrentPage(); diff --git a/toolkit/components/antitracking/test/browser/browser.ini b/toolkit/components/antitracking/test/browser/browser.ini index 83483da67cfa..b2fe8e22f93f 100644 --- a/toolkit/components/antitracking/test/browser/browser.ini +++ b/toolkit/components/antitracking/test/browser/browser.ini @@ -79,8 +79,6 @@ skip-if = serviceworker_e10s [browser_storageAccessPromiseResolveHandlerUserInteraction.js] [browser_storageAccessRemovalNavigateSubframe.js] skip-if = serviceworker_e10s -[browser_storageAccessRemovalNavigateTopframe.js] -skip-if = serviceworker_e10s [browser_storageAccessSandboxed.js] skip-if = serviceworker_e10s [browser_storageAccessWithHeuristics.js] diff --git a/toolkit/components/antitracking/test/browser/browser_storageAccessRemovalNavigateTopframe.js b/toolkit/components/antitracking/test/browser/browser_storageAccessRemovalNavigateTopframe.js deleted file mode 100644 index 71b1340c7e0a..000000000000 --- a/toolkit/components/antitracking/test/browser/browser_storageAccessRemovalNavigateTopframe.js +++ /dev/null @@ -1,40 +0,0 @@ -AntiTracking.runTest("Storage Access is removed when topframe navigates", - // blocking callback - async _ => { - /* import-globals-from storageAccessAPIHelpers.js */ - await noStorageAccessInitially(); - }, - - // non-blocking callback - async _ => { - /* import-globals-from storageAccessAPIHelpers.js */ - if (allowListed) { - await hasStorageAccessInitially(); - } else { - await noStorageAccessInitially(); - } - - /* import-globals-from storageAccessAPIHelpers.js */ - let [threw, rejected] = await callRequestStorageAccess(); - ok(!threw, "requestStorageAccess should not throw"); - ok(!rejected, "requestStorageAccess should be available"); - }, - // cleanup function - async _ => { - await new Promise(resolve => { - Services.clearData.deleteData(Ci.nsIClearDataService.CLEAR_ALL, value => resolve()); - }); - }, - null, // extra prefs - false, // no window open test - false, // no user-interaction test - 0, // no blocking notifications - false, // run in normal window - null, // no iframe sandbox - "navigate-topframe", // access removal type - // after-removal callback - async _ => { - /* import-globals-from storageAccessAPIHelpers.js */ - await noStorageAccessInitially(); - } -); From 8442bbf9c97a253053549b0af9bdb5700d0da1fd Mon Sep 17 00:00:00 2001 From: Coroiu Cristina Date: Sat, 13 Apr 2019 14:57:11 +0300 Subject: [PATCH 13/13] Backed out 3 changesets (bug 1525640) for causing leaks a=backout Backed out changeset efdd32c00dc6 (bug 1525640) Backed out changeset e71641f0465b (bug 1525640) Backed out changeset e3539a40afdf (bug 1525640) --- .../test/webrtcproxychannel_unittest.cpp | 4 ---- netwerk/base/nsISocketTransport.idl | 5 ----- netwerk/base/nsSocketTransport2.cpp | 9 -------- netwerk/base/nsSocketTransport2.h | 1 - netwerk/dns/TRR.cpp | 2 +- netwerk/dns/nsDNSService2.cpp | 1 - .../protocol/http/ClassifierDummyChannel.cpp | 13 ++--------- netwerk/protocol/http/Http2Session.cpp | 2 +- netwerk/protocol/http/Http2Stream.cpp | 2 +- netwerk/protocol/http/HttpBaseChannel.cpp | 22 ++++++------------- netwerk/protocol/http/HttpBaseChannel.h | 12 +++------- netwerk/protocol/http/HttpChannelChild.cpp | 15 +++++-------- netwerk/protocol/http/HttpChannelChild.h | 4 ++-- netwerk/protocol/http/HttpChannelParent.cpp | 5 +---- netwerk/protocol/http/PHttpChannel.ipdl | 1 - netwerk/protocol/http/TunnelUtils.cpp | 8 ------- netwerk/protocol/http/nsHttpChannel.cpp | 10 +++------ netwerk/protocol/http/nsHttpConnection.cpp | 2 +- .../protocol/http/nsHttpConnectionInfo.cpp | 6 ++--- netwerk/protocol/http/nsHttpConnectionInfo.h | 11 ++++------ netwerk/protocol/http/nsHttpTransaction.cpp | 2 -- netwerk/protocol/http/nsHttpTransaction.h | 2 -- .../protocol/http/nsIHttpChannelInternal.idl | 9 +------- 23 files changed, 35 insertions(+), 113 deletions(-) diff --git a/media/mtransport/test/webrtcproxychannel_unittest.cpp b/media/mtransport/test/webrtcproxychannel_unittest.cpp index acae17fd0a3b..1f664d8cd51a 100644 --- a/media/mtransport/test/webrtcproxychannel_unittest.cpp +++ b/media/mtransport/test/webrtcproxychannel_unittest.cpp @@ -182,10 +182,6 @@ class FakeSocketTransportProvider : public nsISocketTransport { MOZ_ASSERT(false); return NS_OK; } - NS_IMETHOD ResolvedByTRR(bool *aResolvedByTRR) override { - MOZ_ASSERT(false); - return NS_OK; - } // nsITransport NS_IMETHOD OpenInputStream(uint32_t aFlags, uint32_t aSegmentSize, diff --git a/netwerk/base/nsISocketTransport.idl b/netwerk/base/nsISocketTransport.idl index ad9a58134128..c59ebe7a9104 100644 --- a/netwerk/base/nsISocketTransport.idl +++ b/netwerk/base/nsISocketTransport.idl @@ -314,9 +314,4 @@ interface nsISocketTransport : nsITransport * The value is set after PR_Connect is called. */ readonly attribute boolean esniUsed; - - /** - * IP address resolved using TRR. - */ - bool resolvedByTRR(); }; diff --git a/netwerk/base/nsSocketTransport2.cpp b/netwerk/base/nsSocketTransport2.cpp index fb0c7281fe96..6b8808b6e0f1 100644 --- a/netwerk/base/nsSocketTransport2.cpp +++ b/netwerk/base/nsSocketTransport2.cpp @@ -708,7 +708,6 @@ nsSocketTransport::nsSocketTransport() mInputClosed(true), mOutputClosed(true), mResolving(false), - mResolvedByTRR(false), mDNSLookupStatus(NS_OK), mDNSARequestFinished(0), mEsniQueried(false), @@ -1804,7 +1803,6 @@ bool nsSocketTransport::RecoverFromError() { // try next ip address only if past the resolver stage... if (mState == STATE_CONNECTING && mDNSRecord) { nsresult rv = mDNSRecord->GetNextAddr(SocketPort(), &mNetAddr); - mDNSRecord->IsTRR(&mResolvedByTRR); if (NS_SUCCEEDED(rv)) { SOCKET_LOG((" trying again with next ip address\n")); tryAgain = true; @@ -2098,7 +2096,6 @@ void nsSocketTransport::OnSocketEvent(uint32_t type, nsresult status, mDNSTxtRequest = nullptr; if (mDNSRecord) { mDNSRecord->GetNextAddr(SocketPort(), &mNetAddr); - mDNSRecord->IsTRR(&mResolvedByTRR); } // status contains DNS lookup status if (NS_FAILED(status)) { @@ -3526,11 +3523,5 @@ nsSocketTransport::GetEsniUsed(bool *aEsniUsed) { return NS_OK; } -NS_IMETHODIMP -nsSocketTransport::ResolvedByTRR(bool *aResolvedByTRR) { - *aResolvedByTRR = mResolvedByTRR; - return NS_OK; -} - } // namespace net } // namespace mozilla diff --git a/netwerk/base/nsSocketTransport2.h b/netwerk/base/nsSocketTransport2.h index ee7d86e82f6d..904a241557c0 100644 --- a/netwerk/base/nsSocketTransport2.h +++ b/netwerk/base/nsSocketTransport2.h @@ -328,7 +328,6 @@ class nsSocketTransport final : public nsASocketHandler, nsCOMPtr mDNSRequest; nsCOMPtr mDNSRecord; - bool mResolvedByTRR; nsresult mDNSLookupStatus; PRIntervalTime mDNSARequestFinished; diff --git a/netwerk/dns/TRR.cpp b/netwerk/dns/TRR.cpp index 92e39ce14f44..8b91096d989e 100644 --- a/netwerk/dns/TRR.cpp +++ b/netwerk/dns/TRR.cpp @@ -275,7 +275,7 @@ nsresult TRR::SendHTTPRequest() { // update with each HEADERS or reply to a DATA with a WINDOW UPDATE rv = internalChannel->SetInitialRwin(127 * 1024); NS_ENSURE_SUCCESS(rv, rv); - rv = internalChannel->SetIsTRRServiceChannel(true); + rv = internalChannel->SetTrr(true); NS_ENSURE_SUCCESS(rv, rv); mAllowRFC1918 = gTRRService->AllowRFC1918(); diff --git a/netwerk/dns/nsDNSService2.cpp b/netwerk/dns/nsDNSService2.cpp index 4d3cdf3346db..ed20e02016b8 100644 --- a/netwerk/dns/nsDNSService2.cpp +++ b/netwerk/dns/nsDNSService2.cpp @@ -118,7 +118,6 @@ nsDNSRecord::IsTRR(bool *retval) { } return NS_OK; } - NS_IMETHODIMP nsDNSRecord::GetNextAddr(uint16_t port, NetAddr *addr) { if (mDone) { diff --git a/netwerk/protocol/http/ClassifierDummyChannel.cpp b/netwerk/protocol/http/ClassifierDummyChannel.cpp index 1ba46abe8832..47bfabcbe249 100644 --- a/netwerk/protocol/http/ClassifierDummyChannel.cpp +++ b/netwerk/protocol/http/ClassifierDummyChannel.cpp @@ -476,19 +476,10 @@ ClassifierDummyChannel::SetBeConservative(bool aBeConservative) { } NS_IMETHODIMP -ClassifierDummyChannel::GetIsTRRServiceChannel(bool* aTrr) { - return NS_ERROR_NOT_IMPLEMENTED; -} +ClassifierDummyChannel::GetTrr(bool* aTrr) { return NS_ERROR_NOT_IMPLEMENTED; } NS_IMETHODIMP -ClassifierDummyChannel::SetIsTRRServiceChannel(bool aTrr) { - return NS_ERROR_NOT_IMPLEMENTED; -} - -NS_IMETHODIMP -ClassifierDummyChannel::GetIsResolvedByTRR(bool* aResolvedByTRR) { - return NS_ERROR_NOT_IMPLEMENTED; -} +ClassifierDummyChannel::SetTrr(bool aTrr) { return NS_ERROR_NOT_IMPLEMENTED; } NS_IMETHODIMP ClassifierDummyChannel::GetTlsFlags(uint32_t* aTlsFlags) { diff --git a/netwerk/protocol/http/Http2Session.cpp b/netwerk/protocol/http/Http2Session.cpp index 9e328be3b8fd..ca675b22b940 100644 --- a/netwerk/protocol/http/Http2Session.cpp +++ b/netwerk/protocol/http/Http2Session.cpp @@ -416,7 +416,7 @@ uint32_t Http2Session::RegisterStreamID(Http2Stream *stream, uint32_t aNewID) { // don't count push streams here MOZ_ASSERT(stream->Transaction(), "no transation for the stream!"); RefPtr ci(stream->Transaction()->ConnectionInfo()); - if (ci && ci->GetIsTrrServiceChannel()) { + if (ci && ci->GetTrrUsed()) { IncrementTrrCounter(); } } diff --git a/netwerk/protocol/http/Http2Stream.cpp b/netwerk/protocol/http/Http2Stream.cpp index 67fd7dfed065..79d5986efeb6 100644 --- a/netwerk/protocol/http/Http2Stream.cpp +++ b/netwerk/protocol/http/Http2Stream.cpp @@ -496,7 +496,7 @@ nsresult Http2Stream::ParseHttpRequestHeaders(const char *buf, uint32_t avail, // if the "mother stream" had TRR, this one is a TRR stream too! RefPtr ci(Transaction()->ConnectionInfo()); - if (ci && ci->GetIsTrrServiceChannel()) { + if (ci && ci->GetTrrUsed()) { mSession->IncrementTrrCounter(); } diff --git a/netwerk/protocol/http/HttpBaseChannel.cpp b/netwerk/protocol/http/HttpBaseChannel.cpp index 4862dd787fa2..e63a94ceb35c 100644 --- a/netwerk/protocol/http/HttpBaseChannel.cpp +++ b/netwerk/protocol/http/HttpBaseChannel.cpp @@ -196,8 +196,7 @@ HttpBaseChannel::HttpBaseChannel() mAllowSpdy(true), mAllowAltSvc(true), mBeConservative(false), - mIsTRRServiceChannel(false), - mResolvedByTRR(false), + mTRR(false), mResponseTimeoutEnabled(true), mAllRedirectsSameOrigin(true), mAllRedirectsPassTimingAllowCheck(true), @@ -2702,23 +2701,16 @@ HttpBaseChannel::SetBeConservative(bool aBeConservative) { } NS_IMETHODIMP -HttpBaseChannel::GetIsTRRServiceChannel(bool* aIsTRRServiceChannel) { - NS_ENSURE_ARG_POINTER(aIsTRRServiceChannel); +HttpBaseChannel::GetTrr(bool* aTRR) { + NS_ENSURE_ARG_POINTER(aTRR); - *aIsTRRServiceChannel = mIsTRRServiceChannel; + *aTRR = mTRR; return NS_OK; } NS_IMETHODIMP -HttpBaseChannel::SetIsTRRServiceChannel(bool aIsTRRServiceChannel) { - mIsTRRServiceChannel = aIsTRRServiceChannel; - return NS_OK; -} - -NS_IMETHODIMP -HttpBaseChannel::GetIsResolvedByTRR(bool* aResolvedByTRR) { - NS_ENSURE_ARG_POINTER(aResolvedByTRR); - *aResolvedByTRR = mResolvedByTRR; +HttpBaseChannel::SetTrr(bool aTRR) { + mTRR = aTRR; return NS_OK; } @@ -3585,7 +3577,7 @@ nsresult HttpBaseChannel::SetupReplacementChannel(nsIURI* newURI, MOZ_ASSERT(NS_SUCCEEDED(rv)); rv = httpInternal->SetBeConservative(mBeConservative); MOZ_ASSERT(NS_SUCCEEDED(rv)); - rv = httpInternal->SetIsTRRServiceChannel(mIsTRRServiceChannel); + rv = httpInternal->SetTrr(mTRR); MOZ_ASSERT(NS_SUCCEEDED(rv)); rv = httpInternal->SetTlsFlags(mTlsFlags); MOZ_ASSERT(NS_SUCCEEDED(rv)); diff --git a/netwerk/protocol/http/HttpBaseChannel.h b/netwerk/protocol/http/HttpBaseChannel.h index 3f36e5b6cfdd..78659863de0f 100644 --- a/netwerk/protocol/http/HttpBaseChannel.h +++ b/netwerk/protocol/http/HttpBaseChannel.h @@ -276,9 +276,8 @@ class HttpBaseChannel : public nsHashPropertyBag, NS_IMETHOD SetAllowAltSvc(bool aAllowAltSvc) override; NS_IMETHOD GetBeConservative(bool *aBeConservative) override; NS_IMETHOD SetBeConservative(bool aBeConservative) override; - NS_IMETHOD GetIsTRRServiceChannel(bool *aTRR) override; - NS_IMETHOD SetIsTRRServiceChannel(bool aTRR) override; - NS_IMETHOD GetIsResolvedByTRR(bool *aResolvedByTRR) override; + NS_IMETHOD GetTrr(bool *aTRR) override; + NS_IMETHOD SetTrr(bool aTRR) override; NS_IMETHOD GetTlsFlags(uint32_t *aTlsFlags) override; NS_IMETHOD SetTlsFlags(uint32_t aTlsFlags) override; NS_IMETHOD GetApiRedirectToURI(nsIURI **aApiRedirectToURI) override; @@ -702,12 +701,7 @@ class HttpBaseChannel : public nsHashPropertyBag, // classification. If this is changed or removed, make sure we also update // NS_ShouldClassifyChannel accordingly !!! uint32_t mBeConservative : 1; - // If the current channel is used to as a TRR connection. - uint32_t mIsTRRServiceChannel : 1; - // If the request was performed to a TRR resolved IP address. - // Will be false if loading the resource does not create a connection - // (for example when it's loaded from the cache). - uint32_t mResolvedByTRR : 1; + uint32_t mTRR : 1; uint32_t mResponseTimeoutEnabled : 1; // A flag that should be false only if a cross-domain redirect occurred uint32_t mAllRedirectsSameOrigin : 1; diff --git a/netwerk/protocol/http/HttpChannelChild.cpp b/netwerk/protocol/http/HttpChannelChild.cpp index 4601ba2f3750..ac0a9d2531de 100644 --- a/netwerk/protocol/http/HttpChannelChild.cpp +++ b/netwerk/protocol/http/HttpChannelChild.cpp @@ -406,7 +406,7 @@ class StartRequestEvent : public NeckoTargetChannelEvent { const NetAddr& aPeerAddr, const uint32_t& aCacheKey, const nsCString& altDataType, const int64_t& altDataLen, const bool& deliveringAltData, const bool& aApplyConversion, - const bool& aIsResolvedByTRR, const ResourceTimingStruct& aTiming) + const ResourceTimingStruct& aTiming) : NeckoTargetChannelEvent(aChild), mChannelStatus(aChannelStatus), mResponseHead(aResponseHead), @@ -427,7 +427,6 @@ class StartRequestEvent : public NeckoTargetChannelEvent { mAltDataLen(altDataLen), mDeliveringAltData(deliveringAltData), mLoadInfoForwarder(loadInfoForwarder), - mIsResolvedByTRR(aIsResolvedByTRR), mTiming(aTiming) {} void Run() override { @@ -438,7 +437,7 @@ class StartRequestEvent : public NeckoTargetChannelEvent { mCacheFetchCount, mCacheExpirationTime, mCachedCharset, mSecurityInfoSerialization, mSelfAddr, mPeerAddr, mCacheKey, mAltDataType, mAltDataLen, mDeliveringAltData, mApplyConversion, - mIsResolvedByTRR, mTiming); + mTiming); } private: @@ -461,7 +460,6 @@ class StartRequestEvent : public NeckoTargetChannelEvent { int64_t mAltDataLen; bool mDeliveringAltData; ParentLoadInfoForwarderArgs mLoadInfoForwarder; - bool mIsResolvedByTRR; ResourceTimingStruct mTiming; }; @@ -476,8 +474,7 @@ mozilla::ipc::IPCResult HttpChannelChild::RecvOnStartRequest( const NetAddr& peerAddr, const int16_t& redirectCount, const uint32_t& cacheKey, const nsCString& altDataType, const int64_t& altDataLen, const bool& deliveringAltData, - const bool& aApplyConversion, const bool& aIsResolvedByTRR, - const ResourceTimingStruct& aTiming) { + const bool& aApplyConversion, const ResourceTimingStruct& aTiming) { AUTO_PROFILER_LABEL("HttpChannelChild::RecvOnStartRequest", NETWORK); LOG(("HttpChannelChild::RecvOnStartRequest [this=%p]\n", this)); // mFlushedForDiversion and mDivertingToParent should NEVER be set at this @@ -496,8 +493,7 @@ mozilla::ipc::IPCResult HttpChannelChild::RecvOnStartRequest( loadInfoForwarder, isFromCache, cacheEntryAvailable, cacheEntryId, cacheFetchCount, cacheExpirationTime, cachedCharset, securityInfoSerialization, selfAddr, peerAddr, cacheKey, altDataType, - altDataLen, deliveringAltData, aApplyConversion, aIsResolvedByTRR, - aTiming)); + altDataLen, deliveringAltData, aApplyConversion, aTiming)); { // Child's mEventQ is to control the execution order of the IPC messages @@ -531,7 +527,7 @@ void HttpChannelChild::OnStartRequest( const NetAddr& peerAddr, const uint32_t& cacheKey, const nsCString& altDataType, const int64_t& altDataLen, const bool& deliveringAltData, const bool& aApplyConversion, - const bool& aIsResolvedByTRR, const ResourceTimingStruct& aTiming) { + const ResourceTimingStruct& aTiming) { LOG(("HttpChannelChild::OnStartRequest [this=%p]\n", this)); // mFlushedForDiversion and mDivertingToParent should NEVER be set at this @@ -584,7 +580,6 @@ void HttpChannelChild::OnStartRequest( mAvailableCachedAltDataType = altDataType; mDeliveringAltData = deliveringAltData; mAltDataLength = altDataLen; - mResolvedByTRR = aIsResolvedByTRR; SetApplyConversion(aApplyConversion); diff --git a/netwerk/protocol/http/HttpChannelChild.h b/netwerk/protocol/http/HttpChannelChild.h index c78f937e2e6f..bd0a3014ddd4 100644 --- a/netwerk/protocol/http/HttpChannelChild.h +++ b/netwerk/protocol/http/HttpChannelChild.h @@ -145,7 +145,7 @@ class HttpChannelChild final : public PHttpChannelChild, const NetAddr& peerAddr, const int16_t& redirectCount, const uint32_t& cacheKey, const nsCString& altDataType, const int64_t& altDataLen, const bool& deliveringAltData, - const bool& aApplyConversion, const bool& aIsResolvedByTRR, + const bool& aApplyConversion, const ResourceTimingStruct& aTiming) override; mozilla::ipc::IPCResult RecvFailedAsyncOpen(const nsresult& status) override; mozilla::ipc::IPCResult RecvRedirect1Begin( @@ -470,7 +470,7 @@ class HttpChannelChild final : public PHttpChannelChild, const NetAddr& peerAddr, const uint32_t& cacheKey, const nsCString& altDataType, const int64_t& altDataLen, const bool& deliveringAltData, const bool& aApplyConversion, - const bool& aIsResolvedByTRR, const ResourceTimingStruct& aTiming); + const ResourceTimingStruct& aTiming); void MaybeDivertOnData(const nsCString& data, const uint64_t& offset, const uint32_t& count); void OnTransportAndData(const nsresult& channelStatus, const nsresult& status, diff --git a/netwerk/protocol/http/HttpChannelParent.cpp b/netwerk/protocol/http/HttpChannelParent.cpp index d271b4dfc553..ba501da86496 100644 --- a/netwerk/protocol/http/HttpChannelParent.cpp +++ b/netwerk/protocol/http/HttpChannelParent.cpp @@ -1461,9 +1461,6 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) { ResourceTimingStruct timing; GetTimingAttributes(mChannel, timing); - bool isResolvedByTRR = false; - chan->GetIsResolvedByTRR(&isResolvedByTRR); - rv = NS_OK; if (mIPCClosed || !SendOnStartRequest( @@ -1473,7 +1470,7 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) { cacheEntryId, fetchCount, expirationTime, cachedCharset, secInfoSerialization, chan->GetSelfAddr(), chan->GetPeerAddr(), redirectCount, cacheKey, altDataType, altDataLen, deliveringAltData, - applyConversion, isResolvedByTRR, timing)) { + applyConversion, timing)) { rv = NS_ERROR_UNEXPECTED; } requestHead->Exit(); diff --git a/netwerk/protocol/http/PHttpChannel.ipdl b/netwerk/protocol/http/PHttpChannel.ipdl index d2854ce1ad41..a7314d076e8d 100644 --- a/netwerk/protocol/http/PHttpChannel.ipdl +++ b/netwerk/protocol/http/PHttpChannel.ipdl @@ -126,7 +126,6 @@ child: int64_t altDataLength, bool deliveringAltData, bool applyConversion, - bool isResolvedByTRR, ResourceTimingStruct timing); // Used to cancel child channel if we hit errors during creating and diff --git a/netwerk/protocol/http/TunnelUtils.cpp b/netwerk/protocol/http/TunnelUtils.cpp index 231a5452c7ef..834e2866cbb0 100644 --- a/netwerk/protocol/http/TunnelUtils.cpp +++ b/netwerk/protocol/http/TunnelUtils.cpp @@ -1827,14 +1827,6 @@ SocketTransportShim::GetEsniUsed(bool *aEsniUsed) { return NS_ERROR_NOT_IMPLEMENTED; } -NS_IMETHODIMP -SocketTransportShim::ResolvedByTRR(bool *aResolvedByTRR) { - if (mIsWebsocket) { - LOG3(("WARNING: SocketTransportShim::IsTRR %p", this)); - } - return NS_ERROR_NOT_IMPLEMENTED; -} - #define FWD_TS_PTR(fx, ts) \ NS_IMETHODIMP \ SocketTransportShim::fx(ts *arg) { return mWrapped->fx(arg); } diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 99953cad47f0..44c6c7d1cc2d 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -617,7 +617,7 @@ nsresult nsHttpChannel::ContinueOnBeforeConnect(bool aShouldUpgrade, } } - if (mIsTRRServiceChannel) { + if (mTRR) { mCaps |= NS_HTTP_LARGE_KEEPALIVE | NS_HTTP_DISABLE_TRR; } @@ -632,7 +632,7 @@ nsresult nsHttpChannel::ContinueOnBeforeConnect(bool aShouldUpgrade, mConnectionInfo->SetBeConservative((mCaps & NS_HTTP_BE_CONSERVATIVE) || mBeConservative); mConnectionInfo->SetTlsFlags(mTlsFlags); - mConnectionInfo->SetIsTrrServiceChannel(mIsTRRServiceChannel); + mConnectionInfo->SetTrrUsed(mTRR); mConnectionInfo->SetTrrDisabled(mCaps & NS_HTTP_DISABLE_TRR); mConnectionInfo->SetIPv4Disabled(mCaps & NS_HTTP_DISABLE_IPV4); mConnectionInfo->SetIPv6Disabled(mCaps & NS_HTTP_DISABLE_IPV6); @@ -4037,7 +4037,7 @@ nsresult nsHttpChannel::OpenCacheEntryInternal( if (mPostID) { extension.Append(nsPrintfCString("%d", mPostID)); } - if (mIsTRRServiceChannel) { + if (mTRR) { extension.Append("TRR"); } @@ -8429,15 +8429,11 @@ nsHttpChannel::OnTransportStatus(nsITransport *trans, nsresult status, status == NS_NET_STATUS_WAITING_FOR) { if (mTransaction) { mTransaction->GetNetworkAddresses(mSelfAddr, mPeerAddr); - mResolvedByTRR = mTransaction->ResolvedByTRR(); } else { nsCOMPtr socketTransport = do_QueryInterface(trans); if (socketTransport) { socketTransport->GetSelfAddr(&mSelfAddr); socketTransport->GetPeerAddr(&mPeerAddr); - bool isTrr = false; - socketTransport->ResolvedByTRR(&isTrr); - mResolvedByTRR = isTrr; } } } diff --git a/netwerk/protocol/http/nsHttpConnection.cpp b/netwerk/protocol/http/nsHttpConnection.cpp index fe8cae8304a0..fd0afe2ff33e 100644 --- a/netwerk/protocol/http/nsHttpConnection.cpp +++ b/netwerk/protocol/http/nsHttpConnection.cpp @@ -139,7 +139,7 @@ nsHttpConnection::~nsHttpConnection() { } MOZ_ASSERT(ci); - if (ci->GetIsTrrServiceChannel()) { + if (ci->GetTrrUsed()) { Telemetry::Accumulate(Telemetry::DNS_TRR_REQUEST_PER_CONN, mHttp1xTransactionCount); } diff --git a/netwerk/protocol/http/nsHttpConnectionInfo.cpp b/netwerk/protocol/http/nsHttpConnectionInfo.cpp index b5a6dad98c1d..81bf1ae4d7b9 100644 --- a/netwerk/protocol/http/nsHttpConnectionInfo.cpp +++ b/netwerk/protocol/http/nsHttpConnectionInfo.cpp @@ -81,7 +81,7 @@ void nsHttpConnectionInfo::Init(const nsACString &host, int32_t port, mNPNToken = npnToken; mOriginAttributes = originAttributes; mTlsFlags = 0x0; - mIsTrrServiceChannel = false; + mTrrUsed = false; mTrrDisabled = false; mIPv4Disabled = false; mIPv6Disabled = false; @@ -250,7 +250,7 @@ already_AddRefed nsHttpConnectionInfo::Clone() const { clone->SetNoSpdy(GetNoSpdy()); clone->SetBeConservative(GetBeConservative()); clone->SetTlsFlags(GetTlsFlags()); - clone->SetIsTrrServiceChannel(GetIsTrrServiceChannel()); + clone->SetTrrUsed(GetTrrUsed()); clone->SetTrrDisabled(GetTrrDisabled()); clone->SetIPv4Disabled(GetIPv4Disabled()); clone->SetIPv6Disabled(GetIPv6Disabled()); @@ -276,7 +276,7 @@ void nsHttpConnectionInfo::CloneAsDirectRoute(nsHttpConnectionInfo **outCI) { clone->SetNoSpdy(GetNoSpdy()); clone->SetBeConservative(GetBeConservative()); clone->SetTlsFlags(GetTlsFlags()); - clone->SetIsTrrServiceChannel(GetIsTrrServiceChannel()); + clone->SetTrrUsed(GetTrrUsed()); clone->SetTrrDisabled(GetTrrDisabled()); clone->SetIPv4Disabled(GetIPv4Disabled()); clone->SetIPv6Disabled(GetIPv6Disabled()); diff --git a/netwerk/protocol/http/nsHttpConnectionInfo.h b/netwerk/protocol/http/nsHttpConnectionInfo.h index aaa6c632ba09..480a04288c5e 100644 --- a/netwerk/protocol/http/nsHttpConnectionInfo.h +++ b/netwerk/protocol/http/nsHttpConnectionInfo.h @@ -125,12 +125,9 @@ class nsHttpConnectionInfo final : public ARefBase { void SetTlsFlags(uint32_t aTlsFlags); uint32_t GetTlsFlags() const { return mTlsFlags; } - // IsTrrServiceChannel means that this connection is used to send TRR requests - // over - void SetIsTrrServiceChannel(bool aIsTRRChannel) { - mIsTrrServiceChannel = aIsTRRChannel; - } - bool GetIsTrrServiceChannel() const { return mIsTrrServiceChannel; } + // TrrUsed means that this connection is used to send TRR requests over + void SetTrrUsed(bool aUsed) { mTrrUsed = aUsed; } + bool GetTrrUsed() const { return mTrrUsed; } // SetTrrDisabled means don't use TRR to resolve host names for this // connection @@ -198,7 +195,7 @@ class nsHttpConnectionInfo final : public ARefBase { OriginAttributes mOriginAttributes; uint32_t mTlsFlags; - uint16_t mIsTrrServiceChannel : 1; + uint16_t mTrrUsed : 1; uint16_t mTrrDisabled : 1; uint16_t mIPv4Disabled : 1; uint16_t mIPv6Disabled : 1; diff --git a/netwerk/protocol/http/nsHttpTransaction.cpp b/netwerk/protocol/http/nsHttpTransaction.cpp index de571e29bf8c..99516732455d 100644 --- a/netwerk/protocol/http/nsHttpTransaction.cpp +++ b/netwerk/protocol/http/nsHttpTransaction.cpp @@ -143,7 +143,6 @@ nsHttpTransaction::nsHttpTransaction() mPassedRatePacing(false), mSynchronousRatePaceRequest(false), mClassOfService(0), - mResolvedByTRR(false), m0RTTInProgress(false), mDoNotTryEarlyData(false), mEarlyDataDisposition(EARLY_NONE), @@ -592,7 +591,6 @@ void nsHttpTransaction::OnTransportStatus(nsITransport *transport, MutexAutoLock lock(mLock); socketTransport->GetSelfAddr(&mSelfAddr); socketTransport->GetPeerAddr(&mPeerAddr); - socketTransport->ResolvedByTRR(&mResolvedByTRR); } } diff --git a/netwerk/protocol/http/nsHttpTransaction.h b/netwerk/protocol/http/nsHttpTransaction.h index f06d2cc0dcc2..aa70956904aa 100644 --- a/netwerk/protocol/http/nsHttpTransaction.h +++ b/netwerk/protocol/http/nsHttpTransaction.h @@ -464,12 +464,10 @@ class nsHttpTransaction final : public nsAHttpTransaction, public: void GetNetworkAddresses(NetAddr &self, NetAddr &peer); - bool ResolvedByTRR() { return mResolvedByTRR; } private: NetAddr mSelfAddr; NetAddr mPeerAddr; - bool mResolvedByTRR; bool m0RTTInProgress; bool mDoNotTryEarlyData; diff --git a/netwerk/protocol/http/nsIHttpChannelInternal.idl b/netwerk/protocol/http/nsIHttpChannelInternal.idl index aa613d9da304..6f0452071ec9 100644 --- a/netwerk/protocol/http/nsIHttpChannelInternal.idl +++ b/netwerk/protocol/http/nsIHttpChannelInternal.idl @@ -243,14 +243,7 @@ interface nsIHttpChannelInternal : nsISupports * True if channel is used by the internal trusted recursive resolver * This flag places data for the request in a cache segment specific to TRR */ - [noscript, must_use] attribute boolean isTRRServiceChannel; - - /** - * If the channel's remote IP was resolved using TRR. - * Is false for resources loaded from the cache or resources that have an - * IP literal host. - */ - [noscript, must_use] readonly attribute boolean isResolvedByTRR; + [noscript, must_use] attribute boolean trr; /** * An opaque flags for non-standard behavior of the TLS system.