diff options
author | erikchen <erikchen@chromium.org> | 2016-01-08 09:34:23 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-01-08 17:35:16 +0000 |
commit | d1a2d1074fe8c1229cbb7acf418a7da51f10de6b (patch) | |
tree | 19982872bf98513541ce6e5d6416c41c0bc2a1f6 | |
parent | e5767d93e5853caabd143685b968081274c71a0a (diff) | |
download | chromium_src-d1a2d1074fe8c1229cbb7acf418a7da51f10de6b.zip chromium_src-d1a2d1074fe8c1229cbb7acf418a7da51f10de6b.tar.gz chromium_src-d1a2d1074fe8c1229cbb7acf418a7da51f10de6b.tar.bz2 |
content: More paranoid debugging.
This CL creates the debug IPC messages ResourceMsg_SetDataBufferDebug1 and
ResourceMsg_SetDataBufferDebug2, which always precede ResourceMsg_SetDataBuffer.
This goal is to determine whether the HANDLE in ResourceMsg_SetDataBuffer is
being modified in transit.
BUG=527588
Review URL: https://codereview.chromium.org/1561883003
Cr-Commit-Position: refs/heads/master@{#368362}
-rw-r--r-- | content/browser/loader/async_resource_handler.cc | 12 | ||||
-rw-r--r-- | content/child/resource_dispatcher.cc | 36 | ||||
-rw-r--r-- | content/child/resource_dispatcher.h | 11 | ||||
-rw-r--r-- | content/child/resource_dispatcher_unittest.cc | 7 | ||||
-rw-r--r-- | content/common/resource_messages.h | 18 |
5 files changed, 81 insertions, 3 deletions
diff --git a/content/browser/loader/async_resource_handler.cc b/content/browser/loader/async_resource_handler.cc index df08aa7..512ad72 100644 --- a/content/browser/loader/async_resource_handler.cc +++ b/content/browser/loader/async_resource_handler.cc @@ -16,6 +16,7 @@ #include "base/metrics/histogram_macros.h" #include "base/strings/string_number_conversions.h" #include "base/time/time.h" +#include "build/build_config.h" #include "content/browser/devtools/devtools_netlog_observer.h" #include "content/browser/host_zoom_map_impl.h" #include "content/browser/loader/resource_buffer.h" @@ -33,6 +34,10 @@ #include "net/log/net_log.h" #include "net/url_request/redirect_info.h" +#if defined(OS_WIN) +#include <windows.h> +#endif + using base::TimeDelta; using base::TimeTicks; @@ -322,6 +327,13 @@ bool AsyncResourceHandler::OnReadCompleted(int bytes_read, bool* defer) { // TODO(erikchen): Temporary debugging. http://crbug.com/527588. CHECK_LE(size, kBufferSize); +#if defined(OS_WIN) + int handle_int = static_cast<int>(HandleToLong(handle.GetHandle())); + filter->Send( + new ResourceMsg_SetDataBufferDebug1(GetRequestID(), handle_int)); + filter->Send( + new ResourceMsg_SetDataBufferDebug2(GetRequestID(), handle_int + 3)); +#endif filter->Send(new ResourceMsg_SetDataBuffer( GetRequestID(), handle, size, filter->peer_pid())); sent_first_data_msg_ = true; diff --git a/content/child/resource_dispatcher.cc b/content/child/resource_dispatcher.cc index bcb99b9..94cfc79 100644 --- a/content/child/resource_dispatcher.cc +++ b/content/child/resource_dispatcher.cc @@ -40,6 +40,10 @@ #include "net/base/request_priority.h" #include "net/http/http_response_headers.h" +#if defined(OS_WIN) +#include <windows.h> +#endif + namespace content { namespace { @@ -186,6 +190,22 @@ void ResourceDispatcher::OnReceivedCachedMetadata( request_info->peer->OnReceivedCachedMetadata(&data.front(), data.size()); } +#if defined(OS_WIN) +void ResourceDispatcher::OnSetDataBufferDebug1(int request_id, int handle) { + PendingRequestInfo* request_info = GetPendingRequestInfo(request_id); + if (!request_info) + return; + request_info->handle1 = handle; +} + +void ResourceDispatcher::OnSetDataBufferDebug2(int request_id, int handle) { + PendingRequestInfo* request_info = GetPendingRequestInfo(request_id); + if (!request_info) + return; + request_info->handle2 = handle - 3; +} +#endif + void ResourceDispatcher::OnSetDataBuffer(int request_id, base::SharedMemoryHandle shm_handle, int shm_size, @@ -197,6 +217,11 @@ void ResourceDispatcher::OnSetDataBuffer(int request_id, bool shm_valid = base::SharedMemory::IsHandleValid(shm_handle); CHECK((shm_valid && shm_size > 0) || (!shm_valid && !shm_size)); +#if defined(OS_WIN) + int handle_int = static_cast<int>(HandleToLong(shm_handle.GetHandle())); + CHECK(request_info->handle2 != -2 && request_info->handle2 == handle_int); + CHECK(request_info->handle1 != -2 && request_info->handle1 == handle_int); +#endif request_info->buffer.reset( new base::SharedMemory(shm_handle, true)); // read only @@ -536,8 +561,7 @@ ResourceDispatcher::PendingRequestInfo::PendingRequestInfo( frame_origin(frame_origin), response_url(request_url), download_to_file(download_to_file), - request_start(base::TimeTicks::Now()) { -} + request_start(base::TimeTicks::Now()) {} ResourceDispatcher::PendingRequestInfo::~PendingRequestInfo() { if (threaded_data_provider) @@ -551,6 +575,10 @@ void ResourceDispatcher::DispatchMessage(const IPC::Message& message) { IPC_MESSAGE_HANDLER(ResourceMsg_ReceivedCachedMetadata, OnReceivedCachedMetadata) IPC_MESSAGE_HANDLER(ResourceMsg_ReceivedRedirect, OnReceivedRedirect) +#if defined(OS_WIN) + IPC_MESSAGE_HANDLER(ResourceMsg_SetDataBufferDebug1, OnSetDataBufferDebug1) + IPC_MESSAGE_HANDLER(ResourceMsg_SetDataBufferDebug2, OnSetDataBufferDebug2) +#endif IPC_MESSAGE_HANDLER(ResourceMsg_SetDataBuffer, OnSetDataBuffer) IPC_MESSAGE_HANDLER(ResourceMsg_DataReceivedDebug, OnReceivedDataDebug) IPC_MESSAGE_HANDLER(ResourceMsg_DataReceived, OnReceivedData) @@ -735,6 +763,10 @@ bool ResourceDispatcher::IsResourceDispatcherMessage( case ResourceMsg_ReceivedResponse::ID: case ResourceMsg_ReceivedCachedMetadata::ID: case ResourceMsg_ReceivedRedirect::ID: +#if defined(OS_WIN) + case ResourceMsg_SetDataBufferDebug1::ID: + case ResourceMsg_SetDataBufferDebug2::ID: +#endif case ResourceMsg_SetDataBuffer::ID: case ResourceMsg_DataReceivedDebug::ID: case ResourceMsg_DataReceived::ID: diff --git a/content/child/resource_dispatcher.h b/content/child/resource_dispatcher.h index 8f1946e..edd5450 100644 --- a/content/child/resource_dispatcher.h +++ b/content/child/resource_dispatcher.h @@ -21,6 +21,7 @@ #include "base/memory/weak_ptr.h" #include "base/single_thread_task_runner.h" #include "base/time/time.h" +#include "build/build_config.h" #include "content/common/content_export.h" #include "content/public/common/resource_type.h" #include "ipc/ipc_listener.h" @@ -178,6 +179,12 @@ class CONTENT_EXPORT ResourceDispatcher : public IPC::Listener { // Debugging for https://code.google.com/p/chromium/issues/detail?id=527588. int data_offset = -1; +#if defined(OS_WIN) + // This handle is passed through Chrome IPC as a raw int. + int handle1 = -2; + // This handle is passed through Chrome IPC as a raw int + 3. + int handle2 = -2; +#endif }; using PendingRequestMap = std::map<int, scoped_ptr<PendingRequestInfo>>; @@ -195,6 +202,10 @@ class CONTENT_EXPORT ResourceDispatcher : public IPC::Listener { void OnReceivedRedirect(int request_id, const net::RedirectInfo& redirect_info, const ResourceResponseHead& response_head); +#if defined(OS_WIN) + void OnSetDataBufferDebug1(int request_id, int handle); + void OnSetDataBufferDebug2(int request_id, int handle); +#endif void OnSetDataBuffer(int request_id, base::SharedMemoryHandle shm_handle, int shm_size, diff --git a/content/child/resource_dispatcher_unittest.cc b/content/child/resource_dispatcher_unittest.cc index aafa029..f902985 100644 --- a/content/child/resource_dispatcher_unittest.cc +++ b/content/child/resource_dispatcher_unittest.cc @@ -17,6 +17,7 @@ #include "base/process/process_handle.h" #include "base/run_loop.h" #include "base/stl_util.h" +#include "build/build_config.h" #include "content/child/request_extra_data.h" #include "content/child/request_info.h" #include "content/common/appcache_interfaces.h" @@ -298,6 +299,12 @@ class ResourceDispatcherTest : public testing::Test, public IPC::Sender { base::SharedMemoryHandle duplicate_handle; EXPECT_TRUE(shared_memory->ShareToProcess(base::GetCurrentProcessHandle(), &duplicate_handle)); +#if defined(OS_WIN) + EXPECT_TRUE(dispatcher_.OnMessageReceived(ResourceMsg_SetDataBufferDebug1( + request_id, HandleToLong(duplicate_handle.GetHandle())))); + EXPECT_TRUE(dispatcher_.OnMessageReceived(ResourceMsg_SetDataBufferDebug2( + request_id, HandleToLong(duplicate_handle.GetHandle()) + 3))); +#endif EXPECT_TRUE(dispatcher_.OnMessageReceived( ResourceMsg_SetDataBuffer(request_id, duplicate_handle, shared_memory->requested_size(), 0))); diff --git a/content/common/resource_messages.h b/content/common/resource_messages.h index cc0b3d0..918afc1 100644 --- a/content/common/resource_messages.h +++ b/content/common/resource_messages.h @@ -12,6 +12,7 @@ #include "base/memory/shared_memory.h" #include "base/process/process.h" +#include "build/build_config.h" #include "content/common/content_param_traits_macros.h" #include "content/common/navigation_params.h" #include "content/common/resource_request_body.h" @@ -333,6 +334,22 @@ IPC_MESSAGE_CONTROL3(ResourceMsg_ReceivedRedirect, net::RedirectInfo /* redirect_info */, content::ResourceResponseHead) +#if defined(OS_WIN) +// A message that always precedes ResourceMsg_SetDataBuffer. |shm_handle| is the +// underlying HANDLE of base::SharedMemoryHandle converted to an int. Exists to +// help debug https://code.google.com/p/chromium/issues/detail?id=527588. +IPC_MESSAGE_CONTROL2(ResourceMsg_SetDataBufferDebug1, + int /* request_id */, + int /* shm_handle */) + +// A message that always precedes ResourceMsg_SetDataBuffer. |shm_handle| is the +// underlying HANDLE of base::SharedMemoryHandle converted to an int + 3. Exists +// to help debug https://code.google.com/p/chromium/issues/detail?id=527588. +IPC_MESSAGE_CONTROL2(ResourceMsg_SetDataBufferDebug2, + int /* request_id */, + int /* shm_handle */) +#endif + // Sent to set the shared memory buffer to be used to transmit response data to // the renderer. Subsequent DataReceived messages refer to byte ranges in the // shared memory buffer. The shared memory buffer should be retained by the @@ -343,7 +360,6 @@ IPC_MESSAGE_CONTROL3(ResourceMsg_ReceivedRedirect, // // TODO(darin): The |renderer_pid| parameter is just a temporary parameter, // added to help in debugging crbug/160401. -// IPC_MESSAGE_CONTROL4(ResourceMsg_SetDataBuffer, int /* request_id */, base::SharedMemoryHandle /* shm_handle */, |