New issue
Advanced search Search tips

Issue 751111 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Convert HttpCache::DoneWithResponseHeaders CHECKS to DCHECKS

Project Member Reported by shivanisha@chromium.org, Aug 1 2017

Issue description

Convert HttpCache::DoneWithResponseHeaders CHECKS to DCHECKS.

Only one assert is failing in the field now:
CHECK(is_partial || (!entry->writer && entry->done_headers_queue.empty()))

which is being tracked in  crbug.com/750725

Rest can be safely converted to DCHECKs.
 
Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f4cadce96567442121b9d97d2b4c733f05dd59c2

commit f4cadce96567442121b9d97d2b4c733f05dd59c2
Author: Shivani Sharma <shivanisha@chromium.org>
Date: Tue Aug 01 18:01:22 2017

Convert CHECK to DCHECK
While working on the actual fix, converting these to DCHECKS 

Only one assert is failing in the field now:
CHECK(is_partial || (!entry->writer && entry->done_headers_queue.empty()))

which is being tracked in  crbug.com/750725

Rest can be safely converted to DCHECKs.

Bug:  751111 
Change-Id: I5d7bb011115248ff101c3f9be180f1e9bb55109b
Reviewed-on: https://chromium-review.googlesource.com/596187
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: Shivani Sharma <shivanisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491051}
[modify] https://crrev.com/f4cadce96567442121b9d97d2b4c733f05dd59c2/net/http/http_cache.cc

Labels: -Pri-3 Merge-Request-61 OS-All Pri-2
Project Member

Comment 4 by sheriffbot@chromium.org, Aug 4 2017

Labels: -Merge-Request-61 Hotlist-Merge-Approved Merge-Approved-61
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 5 by sheriffbot@chromium.org, Aug 7 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 7 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab752ece07024f6a9a5a80c8c73992d6f109e32a

commit ab752ece07024f6a9a5a80c8c73992d6f109e32a
Author: Shivani Sharma <shivanisha@chromium.org>
Date: Mon Aug 07 17:34:58 2017

Convert CHECK to DCHECK While working on the actual fix, converting these to DCHECKS

Only one assert is failing in the field now:
CHECK(is_partial || (!entry->writer && entry->done_headers_queue.empty()))

which is being tracked in  crbug.com/750725

Rest can be safely converted to DCHECKs.

(cherry picked from commit f4cadce96567442121b9d97d2b4c733f05dd59c2)
TBR=jkarlin@chromium.org

Bug:  751111 
Change-Id: I5d7bb011115248ff101c3f9be180f1e9bb55109b
Reviewed-on: https://chromium-review.googlesource.com/596187
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: Shivani Sharma <shivanisha@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#491051}
Reviewed-on: https://chromium-review.googlesource.com/604112
Reviewed-by: Shivani Sharma <shivanisha@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#357}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/ab752ece07024f6a9a5a80c8c73992d6f109e32a/net/http/http_cache.cc

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c46134a6372b27b3ed67715706c461350a170480

commit c46134a6372b27b3ed67715706c461350a170480
Author: Shivani Sharma <shivanisha@chromium.org>
Date: Tue Sep 19 17:46:22 2017

Convert Check to Dcheck

Bug:  751111 
Change-Id: I03fc65fd2908018620b40eafb10f800d283233de
Reviewed-on: https://chromium-review.googlesource.com/673087
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: Shivani Sharma <shivanisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502883}
[modify] https://crrev.com/c46134a6372b27b3ed67715706c461350a170480/net/http/http_cache.cc

Labels: Merge-Request-62
Project Member

Comment 10 by sheriffbot@chromium.org, Sep 20 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: M62 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-62 Merge-Approved-62
Thanks! approving merge to M62 (branch: 3202)
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 25 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/20d81fd3a170087e87dbafc82c87ca57d9c60fe3

commit 20d81fd3a170087e87dbafc82c87ca57d9c60fe3
Author: Shivani Sharma <shivanisha@chromium.org>
Date: Mon Sep 25 13:30:13 2017

Convert Check to Dcheck

Bug:  751111 
Change-Id: I03fc65fd2908018620b40eafb10f800d283233de
Reviewed-on: https://chromium-review.googlesource.com/673087
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: Shivani Sharma <shivanisha@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#502883}(cherry picked from commit c46134a6372b27b3ed67715706c461350a170480)
Reviewed-on: https://chromium-review.googlesource.com/681774
Reviewed-by: Shivani Sharma <shivanisha@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#424}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/20d81fd3a170087e87dbafc82c87ca57d9c60fe3/net/http/http_cache.cc

Sign in to add a comment