I didn't mean that to be such a personal flame, we need to have some better guards to prevent this from happening. It's just frustrating when the tree is broken for hours at a time. It's happened a lot lately, the tree has been closed and/or hosed for the majority of my workday week. The sheriff wakes up, reverts everything closes the tree, nurses it back to heath, and then it happens again the next morning. It's not really a good strategy.
Maybe we need to limit the hours people are commit, if a sheriff isn't around to fix things? I don't really think that's a good strategy either. I don't know. On Thu, Mar 5, 2009 at 5:34 PM, Dean McNamee <de...@chromium.org> wrote: > I just verified that this patch completely broke everything. You > committed late at night when no sheriff or anyone was around, and then > you went to bed. > > Thanks for not testing your code, and not watching the tree go red after it. > > Appreciation from another time zone, > -- dean > > On Thu, Mar 5, 2009 at 7:38 AM, <hc...@chromium.org> wrote: >> >> Author: hc...@chromium.org >> Date: Wed Mar 4 22:38:52 2009 >> New Revision: 10972 >> >> Log: >> Highlights of changes: >> 1. Added entry to ResourceResponseHead so that it contains >> either a base::PlatformFile (OS_WIN) or >> base::FileDescriptor (OS_POSIX) for passing the file >> handle from browser to renderer process. >> 2. Also added IPC messages for reporting download progress >> and ACK message for it. ResourceLoaderBridge::Peer::OnDownloadProgress >> is added so that the peer is notified of the download >> progress in the renderer process. >> 3. Load flag to kick start the resource loading for media >> files. LOAD_MEDIA_RESOURCE is added so that >> ResourceDispatcherHost knows how to use a different >> ResourceHandler for handling media resource request. >> >> Review URL: http://codereview.chromium.org/27168 >> >> Modified: >> trunk/src/chrome/browser/renderer_host/resource_dispatcher_host.cc >> trunk/src/chrome/browser/renderer_host/resource_handler.h >> trunk/src/chrome/common/render_messages.h >> trunk/src/chrome/common/render_messages_internal.h >> trunk/src/chrome/common/resource_dispatcher.cc >> trunk/src/chrome/common/resource_dispatcher.h >> trunk/src/net/base/load_flags.h >> trunk/src/net/http/http_cache.cc >> trunk/src/net/http/http_cache_unittest.cc >> trunk/src/net/url_request/url_request.h >> trunk/src/webkit/glue/resource_loader_bridge.h >> >> Modified: trunk/src/chrome/browser/renderer_host/resource_dispatcher_host.cc >> ============================================================================== >> --- trunk/src/chrome/browser/renderer_host/resource_dispatcher_host.cc >> (original) >> +++ trunk/src/chrome/browser/renderer_host/resource_dispatcher_host.cc Wed >> Mar 4 22:38:52 2009 >> @@ -815,6 +815,15 @@ >> response->response_head.content_length = request->GetExpectedContentSize(); >> request->GetMimeType(&response->response_head.mime_type); >> >> + // Structure of ResourceResponseHead depends on the platform, so we do it >> + // differently. >> +#if defined(OS_POSIX) >> + response->response_head.response_data_file.fd = >> request->response_data_file(); >> + response->response_head.response_data_file.auto_close = false; >> +#elif defined(OS_WIN) >> + response->response_head.response_data_file = >> request->response_data_file(); >> +#endif >> + >> if (request->ssl_info().cert) { >> int cert_id = >> CertStore::GetSharedInstance()->StoreCert( >> >> Modified: trunk/src/chrome/browser/renderer_host/resource_handler.h >> ============================================================================== >> --- trunk/src/chrome/browser/renderer_host/resource_handler.h (original) >> +++ trunk/src/chrome/browser/renderer_host/resource_handler.h Wed Mar 4 >> 22:38:52 2009 >> @@ -12,6 +12,11 @@ >> #ifndef CHROME_BROWSER_RENDERER_HOST_RESOURCE_HANDLER_H_ >> #define CHROME_BROWSER_RENDERER_HOST_RESOURCE_HANDLER_H_ >> >> +#include "build/build_config.h" >> +#if defined(OS_POSIX) >> +#include "base/file_descriptor_posix.h" >> +#endif >> +#include "base/platform_file.h" >> #include "chrome/common/filter_policy.h" >> #include "net/url_request/url_request_status.h" >> #include "webkit/glue/resource_loader_bridge.h" >> @@ -29,6 +34,20 @@ >> // Specifies if the resource should be filtered before being displayed >> // (insecure resources can be filtered to keep the page secure). >> FilterPolicy::Type filter_policy; >> + >> + // A platform specific handle for a file that carries response data. This >> + // entry is used if the resource request is of type ResourceType::MEDIA >> and >> + // the underlying cache layer keeps the response data in a standalone >> file. >> +#if defined(OS_POSIX) >> + // If the response data file is available, the file handle is stored in >> + // response_data_file.fd, its value is base::kInvalidPlatformFileValue >> + // otherwise. >> + base::FileDescriptor response_data_file; >> +#elif defined(OS_WIN) >> + // An asynchronous file handle to the response data file, its value is >> + // base::kInvalidPlatformFileValue if the file is not available. >> + base::PlatformFile response_data_file; >> +#endif >> }; >> >> // Parameters for a synchronous resource response. >> >> Modified: trunk/src/chrome/common/render_messages.h >> ============================================================================== >> --- trunk/src/chrome/common/render_messages.h (original) >> +++ trunk/src/chrome/common/render_messages.h Wed Mar 4 22:38:52 2009 >> @@ -377,6 +377,9 @@ >> case ResourceType::OBJECT: >> type = L"OBJECT"; >> break; >> + case ResourceType::MEDIA: >> + type = L"MEDIA"; >> + break; >> default: >> type = L"UNKNOWN"; >> break; >> @@ -1334,12 +1337,14 @@ >> ParamTraits<webkit_glue::ResourceLoaderBridge::ResponseInfo>::Write(m, >> p); >> WriteParam(m, p.status); >> WriteParam(m, p.filter_policy); >> + WriteParam(m, p.response_data_file); >> } >> static bool Read(const Message* m, void** iter, param_type* r) { >> return >> ParamTraits<webkit_glue::ResourceLoaderBridge::ResponseInfo>::Read(m, >> iter, r) && >> ReadParam(m, iter, &r->status) && >> - ReadParam(m, iter, &r->filter_policy); >> + ReadParam(m, iter, &r->filter_policy) && >> + ReadParam(m, iter, &r->response_data_file); >> } >> static void Log(const param_type& p, std::wstring* l) { >> // log more? >> >> Modified: trunk/src/chrome/common/render_messages_internal.h >> ============================================================================== >> --- trunk/src/chrome/common/render_messages_internal.h (original) >> +++ trunk/src/chrome/common/render_messages_internal.h Wed Mar 4 22:38:52 >> 2009 >> @@ -175,7 +175,14 @@ >> int /* request_id */, >> ResourceResponseHead) >> >> - // Sent as upload progress is being made >> + // Sent as download progress is being made, size of the resource may be >> + // unknown, in that case |size| is -1. >> + IPC_MESSAGE_ROUTED3(ViewMsg_Resource_DownloadProgress, >> + int /* request_id */, >> + int64 /* position */, >> + int64 /* size */) >> + >> + // Sent as upload progress is being made. >> IPC_MESSAGE_ROUTED3(ViewMsg_Resource_UploadProgress, >> int /* request_id */, >> int64 /* position */, >> @@ -1104,8 +1111,13 @@ >> GURL /* last url */, >> GURL /* url redirected to */) >> >> - // Sent when the renderer process to acknowlege receipt of and >> UploadProgress >> - // message. >> + // Sent by the renderer process to acknowledge receipt of a >> + // DownloadProgress message. >> + IPC_MESSAGE_ROUTED1(ViewHostMsg_DownloadProgress_ACK, >> + int /* request_id */) >> + >> + // Sent by the renderer process to acknowledge receipt of a >> + // UploadProgress message. >> IPC_MESSAGE_ROUTED1(ViewHostMsg_UploadProgress_ACK, >> int /* request_id */) >> >> >> Modified: trunk/src/chrome/common/resource_dispatcher.cc >> ============================================================================== >> --- trunk/src/chrome/common/resource_dispatcher.cc (original) >> +++ trunk/src/chrome/common/resource_dispatcher.cc Wed Mar 4 22:38:52 >> 2009 >> @@ -278,6 +278,13 @@ >> return true; >> } >> >> +void ResourceDispatcher::OnDownloadProgress( >> + int request_id, int64 position, int64 size) { >> + // TODO(hclam): delegate this message to >> + // ResourceLoaderBridge::Peer::OnDownloadProgress and send an ACK message >> + // back to ResourceDispatcherHost. >> +} >> + >> void ResourceDispatcher::OnUploadProgress( >> int request_id, int64 position, int64 size) { >> PendingRequestList::iterator it = pending_requests_.find(request_id); >> @@ -458,6 +465,7 @@ >> void ResourceDispatcher::DispatchMessage(const IPC::Message& message) { >> IPC_BEGIN_MESSAGE_MAP(ResourceDispatcher, message) >> IPC_MESSAGE_HANDLER(ViewMsg_Resource_UploadProgress, OnUploadProgress) >> + IPC_MESSAGE_HANDLER(ViewMsg_Resource_DownloadProgress, >> OnDownloadProgress) >> IPC_MESSAGE_HANDLER(ViewMsg_Resource_ReceivedResponse, >> OnReceivedResponse) >> IPC_MESSAGE_HANDLER(ViewMsg_Resource_ReceivedRedirect, >> OnReceivedRedirect) >> IPC_MESSAGE_HANDLER(ViewMsg_Resource_DataReceived, OnReceivedData) >> @@ -505,6 +513,7 @@ >> >> bool ResourceDispatcher::IsResourceMessage(const IPC::Message& message) >> const { >> switch (message.type()) { >> + case ViewMsg_Resource_DownloadProgress::ID: >> case ViewMsg_Resource_UploadProgress::ID: >> case ViewMsg_Resource_ReceivedResponse::ID: >> case ViewMsg_Resource_ReceivedRedirect::ID: >> >> Modified: trunk/src/chrome/common/resource_dispatcher.h >> ============================================================================== >> --- trunk/src/chrome/common/resource_dispatcher.h (original) >> +++ trunk/src/chrome/common/resource_dispatcher.h Wed Mar 4 22:38:52 >> 2009 >> @@ -103,6 +103,7 @@ >> typedef base::hash_map<int, PendingRequestInfo> PendingRequestList; >> >> // Message response handlers, called by the message handler for this >> process. >> + void OnDownloadProgress(int request_id, int64 position, int64 size); >> void OnUploadProgress(int request_id, int64 position, int64 size); >> void OnReceivedResponse(int request_id, const ResourceResponseHead&); >> void OnReceivedRedirect(int request_id, const GURL& new_url); >> >> Modified: trunk/src/net/base/load_flags.h >> ============================================================================== >> --- trunk/src/net/base/load_flags.h (original) >> +++ trunk/src/net/base/load_flags.h Wed Mar 4 22:38:52 2009 >> @@ -37,6 +37,9 @@ >> // If present, upload progress messages should be provided to initiator. >> LOAD_ENABLE_UPLOAD_PROGRESS = 1 << 6, >> >> + // If present, try to download the resource to a standalone file. >> + LOAD_ENABLE_DOWNLOAD_FILE = 1 << 7, >> + >> // If present, ignores certificate mismatches with the domain name. >> // (The default behavior is to trigger an OnSSLCertificateError callback.) >> LOAD_IGNORE_CERT_COMMON_NAME_INVALID = 1 << 8, >> >> Modified: trunk/src/net/http/http_cache.cc >> ============================================================================== >> --- trunk/src/net/http/http_cache.cc (original) >> +++ trunk/src/net/http/http_cache.cc Wed Mar 4 22:38:52 2009 >> @@ -606,6 +606,15 @@ >> if (cache_->mode() == RECORD) >> effective_load_flags_ |= LOAD_BYPASS_CACHE; >> >> + // If HttpCache has type MEDIA make sure LOAD_ENABLE_DOWNLOAD_FILE is set, >> + // otherwise make sure LOAD_ENABLE_DOWNLOAD_FILE is not set when HttpCache >> + // has type other than MEDIA. >> + if (cache_->type() == HttpCache::MEDIA) { >> + DCHECK(effective_load_flags_ & LOAD_ENABLE_DOWNLOAD_FILE); >> + } else { >> + DCHECK(!(effective_load_flags_ & LOAD_ENABLE_DOWNLOAD_FILE)); >> + } >> + >> // Some headers imply load flags. The order here is significant. >> // >> // LOAD_DISABLE_CACHE : no cache read or write >> >> Modified: trunk/src/net/http/http_cache_unittest.cc >> ============================================================================== >> --- trunk/src/net/http/http_cache_unittest.cc (original) >> +++ trunk/src/net/http/http_cache_unittest.cc Wed Mar 4 22:38:52 2009 >> @@ -1225,6 +1225,7 @@ >> #endif >> >> ScopedMockTransaction trans_info(kSimpleGET_Transaction); >> + trans_info.load_flags |= net::LOAD_ENABLE_DOWNLOAD_FILE; >> TestCompletionCallback callback; >> >> { >> >> Modified: trunk/src/net/url_request/url_request.h >> ============================================================================== >> --- trunk/src/net/url_request/url_request.h (original) >> +++ trunk/src/net/url_request/url_request.h Wed Mar 4 22:38:52 2009 >> @@ -307,6 +307,13 @@ >> return response_info_.ssl_info; >> } >> >> + // Returns the platform specific file handle for the standalone file that >> + // contains response data. base::kInvalidPlatformFileValue is returned if >> + // such file is not available. >> + base::PlatformFile response_data_file() { >> + return response_info_.response_data_file; >> + } >> + >> // Returns the cookie values included in the response, if the request is >> one >> // that can have cookies. Returns true if the request is a cookie-bearing >> // type, false otherwise. This method may only be called once the >> >> Modified: trunk/src/webkit/glue/resource_loader_bridge.h >> ============================================================================== >> --- trunk/src/webkit/glue/resource_loader_bridge.h (original) >> +++ trunk/src/webkit/glue/resource_loader_bridge.h Wed Mar 4 22:38:52 >> 2009 >> @@ -91,9 +91,16 @@ >> public: >> virtual ~Peer() {} >> >> + // Called as download progress is made. >> + // note: only for requests with LOAD_ENABLE_DOWNLOAD_FILE set and the >> + // resource is downloaded to a standalone file and the file handle to >> it is >> + // passed in ResponseInfo during OnReceivedResponse. Note that size may >> be >> + // unknown and |size| will be kuint64max in that case. >> + virtual void OnDownloadProgress(uint64 position, uint64 size) {} >> + >> // Called as upload progress is made. >> - // note: only for requests with LOAD_ENABLE_UPLOAD_PROGRESS set >> - virtual void OnUploadProgress(uint64 position, uint64 size) {}; >> + // note: only for requests with LOAD_ENABLE_UPLOAD_PROGRESS set. >> + virtual void OnUploadProgress(uint64 position, uint64 size) {} >> >> // Called when a redirect occurs. >> virtual void OnReceivedRedirect(const GURL& new_url) = 0; >> @@ -105,7 +112,7 @@ >> virtual void OnReceivedResponse(const ResponseInfo& info, >> bool content_filtered) = 0; >> >> - // Called when a chunk of response data is available. This method may >> + // Called when a chunk of response data is available. This method may >> // be called multiple times or not at all if an error occurs. >> virtual void OnReceivedData(const char* data, int len) = 0; >> >> >> >> >> > --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
