Closed Bug 1120652 Opened 6 years ago Closed 6 years ago
Get rid of Fetch
Driver Observer::On Response End()
Since switching to streams everywhere, an 'end' indication is no longer required. It just complicates our worker release.
Whiteboard: [good first bug][lang=C++]
I'd like to work on this bug. I went through the class definition of FetchDriverObserver, but couldn't find a member function OnResponseEnd(). Can you please point me to the relevant code?
(In reply to Tejas Srinivasan from comment #1) > I'd like to work on this bug. I went through the class definition of > FetchDriverObserver, but couldn't find a member function OnResponseEnd(). > Can you please point me to the relevant code? The relevant code will be there once the patches in Bug 1039846 have landed. I will land them today.
The changes have landed. Please go ahead with fixing this. Thanks!
So do I just need to remove the OnResponseEnd() function from the FetchDriverObserver class and its child classes (declaration and definitions)? Is there anything else to do? Do I remove the two function calls as well (, )?  : https://dxr.mozilla.org/mozilla-central/source/dom/fetch/FetchDriver.cpp#475  : https://dxr.mozilla.org/mozilla-central/source/dom/fetch/FetchDriver.cpp#486 Sorry for the noob doubts, I'm kinda new around here.
So after some thinking, we can't really get rid of OnResponseEnd() entirely. On workers, we add a feature to keep the worker alive until the fetch is done. If we remove the feature in OnResponseAvailable() we will end up allowing the worker to terminate before the response is complete. So, let's get rid of MainThreadFetchResolver::OnResponseEnd(), and provide a default empty definition of it in FetchDriverObserver.
(In reply to Nikhil Marathe [:nsm] (needinfo? please) from comment #5) > So, let's get rid of MainThreadFetchResolver::OnResponseEnd(), and provide a default > empty definition of it in FetchDriverObserver. OK, but what about the two function calls? Will any modification be required on that end?
Kindly review the patch I have attached. Thanks.
Attachment #8549429 - Flags: review?(nsm.nikhil)
Attachment #8549429 - Flags: review?(nsm.nikhil) → review+
Assignee: nobody → tejas.srinivasan95
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.