Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2865893002:
Buffer initial response headers in QuicChromiumClientStream (Closed)
|
Created:
3 years, 7 months ago by Ryan Hamilton Modified:
3 years, 7 months ago Reviewers:
Buck CC:
chromium-reviews, cbentzel+watch_chromium.org, net-reviews_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionBuffer initial response headers in QuicChromiumClientStream
if headers arrive on a pushed stream before the delegate is set.
Eliminate the "delegate tasks" member. It turns out that there can
be at most one delegate task, because OnDataAvailable is only
invoked on the delegate after the headers are delivered to it. So
only OnHeadersAvailable was ever buffered. This CL just makes that
explicit.
Review-Url: https://codereview.chromium.org/2865893002
Cr-Commit-Position: refs/heads/master@{#470226}
Committed: https://chromium.googlesource.com/chromium/src/+/6788ad5e2f48d4228195884715f1809e2922e3ac
Patch Set 1 #Patch Set 2 : Rebase #
Depends on Patchset: Dependent Patchsets: Messages
Total messages: 30 (23 generated)
Description was changed from ========== Buffer initial response headesr in QuicChromiumClientStream if headers arrive on a pushed stream before the delegate is set. Eliminate the "delegate tasks" member. It turns out that there can be at most one delegate task, because OnDataAvailable is only invoked on the delegate after the headers are delivered to it. So only OnHeadersAvailable was ever buffered. This CL just makes that explicit. ========== to ========== Buffer initial response headers in QuicChromiumClientStream if headers arrive on a pushed stream before the delegate is set. Eliminate the "delegate tasks" member. It turns out that there can be at most one delegate task, because OnDataAvailable is only invoked on the delegate after the headers are delivered to it. So only OnHeadersAvailable was ever buffered. This CL just makes that explicit. ==========
The CQ bit was checked by rch@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
rch@chromium.org changed reviewers: + ckrasic@chromium.org
The CQ bit was checked by rch@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by rch@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by rch@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
lgtm Nice cleanup.
lgtm Nice cleanup.
The CQ bit was checked by rch@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by rch@chromium.org
The CQ bit was checked by rch@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by rch@chromium.org
The CQ bit was checked by rch@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494301794353470,
"parent_rev": "5e3734d5c1970621238dc4b5fa3d47a8fdd0b090", "commit_rev":
"6788ad5e2f48d4228195884715f1809e2922e3ac"}
Message was sent while issue was closed.
Description was changed from ========== Buffer initial response headers in QuicChromiumClientStream if headers arrive on a pushed stream before the delegate is set. Eliminate the "delegate tasks" member. It turns out that there can be at most one delegate task, because OnDataAvailable is only invoked on the delegate after the headers are delivered to it. So only OnHeadersAvailable was ever buffered. This CL just makes that explicit. ========== to ========== Buffer initial response headers in QuicChromiumClientStream if headers arrive on a pushed stream before the delegate is set. Eliminate the "delegate tasks" member. It turns out that there can be at most one delegate task, because OnDataAvailable is only invoked on the delegate after the headers are delivered to it. So only OnHeadersAvailable was ever buffered. This CL just makes that explicit. Review-Url: https://codereview.chromium.org/2865893002 Cr-Commit-Position: refs/heads/master@{#470226} Committed: https://chromium.googlesource.com/chromium/src/+/6788ad5e2f48d4228195884715f1... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/6788ad5e2f48d4228195884715f1... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
