Change XHRContext and resource timing information to use request URL

[sreeise]

Change resource timing information to return the URL used to create the XHR and store the URL in XHRContext.

---

• ./mach build -d does not report any errors
• ./mach test-tidy does not report any errors
• These changes fix #23329 (GitHub issue number if applicable)

• There are tests for these changes OR
• These changes do not require tests because ___

---

This change is 

[highfive]

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @ferjm (or someone else) soon.

[highfive]

Heads up! This PR modifies the following files:

• @asajeffrey: components/script/dom/xmlhttprequest.rs
• @KiChjang: components/script/dom/xmlhttprequest.rs

[highfive]

Warning

• These commits modify script code, but no tests are modified. Please consider adding a test!

[jdm]

@bors-servo try=wpt

[bors-servo]

⌛ Trying commit a61311a with merge 2292be6...

[bors-servo]

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
State: approved= try=True

[jdm]

Looks like we should write a test for this. I suspect there are already some test cases that exercise this, but they're probably part of tests that fail in other ways so it's hard to notice if this is correct or not. We should add a new one to tests/wpt/mozilla/tests/mozilla that makes an XMLHttpRequest. When the XHR succeeds, we can call window.performance.getEntriesByType("xmlhttprequest") and verify that there is one entry that has the correct URL.

[sreeise]

Looks like we should write a test for this. I suspect there are already some test cases that exercise this, but they're probably part of tests that fail in other ways so it's hard to notice if this is correct or not. We should add a new one to tests/wpt/mozilla/tests/mozilla that makes an XMLHttpRequest. When the XHR succeeds, we can call window.performance.getEntriesByType("xmlhttprequest") and verify that there is one entry that has the correct URL.

In the test the entries are taken from window.performance.getEntriesByType("resource"). I thought that there were only specific names that could be used for getEntriesByType without using performance.mark("name"). I say that just because you wrote getEntriesBy("xmlhttprequest") so I wasn't sure.

[jdm]

Er, yes. I was thinking of code like performance.getEntriesByType("resource").filter(entry => entry.initiatorType == "xmlhttprequest"); and got confused.

[jdm]

No need to wait for onload here. We can write this test like this:

async_test(function(t) {
  let href = window.location.href;
  let request = new XMLHttpRequest();
  let url = resolve("./test.txt");
  request.open('GET', url, true);

  request.onload = t.step_func_done(function() {
    let entries = window.performance.getEntriesByType("resource");
    assert_equals(entries.length, 1);
    assert_equals(entries[0].name, href);
  });
}, "Performance entries should contain the URL where the XMLHttpRequest originated");

I'm a bit surprised that the test passes right now, since I would expect the test harness <script> elements to be included in the resulting performance entries :<

[sreeise]

Hmm, I'm not sure why it doesn't show the performance entries for script elements. Calling getEntries() only shows the new xmlhttprequest as well. I am wondering whether it would be better to clear out the resource entries beforehand just in case: window.performance.clearResourceTimings();.
Or, maybe get the names of the entries first and then test for the new entry, Something like:

   let entries = window.performance.getEntriesByType("resource");
   let nameEntries = [];
   entries.forEach(entry => {
       nameEntries.push(entry.name);
   });
   assert_true(nameEntries.includes(href));

As far as the entries for the script elements, again I am not sure why but I can try to find out what may be the issue there.

[jdm]

I would rather have the test yell at us if we change the implementation in a way that makes it no longer report expected results. Let's merge it as-is.

[jdm]

Thanks for writing the test! Sorry about the delay in reviewing it.

[jdm]

@bors-servo try=wpt

[bors-servo]

⌛ Trying commit e169278 with merge f42db56...

[jdm]

@bors-servo try-

[jdm]

@bors-servo r+

[bors-servo]

📌 Commit e169278 has been approved by jdm

[bors-servo]

⌛ Testing commit e169278 with merge b59ee42...

[bors-servo]

💔 Test failed - status-taskcluster

[jdm]

@bors-servo retry

• network failure

[bors-servo]

⌛ Testing commit e169278 with merge 707a319...

[bors-servo]

💔 Test failed - status-taskcluster

[jdm]

@bors-servo retry

• #23290

[bors-servo]

⌛ Testing commit e169278 with merge 7f7eead...

[bors-servo]

☀️ Test successful - arm64, linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: jdm
Pushing 7f7eead to master...