Skip to content

Commit

Permalink
Remove 3- and 4-year-old "temporary" DCHECKs for investigating bugs.
Browse files Browse the repository at this point in the history
The removed code CHECKed if NetLogWithSource or socket pool request
objects had been deleted. While the issues were never resolved,
they're also not under active investigation, and the CHECKs did not
prove useful, and we don't seem to seeing them in crashes at the
moment.

Bug: 467797, 652868
Change-Id: I07ae2e12f5e940a5cb8f3b23440f23a12df6680e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1612017
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659594}
  • Loading branch information
Matt Menke authored and Commit Bot committed May 14, 2019
1 parent 9450471 commit ef9916b
Show file tree
Hide file tree
Showing 5 changed files with 2 additions and 54 deletions.
4 changes: 0 additions & 4 deletions net/http/http_network_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -709,11 +709,7 @@ int HttpNetworkTransaction::DoLoop(int result) {
rv = DoCreateStream();
break;
case STATE_CREATE_STREAM_COMPLETE:
// TODO(zhongyi): remove liveness checks when crbug.com/652868 is
// solved.
net_log_.CrashIfInvalid();
rv = DoCreateStreamComplete(rv);
net_log_.CrashIfInvalid();
break;
case STATE_INIT_STREAM:
DCHECK_EQ(OK, rv);
Expand Down
19 changes: 1 addition & 18 deletions net/log/net_log_with_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,10 @@ base::Value BytesTransferredCallback(int byte_count,

} // namespace

NetLogWithSource::~NetLogWithSource() {
liveness_ = DEAD;
}
NetLogWithSource::~NetLogWithSource() {}

void NetLogWithSource::AddEntry(NetLogEventType type,
NetLogEventPhase phase) const {
CrashIfInvalid();

if (!net_log_)
return;
net_log_->AddEntry(type, source_, phase, nullptr);
Expand All @@ -52,8 +48,6 @@ void NetLogWithSource::AddEntry(
NetLogEventType type,
NetLogEventPhase phase,
const NetLogParametersCallback& get_parameters) const {
CrashIfInvalid();

if (!net_log_)
return;
net_log_->AddEntry(type, source_, phase, &get_parameters);
Expand Down Expand Up @@ -116,7 +110,6 @@ void NetLogWithSource::AddByteTransferEvent(NetLogEventType event_type,
}

bool NetLogWithSource::IsCapturing() const {
CrashIfInvalid();
return net_log_ && net_log_->IsCapturing();
}

Expand All @@ -130,14 +123,4 @@ NetLogWithSource NetLogWithSource::Make(NetLog* net_log,
return NetLogWithSource(source, net_log);
}

void NetLogWithSource::CrashIfInvalid() const {
Liveness liveness = liveness_;

if (liveness == ALIVE)
return;

base::debug::Alias(&liveness);
CHECK_EQ(ALIVE, liveness);
}

} // namespace net
12 changes: 0 additions & 12 deletions net/log/net_log_with_source.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,27 +69,15 @@ class NET_EXPORT NetLogWithSource {
// the case of NULL net_log.
static NetLogWithSource Make(NetLog* net_log, NetLogSourceType source_type);

// TODO(eroman): Temporary until crbug.com/467797 is solved.
void CrashIfInvalid() const;

const NetLogSource& source() const { return source_; }
NetLog* net_log() const { return net_log_; }

private:
// TODO(eroman): Temporary until crbug.com/467797 is solved.
enum Liveness {
ALIVE = 0xCA11AB13,
DEAD = 0xDEADBEEF,
};

NetLogWithSource(const NetLogSource& source, NetLog* net_log)
: source_(source), net_log_(net_log) {}

NetLogSource source_;
NetLog* net_log_;

// TODO(eroman): Temporary until crbug.com/467797 is solved.
Liveness liveness_ = ALIVE;
};

} // namespace net
Expand Down
9 changes: 1 addition & 8 deletions net/socket/transport_client_socket_pool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,7 @@ TransportClientSocketPool::Request::Request(
DCHECK_EQ(priority_, MAXIMUM_PRIORITY);
}

TransportClientSocketPool::Request::~Request() {
liveness_ = DEAD;
}
TransportClientSocketPool::Request::~Request() {}

void TransportClientSocketPool::Request::AssignJob(ConnectJob* job) {
DCHECK(job);
Expand All @@ -136,10 +134,6 @@ ConnectJob* TransportClientSocketPool::Request::ReleaseJob() {
return job;
}

void TransportClientSocketPool::Request::CrashIfInvalid() const {
CHECK_EQ(liveness_, ALIVE);
}

TransportClientSocketPool::TransportClientSocketPool(
int max_sockets,
int max_sockets_per_group,
Expand Down Expand Up @@ -1842,7 +1836,6 @@ TransportClientSocketPool::Group::RemoveUnboundRequest(
if (unbound_requests_.empty())
backup_job_timer_.Stop();

request->CrashIfInvalid();
SanityCheck();
return request;
}
Expand Down
12 changes: 0 additions & 12 deletions net/socket/transport_client_socket_pool.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,16 +126,7 @@ class NET_EXPORT_PRIVATE TransportClientSocketPool
// request with a job.
ConnectJob* ReleaseJob();

// TODO(eroman): Temporary until crbug.com/467797 is solved.
void CrashIfInvalid() const;

private:
// TODO(eroman): Temporary until crbug.com/467797 is solved.
enum Liveness {
ALIVE = 0xCA11AB13,
DEAD = 0xDEADBEEF,
};

ClientSocketHandle* const handle_;
CompletionOnceCallback callback_;
const ProxyAuthCallback proxy_auth_callback_;
Expand All @@ -148,9 +139,6 @@ class NET_EXPORT_PRIVATE TransportClientSocketPool
const SocketTag socket_tag_;
ConnectJob* job_;

// TODO(eroman): Temporary until crbug.com/467797 is solved.
Liveness liveness_ = ALIVE;

DISALLOW_COPY_AND_ASSIGN(Request);
};

Expand Down

0 comments on commit ef9916b

Please sign in to comment.