Remove HttpStreamFactoryImpl::JobController::|main_job_is_resumed_| |
||||
Issue descriptionAfter https://chromium-review.googlesource.com/c/chromium/src/+/805354, it's possible to remove |main_job_is_resumed_| because we resume the main job exactly once. Filing a bug to investigate and remove |main_job_is_resumed_|.
,
Dec 4 2017
We currently still need |main_job_is_resumed_|. If the main job is resumed after the delay tcp delay, we will set main_job_is_resumed = true. If after a while, the QUIC job fails, the JobController will attempt to resume the main job with 0 delay. We do not want to make the main job resume twice in this case. |main_job_is_resumed_| keeps track of the state. Removing |main_job_is_resumed_| will make HttpStreamFactoryImplJobControllerTest.DelayedTCP* fail. I don't see a way to fix that without introducing another state. We should probably keep it as is. zhongyi@: thought?
,
Dec 4 2017
This shouldn't block us, you can do something like: when alt job fails, check if main job is waiting, if so, reset any delayed the ResumeMainJob, and post a new ResumeMainJob with 0 delay, do nothing otherwise. This would prevent us from resuming multiple times. As long as we can make sure the main_job_->Resume() is called only once, this would work. To get this: you will need to main the cancel and set for ResumeMainJob, and also make sure the ResumeMainJob is always called when main job is still waiting. WDYT? Could you post which line of DelayedTCP* test would fail? We might have some checks there, you could convert it by checking whether main job is still waiting.
,
Dec 4 2017
I will leave this bug open. I will be happy to see the "delay tcp and race quic" logic to be simplified, but I am not certain if I want to spend more time on this.
,
Mar 14 2018
Closing due to lack of activity.
,
Jul 6
,
Jul 6
|
||||
►
Sign in to add a comment |
||||
Comment 1 by xunji...@chromium.org
, Dec 4 2017