Closed Bug 1384506 Opened 3 years ago Closed 3 years ago

dom/base/test/test_urgent_start.html fails when we enable rcwn

Categories

(Core :: Networking: Cache, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: junior, Assigned: junior)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch addObserver (obsolete) — Splinter Review
RCWN would break the test by the following command

mach mochitest --setpref network.http.rcwn.enabled=true dom/base/test/test_urgent_start.html

The reason is the test rely on topic "http-on-examine-response" and "http-on-examine-cached-response"

However, RCWN won't notify both topics if we race.

I found that listening to "http-on-modify-request" could solve the problem.
Is it a reasonable solution, michal?
Flags: needinfo?(michal.novotny)
The test already listens to "http-on-modify-request". I don't understand the test. Honza, you did the review, so why the test expects the entry from the cache? Is that essential for testing urgent-start marking problem? If not, the test could be changed so it avoids using the cache.
Flags: needinfo?(michal.novotny) → needinfo?(honzab.moz)
(In reply to Michal Novotny (:michal) from comment #1)
> The test already listens to "http-on-modify-request". I don't understand the
> test. Honza, you did the review, so why the test expects the entry from the
> cache? Is that essential for testing urgent-start marking problem? If not,
> the test could be changed so it avoids using the cache.

I wrote this test to check the urgent-start flag is marked on the channel. 

I guess you are saying already listening to "http-on-examine-response"?

I send a request multiple times to verify it only marked as urgent-start when it is triggered by user input events. I believe the Necko might cache the response so that I can get the channel by listening to "http-on-examine-cached-response".
Honestly I don't understand the test enough to give a feedback here.  Maybe just disable rcwn for this test?  It tests something else than cache anyway.
Flags: needinfo?(honzab.moz)
disable by last comment
Assignee: nobody → juhsu
Attachment #8890281 - Attachment is obsolete: true
Attachment #8892724 - Flags: review?(ttung)
Comment on attachment 8892724 [details] [diff] [review]
disableRcwnDomUrgentStart, v1

lgtm, thanks!
Attachment #8892724 - Flags: review?(ttung) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d03602f141eb
Disable rcwn in test_urgent_start.html. r=tt
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d03602f141eb
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.