New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 764434 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 762686



Sign in to add a comment

Add a _DEPRECATED suffix to SDCH constants in source_stream_type_list.h

Project Member Reported by xunji...@chromium.org, Sep 12 2017

Issue description

This is to follow up on a review comment to add a _DEPRECATED 
suffix to SDCH constants in source_stream_type_list.h
 

Comment 1 by mmenke@chromium.org, Sep 12 2017

Don't need to do that - can just remove them, unless net-internals has direct references to them.  Net log files have an index of constants, which includes the numeric ID to source type mappings, so a file will always include entries for the sources it contains.
Those are used in UMA histograms. I need to keep those right?
+1 to c#2--I would have suggested removing them, but I think that'd result in the values above those collapsing down to fill the holes.  See https://cs.chromium.org/chromium/src/net/filter/source_stream.h?q=SourceStream&sq=package:chromium&l=25.  

We *could* fix that, and it might be useful to future proof that enum against this problem the next time we fiddle with it.

Comment 4 by mmenke@chromium.org, Sep 13 2017

Ah, you're right.  I was confusing this with net_log_source_type_list.h.  Too many source type lists.  :(
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 15 2017

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

commit 666c068451e48641d613f4470bb7a151b3ca21d8
Author: Helen Li <xunjieli@chromium.org>
Date: Fri Sep 15 19:57:05 2017

Deprecate SDCH-related net::SourceStream types

- Deprecated three SDCH-related net::SourceStream types
- Modify URLRequestHttpJob::SetUpSourceStream() to not include default case

Bug:  764434 ,  764433 ,  762686 
Change-Id: I93320bfee930484da76a0a29fab2224f2b886523
Reviewed-on: https://chromium-review.googlesource.com/667731
Commit-Queue: Helen Li <xunjieli@chromium.org>
Reviewed-by: Randy Smith <rdsmith@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502337}
[modify] https://crrev.com/666c068451e48641d613f4470bb7a151b3ca21d8/net/filter/filter_source_stream.cc
[modify] https://crrev.com/666c068451e48641d613f4470bb7a151b3ca21d8/net/filter/gzip_source_stream.cc
[modify] https://crrev.com/666c068451e48641d613f4470bb7a151b3ca21d8/net/filter/gzip_source_stream.h
[modify] https://crrev.com/666c068451e48641d613f4470bb7a151b3ca21d8/net/filter/gzip_source_stream_fuzzer.cc
[modify] https://crrev.com/666c068451e48641d613f4470bb7a151b3ca21d8/net/filter/gzip_source_stream_unittest.cc
[modify] https://crrev.com/666c068451e48641d613f4470bb7a151b3ca21d8/net/filter/source_stream_type_list.h
[modify] https://crrev.com/666c068451e48641d613f4470bb7a151b3ca21d8/net/url_request/url_request_http_job.cc
[modify] https://crrev.com/666c068451e48641d613f4470bb7a151b3ca21d8/net/url_request/url_request_http_job_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment