1765652 - Fix netwerk/test/browser/browser_103_telemetry.js with parent controlled loads

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Description

[Andrew McCreight [:mccr8]]

This test fails with browser.tabs.documentchannel.parent-controlled set to true.

netwerk/test/browser/browser_103_telemetry.js
  FAIL unexpected counts should be zero for EH_NUM_OF_HINTS_PER_PAGE at index 0 - 1 == 0 - JS frame :: resource://testing-common/TelemetryTestUtils.jsm :: assertHistogram :: line 302
Stack trace:
resource://testing-common/TelemetryTestUtils.jsm:assertHistogram:302
chrome://mochitests/content/browser/netwerk/test/browser/browser_103_telemetry.js:null:33
  FAIL expected counts should match for EH_NUM_OF_HINTS_PER_PAGE at index 1 - 0 == 1 - JS frame :: resource://testing-common/TelemetryTestUtils.jsm :: assertHistogram :: line 296
Stack trace:
resource://testing-common/TelemetryTestUtils.jsm:assertHistogram:296
chrome://mochitests/content/browser/netwerk/test/browser/browser_103_telemetry.js:null:33
  FAIL unexpected counts should be zero for EH_FINAL_RESPONSE at index 4 - 1 == 0 - JS frame :: resource://testing-common/TelemetryTestUtils.jsm :: assertHistogram :: line 302
Stack trace:
resource://testing-common/TelemetryTestUtils.jsm:assertHistogram:302
chrome://mochitests/content/browser/netwerk/test/browser/browser_103_telemetry.js:null:34
  FAIL Should have found an entry for EH_FINAL_RESPONSE at index 0 - false == true - JS frame :: resource://testing-common/TelemetryTestUtils.jsm :: assertHistogram :: line 309
Stack trace:
resource://testing-common/TelemetryTestUtils.jsm:assertHistogram:309
chrome://mochitests/content/browser/netwerk/test/browser/browser_103_telemetry.js:null:34
  FAIL false == true - JS frame :: chrome://mochitests/content/browser/netwerk/test/browser/browser_103_telemetry.js :: <TOP_LEVEL> :: line 42
Stack trace:
chrome://mochitests/content/browser/netwerk/test/browser/browser_103_telemetry.js:null:42
  FAIL unexpected counts should be zero for EH_FINAL_RESPONSE at index 4 - 1 == 0 - JS frame :: resource://testing-common/TelemetryTestUtils.jsm :: assertHistogram :: line 302
Stack trace:
resource://testing-common/TelemetryTestUtils.jsm:assertHistogram:302
chrome://mochitests/content/browser/netwerk/test/browser/browser_103_telemetry.js:null:92
  FAIL Should have found an entry for EH_FINAL_RESPONSE at index 2 - false == true - JS frame :: resource://testing-common/TelemetryTestUtils.jsm :: assertHistogram :: line 309
Stack trace:
resource://testing-common/TelemetryTestUtils.jsm:assertHistogram:309
chrome://mochitests/content/browser/netwerk/test/browser/browser_103_telemetry.js:null:92

I haven't had a chance to look into it further yet.

Comment 1

[Kershaw Chang [:kershaw]]

Manuel, could you take a look?
Thanks.

Comment 2

[Andrew McCreight [:mccr8]]

Bug 1721217 fixed a bunch of test failures with parent controlled navigation, so perhaps there's an approach that will work for this test in one of those patches. It is possible that this is detecting an actual issue with the navigation change.

Comment 3

[Manuel Bucher [:manuel]]

I think we need to allow concurrent loads if the other load originated from an 103 early hint. So the patch might look similar to https://phabricator.services.mozilla.com/D126846

/docshell/base/CanonicalBrowsingContext.cpp#2184-2187

    // If we want to do a download, don't cancel the current navigation.
    if (!aLoad->IsDownload() && !aLoad->IsEarlyHintPreload()) {
      mCurrentLoad->Cancel(NS_BINDING_CANCELLED_OLD_LOAD);
    }

Comment 4

[Manuel Bucher [:manuel]]

Attachment: Bug 1765652 - Fix early hints still preloading with parent controlled loads r=#necko

Fixes early hint preloads always getting cancelled with
browser.tabs.documentchannel.parent-controlled set to true

Comment 5

[Manuel Bucher [:manuel]]

I don't yet understand what the browser.tabs.documentchannel.parent-controlled true should achieve. Therefore I don't know which way of fixing it is the best. Also why does mCurrentLoad seem to be set, even though it is the first load of the page?

I submitted a patch, which could be reasonable, but I'm not sure if it is correct.

Doing it the way explained in #c3 would also work.

Comment 6

[Manuel Bucher [:manuel]]

@mccr8 I'm going to land this today. So shouldn't block Bug 1746524 anymore. And if landing doesn't work, it would be ok to move on with Bug 1746524. Because this bug currently only disables telemetry.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2401a5039495
Fix early hints still preloading with parent controlled loads r=necko-reviewers,kershaw

Comment 8

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/2401a5039495