Hello bpm,
I'd like you to do a code review. Please execute
g4 diff -c 8143836
or point your web browser to
http://mondrian/8143836
to review the following code:
Change 8143836 by [EMAIL PROTECTED] on 2008/08/29 18:05:49 *pending*
Fix for a crashing bug when Yahoo toolbar is installed.
R=bpm
CC=aa,[email protected]
DELTA=29 (22 added, 1 deleted, 6 changed)
OCL=8143836
Affected files ...
...
//depot/googleclient/gears/opensource/gears/localserver/ie/http_request_ie.cc#17
edit
...
//depot/googleclient/gears/opensource/gears/localserver/ie/http_request_ie.h#12
edit
...
//depot/googleclient/gears/opensource/gears/localserver/ie/progress_input_stream.cc#1
edit
...
//depot/googleclient/gears/opensource/gears/localserver/ie/progress_input_stream.h#1
edit
29 delta lines: 22 added, 1 deleted, 6 changed
Also consider running:
g4 lint -c 8143836
which verifies that the changelist doesn't introduce new style violations.
If you can't do the review, please let me know as soon as possible. During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately. Visit
http://www/eng/code_review.html for more information.
This is a semiautomated message from "g4 mail". Complaints or suggestions?
Mail [EMAIL PROTECTED]
Change 8143836 by [EMAIL PROTECTED] on 2008/08/29 18:05:49 *pending*
Fix for a crashing bug when Yahoo toolbar is installed.
Affected files ...
...
//depot/googleclient/gears/opensource/gears/localserver/ie/http_request_ie.cc#17
edit
...
//depot/googleclient/gears/opensource/gears/localserver/ie/http_request_ie.h#12
edit
...
//depot/googleclient/gears/opensource/gears/localserver/ie/progress_input_stream.cc#1
edit
...
//depot/googleclient/gears/opensource/gears/localserver/ie/progress_input_stream.h#1
edit
====
//depot/googleclient/gears/opensource/gears/localserver/ie/http_request_ie.cc#17
-
C:\p4\michaeln-scour2/googleclient/gears/opensource/gears/localserver/ie/http_request_ie.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/localserver/ie/http_request_ie.cc
2008-09-02 12:32:27.000000000 -0700
+++ googleclient/gears/opensource/gears/localserver/ie/http_request_ie.cc
2008-09-02 12:27:02.000000000 -0700
@@ -39,7 +39,6 @@
#include "gears/blob/blob_stream_ie.h"
#include "gears/blob/buffer_blob.h"
#include "gears/localserver/ie/http_handler_ie.h"
-#include "gears/localserver/ie/progress_input_stream.h"
#include "gears/localserver/ie/urlmon_utils.h"
// We use URLMON's pull-data model which requires making stream read calls
@@ -91,6 +90,10 @@
}
void IEHttpRequest::FinalRelease() {
+ if (post_data_stream_) {
+ post_data_stream_->DetachRequest();
+ post_data_stream_.Release();
+ }
}
void IEHttpRequest::Ref() {
@@ -556,8 +559,8 @@
info->stgmedData.tymed = TYMED_ISTREAM;
info->stgmedData.pstm = static_cast<IStream*>(stream);
- // stream has a 0 reference count at this point. The caller of GetBindInfo
- // will immediately do an AddRef on stream.
+
+ post_data_stream_ = stream;
}
if (cookie_behavior_ == DO_NOT_SEND_BROWSER_COOKIES) {
====
//depot/googleclient/gears/opensource/gears/localserver/ie/http_request_ie.h#12
-
C:\p4\michaeln-scour2/googleclient/gears/opensource/gears/localserver/ie/http_request_ie.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/localserver/ie/http_request_ie.h
2008-09-02 12:32:27.000000000 -0700
+++ googleclient/gears/opensource/gears/localserver/ie/http_request_ie.h
2008-09-02 11:29:52.000000000 -0700
@@ -34,6 +34,7 @@
#include "gears/localserver/common/http_request.h"
#include "gears/localserver/common/localserver_db.h"
#include "gears/localserver/common/progress_event.h"
+#include "gears/localserver/ie/progress_input_stream.h"
class BlobInterface;
class ByteStore;
@@ -227,7 +228,9 @@
int bind_verb_;
// The POST data
+ friend ProgressInputStream;
scoped_refptr<BlobInterface> post_data_;
+ CComPtr<ProgressInputStream> post_data_stream_;
// Additional request headers we've been asked to send with the request
std::string16 additional_headers_;
====
//depot/googleclient/gears/opensource/gears/localserver/ie/progress_input_stream.cc#1
-
C:\p4\michaeln-scour2/googleclient/gears/opensource/gears/localserver/ie/progress_input_stream.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/localserver/ie/progress_input_stream.cc
2008-09-02 12:32:27.000000000 -0700
+++ googleclient/gears/opensource/gears/localserver/ie/progress_input_stream.cc
2008-09-02 12:30:19.000000000 -0700
@@ -31,10 +31,18 @@
//ProgressInputStream implementation
//------------------------------------------------------------------------------
-ProgressInputStream::ProgressInputStream() {
+ProgressInputStream::ProgressInputStream() : request_(NULL) {
}
ProgressInputStream::~ProgressInputStream() {
+ // When the yahoo toolbar is installed, the stream gets released one too
+ // many times. We arrange to detect when this is occuring and avoid the extra
+ // release in that case.
+ if (request_) {
+ // The request still has a reference to this object, yet it's being
deleted.
+ // Drop our reference w/o releasing it.
+ request_->post_data_stream_.Detach();
+ }
}
void ProgressInputStream::Initialize(IEHttpRequest * request,
@@ -43,11 +51,16 @@
input_stream_ = input_stream;
}
+void ProgressInputStream::DetachRequest() {
+ request_ = NULL;
+}
+
//------------------------------------------------------------------------------
// ISequentialStream implementation
//------------------------------------------------------------------------------
STDMETHODIMP ProgressInputStream::Read(void* pv, ULONG cb, ULONG* read) {
+ if (!request_) return E_FAIL;
HRESULT result = input_stream_->Read(pv, cb, read);
if (SUCCEEDED(result) && *read > 0) {
static const LARGE_INTEGER offset = { 0 };
@@ -55,7 +68,7 @@
input_stream_->Seek(offset, STREAM_SEEK_CUR, &position);
STATSTG statstg;
input_stream_->Stat(&statstg, STATFLAG_NONAME);
- ProgressEvent::Update(request_.get(), request_.get(),
+ ProgressEvent::Update(request_, request_,
position.QuadPart, statstg.cbSize.QuadPart);
}
return result;
@@ -111,6 +124,7 @@
}
STDMETHODIMP ProgressInputStream::Clone(IStream **ppstm) {
+ if (!request_) return E_FAIL;
CComObject<ProgressInputStream> *stream = NULL;
HRESULT result = CComObject<ProgressInputStream>::CreateInstance(&stream);
if (FAILED(result)) {
@@ -122,7 +136,7 @@
if (FAILED(result)) {
return result;
}
- stream->Initialize(request_.get(), input_stream_clone);
+ stream->Initialize(request_, input_stream_clone);
*ppstm = istream.Detach();
return S_OK;
}
====
//depot/googleclient/gears/opensource/gears/localserver/ie/progress_input_stream.h#1
-
C:\p4\michaeln-scour2/googleclient/gears/opensource/gears/localserver/ie/progress_input_stream.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/localserver/ie/progress_input_stream.h
2008-09-02 12:32:27.000000000 -0700
+++ googleclient/gears/opensource/gears/localserver/ie/progress_input_stream.h
2008-09-02 11:34:17.000000000 -0700
@@ -42,6 +42,7 @@
ProgressInputStream();
virtual ~ProgressInputStream();
void Initialize(IEHttpRequest * request, IStream *input_stream);
+ void DetachRequest();
// ISequentialStream
STDMETHOD(Read)(void* pv, ULONG cb, ULONG* read);
@@ -63,7 +64,7 @@
STDMETHOD(Clone)(IStream **ppstm);
private:
- scoped_refptr<IEHttpRequest> request_;
+ IEHttpRequest *request_;
CComPtr<IStream> input_stream_;
};
