|
|
Chromium Code Reviews|
Created:
4 years, 3 months ago by Ryan Hamilton Modified:
4 years, 3 months ago CC:
chromium-reviews, cbentzel+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionIgnore errors on SetDoNotFragment on platforms where it is not implemented.
BUG=641709
R=ianswett@chromium.org, jri@chromium.org
Committed: https://chromium.googlesource.com/chromium/src/+/8449205b62b9278cb76710b9bbbb06e7a7c07077
Patch Set 1 #
Total comments: 4
Patch Set 2 : Fix enum name #Messages
Total messages: 50 (25 generated)
rch@chromium.org changed reviewers: + ianswett@chromium.org, jri@chromium.org
https://codereview.chromium.org/2287073002/diff/1/net/quic/chromium/quic_stre... File net/quic/chromium/quic_stream_factory.cc (right): https://codereview.chromium.org/2287073002/diff/1/net/quic/chromium/quic_stre... net/quic/chromium/quic_stream_factory.cc:1710: if (rv != OK && rv != ERR_NOT_IMPLEMENTED) { Alternatively, we could ignore ALL errors since it's not crucial if this does not succeed. That being said, from UMA, it's only failing on OS X so this fix is sufficient.
The CQ bit was checked by ianswett@chromium.org
lgtm I'm happy with this approach. Thanks for the quick fix.
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
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
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 commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
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 commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
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 commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
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 commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
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 commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by rch@chromium.org
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 commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
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...
lgtm
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...
mef@chromium.org changed reviewers: + mef@chromium.org
https://codereview.chromium.org/2287073002/diff/1/net/quic/chromium/quic_stre... File net/quic/chromium/quic_stream_factory.cc (right): https://codereview.chromium.org/2287073002/diff/1/net/quic/chromium/quic_stre... net/quic/chromium/quic_stream_factory.cc:1711: HistogramCreateSessionFailure(CREATION_ERROR_SETTING_NO_NOT_FRAGMENT); nit: This should probably be CREATION_ERROR_SETTING_DO_NOT_FRAGMENT (s/_NO_/_DO_/).
The CQ bit was unchecked by jri@chromium.org
The CQ bit was checked by rch@chromium.org
https://codereview.chromium.org/2287073002/diff/1/net/quic/chromium/quic_stre... File net/quic/chromium/quic_stream_factory.cc (right): https://codereview.chromium.org/2287073002/diff/1/net/quic/chromium/quic_stre... net/quic/chromium/quic_stream_factory.cc:1711: HistogramCreateSessionFailure(CREATION_ERROR_SETTING_NO_NOT_FRAGMENT); On 2016/08/29 19:27:14, mef wrote: > nit: This should probably be CREATION_ERROR_SETTING_DO_NOT_FRAGMENT > (s/_NO_/_DO_/). Good point. Will do so in a followup since this has been a bear to land)
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Ignore errors on SetDoNotFragment on platforms where it is not implemented. BUG=641709 ========== to ========== Ignore errors on SetDoNotFragment on platforms where it is not implemented. BUG=641709 R=ianswett@chromium.org, jri@chromium.org Committed: https://crrev.com/8449205b62b9278cb76710b9bbbb06e7a7c07077 Cr-Commit-Position: refs/heads/master@{#415037} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/8449205b62b9278cb76710b9bbbb06e7a7c07077 Cr-Commit-Position: refs/heads/master@{#415037}
Message was sent while issue was closed.
Description was changed from ========== Ignore errors on SetDoNotFragment on platforms where it is not implemented. BUG=641709 R=ianswett@chromium.org, jri@chromium.org Committed: https://crrev.com/8449205b62b9278cb76710b9bbbb06e7a7c07077 Cr-Commit-Position: refs/heads/master@{#415037} ========== to ========== Ignore errors on SetDoNotFragment on platforms where it is not implemented. BUG=641709 R=ianswett@chromium.org, jri@chromium.org Committed: https://chromium.googlesource.com/chromium/src/+/8449205b62b9278cb76710b9bbbb... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as 8449205b62b9278cb76710b9bbbb06e7a7c07077 (presubmit successful).
Message was sent while issue was closed.
https://codereview.chromium.org/2287073002/diff/1/net/quic/chromium/quic_stre... File net/quic/chromium/quic_stream_factory.cc (right): https://codereview.chromium.org/2287073002/diff/1/net/quic/chromium/quic_stre... net/quic/chromium/quic_stream_factory.cc:1711: HistogramCreateSessionFailure(CREATION_ERROR_SETTING_NO_NOT_FRAGMENT); On 2016/08/29 19:42:48, Ryan Hamilton wrote: > On 2016/08/29 19:27:14, mef wrote: > > nit: This should probably be CREATION_ERROR_SETTING_DO_NOT_FRAGMENT > > (s/_NO_/_DO_/). > > Good point. Will do so in a followup since this has been a bear to land) Ok, I gave up on the CQ and landed this manually, so I fixed this. Done. |
