A lice-sized code review (xdong 2010-01-16 13:55:02)

[idlec...@gmail.com]

Hello phnixwxz,

I'd like you to do a code review. Please review the following patch:

commit 452553f788d072328eef08f5283659a74fc02c56
Author: Tiger Dong <xd...@google.com>
Date: Sat Jan 16 13:50:56 2010 +0800

Treat setRequestHeader("cookie", "none") and setRequestHeader("Cookie", "none") differently in qt_xml_http_request.

diff --git a/extensions/qt_xml_http_request/qt_xml_http_request.cc b/extensions/qt_xml_http_request/qt_xml_http_request.cc
index 2d65d86..1777b56 100644
--- a/extensions/qt_xml_http_request/qt_xml_http_request.cc
+++ b/extensions/qt_xml_http_request/qt_xml_http_request.cc
@@ -276,7 +276,9 @@ class XMLHttpRequest : public QObject,
return NO_ERR;
}

- if (strcasecmp(header, "Cookie") == 0 &&
+ // strcasecmp shall be used, but it'll break gmail gadget.
+ // Microsoft XHR is also case sensitive.
+ if (strcmp(header, "Cookie") == 0 &&
value && strcasecmp(value, "none") == 0) {
// Microsoft XHR hidden feature: setRequestHeader('Cookie', 'none')
// clears all cookies. Some gadgets (e.g. reader) use this.
@@ -320,8 +322,7 @@ class XMLHttpRequest : public QObject,
// Add an internal reference when this request is working to prevent
// this object from being GC'ed.
Ref();
- if (!no_cookie_)
- RestoreCookie(QUrl(url_.c_str()), request_header_);
+ if (!no_cookie_) RestoreCookie(QUrl(url_.c_str()), request_header_);

if (data.size()) {
send_data_ = new QByteArray(data.c_str(),
@@ -705,8 +706,7 @@ class XMLHttpRequest : public QObject,
} else {
redirected_times_++;
// FIXME(idlecat): What is the right behavior when redirected?
- if (no_cookie_)
- RestoreCookie(QUrl(url_.c_str()), request_header_);
+ if (!no_cookie_) RestoreCookie(QUrl(url_.c_str()), request_header_);

if (send_data_)
http_->request(*request_header_, *send_data_);

This is a semiautomated message from "gitmail". Complaints or suggestions?
Mail edy...@gmail.com.

[Xianzhu Wang]

Seems a workaround of a bug of the gmail gadget?

2010/1/16 <idlec...@gmail.com>:

[idlec...@gmail.com]

Yes, copied from curl_xml_http_request.
I don't understand why gmail gadget tries to set header "cookie" to "none" if it's not to disable the cookies. Any idea?

[Xianzhu Wang]

The CL LGTM.

I believe it is just a useless statement left there, but our Linux
implementation must work around it.

2010/1/16 <idlec...@gmail.com>: