Commit d0570f0a authored by Zhongyi Shi's avatar Zhongyi Shi Committed by Commit Bot

Revert "Add instrumentation to investigate a possible UAF."

This reverts commit 68ddeefb.

Reason for revert: crbug.com/901501 has been merged to crbug.com/986211, which is fixed.

Original change's description:
> Add instrumentation to investigate a possible UAF.
>
> Bug: 901501
> Change-Id: Ic778c96c1bb10a9f4bb2f8f3efd883e452bc9742
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1606770
> Auto-Submit: Zhongyi Shi <zhongyi@chromium.org>
> Commit-Queue: Ryan Hamilton <rch@chromium.org>
> Reviewed-by: Ryan Hamilton <rch@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#658861}

TBR=rch@chromium.org,zhongyi@chromium.org

Bug: 901501
Change-Id: I84096ef16edfd37210a7c02f4c1c523dd5d76c57
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1856724Reviewed-by: default avatarRyan Hamilton <rch@chromium.org>
Commit-Queue: Zhongyi Shi <zhongyi@chromium.org>
Commit-Queue: Ryan Hamilton <rch@chromium.org>
Auto-Submit: Zhongyi Shi <zhongyi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705246}
parent b00aaaba
...@@ -10,9 +10,6 @@ ...@@ -10,9 +10,6 @@
#include <utility> #include <utility>
#include "base/bind.h" #include "base/bind.h"
#ifdef TEMP_INSTRUMENTATION_901501
#include "base/debug/alias.h"
#endif
#include "base/location.h" #include "base/location.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
...@@ -130,14 +127,6 @@ SpdyHttpStream::~SpdyHttpStream() { ...@@ -130,14 +127,6 @@ SpdyHttpStream::~SpdyHttpStream() {
stream_->DetachDelegate(); stream_->DetachDelegate();
DCHECK(!stream_); DCHECK(!stream_);
} }
#ifdef TEMP_INSTRUMENTATION_901501
liveness_ = DEAD;
stack_trace_ = base::debug::StackTrace();
// Probably not necessary, but just in case compiler tries to optimize out the
// writes to liveness_ and stack_trace_.
base::debug::Alias(&liveness_);
base::debug::Alias(&stack_trace_);
#endif
} }
int SpdyHttpStream::InitializeStream(const HttpRequestInfo* request_info, int SpdyHttpStream::InitializeStream(const HttpRequestInfo* request_info,
...@@ -455,8 +444,7 @@ void SpdyHttpStream::OnDataSent() { ...@@ -455,8 +444,7 @@ void SpdyHttpStream::OnDataSent() {
void SpdyHttpStream::OnTrailers(const spdy::SpdyHeaderBlock& trailers) {} void SpdyHttpStream::OnTrailers(const spdy::SpdyHeaderBlock& trailers) {}
void SpdyHttpStream::OnClose(int status) { void SpdyHttpStream::OnClose(int status) {
CHECK(stream_); DCHECK(stream_);
CrashIfInvalid();
// Cancel any pending reads from the upload data stream. // Cancel any pending reads from the upload data stream.
if (request_info_ && request_info_->upload_data_stream) if (request_info_ && request_info_->upload_data_stream)
...@@ -480,8 +468,6 @@ void SpdyHttpStream::OnClose(int status) { ...@@ -480,8 +468,6 @@ void SpdyHttpStream::OnClose(int status) {
return; return;
} }
CrashIfInvalid();
if (status == OK) { if (status == OK) {
// We need to complete any pending buffered read now. // We need to complete any pending buffered read now.
DoBufferedReadCallback(); DoBufferedReadCallback();
...@@ -489,8 +475,6 @@ void SpdyHttpStream::OnClose(int status) { ...@@ -489,8 +475,6 @@ void SpdyHttpStream::OnClose(int status) {
return; return;
} }
CrashIfInvalid();
if (!response_callback_.is_null()) { if (!response_callback_.is_null()) {
DoResponseCallback(status); DoResponseCallback(status);
} }
...@@ -607,8 +591,6 @@ bool SpdyHttpStream::ShouldWaitForMoreBufferedData() const { ...@@ -607,8 +591,6 @@ bool SpdyHttpStream::ShouldWaitForMoreBufferedData() const {
} }
void SpdyHttpStream::DoBufferedReadCallback() { void SpdyHttpStream::DoBufferedReadCallback() {
CrashIfInvalid();
buffered_read_callback_pending_ = false; buffered_read_callback_pending_ = false;
// If the transaction is cancelled or errored out, we don't need to complete // If the transaction is cancelled or errored out, we don't need to complete
...@@ -630,8 +612,6 @@ void SpdyHttpStream::DoBufferedReadCallback() { ...@@ -630,8 +612,6 @@ void SpdyHttpStream::DoBufferedReadCallback() {
if (!user_buffer_.get()) if (!user_buffer_.get())
return; return;
CrashIfInvalid();
if (!response_body_queue_.IsEmpty()) { if (!response_body_queue_.IsEmpty()) {
int rv = int rv =
response_body_queue_.Dequeue(user_buffer_->data(), user_buffer_len_); response_body_queue_.Dequeue(user_buffer_->data(), user_buffer_len_);
...@@ -694,22 +674,4 @@ void SpdyHttpStream::SetPriority(RequestPriority priority) { ...@@ -694,22 +674,4 @@ void SpdyHttpStream::SetPriority(RequestPriority priority) {
} }
} }
void SpdyHttpStream::CrashIfInvalid() const {
#ifdef TEMP_INSTRUMENTATION_901501
Liveness liveness = liveness_;
if (liveness == ALIVE)
return;
// Copy relevant variables onto the stack to guarantee they will be available
// in minidumps, and then crash.
base::debug::StackTrace stack_trace = stack_trace_;
base::debug::Alias(&liveness);
base::debug::Alias(&stack_trace);
CHECK_EQ(ALIVE, liveness);
#endif
}
} // namespace net } // namespace net
...@@ -10,13 +10,6 @@ ...@@ -10,13 +10,6 @@
#include <list> #include <list>
#include <memory> #include <memory>
#include "build/build_config.h"
// TODO(zhongyi): Temporary while investigating http://crbug.com/901501.
#ifndef OS_NACL
#define TEMP_INSTRUMENTATION_901501
#include "base/debug/stack_trace.h"
#endif
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
...@@ -103,14 +96,6 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate, ...@@ -103,14 +96,6 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate,
NetLogSource source_dependency() const override; NetLogSource source_dependency() const override;
private: private:
#ifdef TEMP_INSTRUMENTATION_901501
// TODO(zhongyi): Temporary while investigating http://crbug.com/901501.
enum Liveness {
ALIVE = 0xCA11AB13,
DEAD = 0xDEADBEEF,
};
#endif
// Helper function used to initialize private members and to set delegate on // Helper function used to initialize private members and to set delegate on
// stream when stream is created. // stream when stream is created.
void InitializeStreamHelper(); void InitializeStreamHelper();
...@@ -151,9 +136,6 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate, ...@@ -151,9 +136,6 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate,
void DoBufferedReadCallback(); void DoBufferedReadCallback();
bool ShouldWaitForMoreBufferedData() const; bool ShouldWaitForMoreBufferedData() const;
// TODO(zhongyi): Temporary while investigating http://crbug.com/901501.
void CrashIfInvalid() const;
const base::WeakPtr<SpdySession> spdy_session_; const base::WeakPtr<SpdySession> spdy_session_;
// The ID of the pushed stream if one is claimed by this request. // The ID of the pushed stream if one is claimed by this request.
...@@ -227,12 +209,6 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate, ...@@ -227,12 +209,6 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate,
bool was_alpn_negotiated_; bool was_alpn_negotiated_;
#ifdef TEMP_INSTRUMENTATION_901501
// TODO(zhongyi): Temporary while investigating http://crbug.com/901501.
Liveness liveness_ = ALIVE;
base::debug::StackTrace stack_trace_;
#endif
base::WeakPtrFactory<SpdyHttpStream> weak_factory_{this}; base::WeakPtrFactory<SpdyHttpStream> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(SpdyHttpStream); DISALLOW_COPY_AND_ASSIGN(SpdyHttpStream);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment