diff options
author | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-02 19:13:00 +0000 |
---|---|---|
committer | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-02 19:13:00 +0000 |
commit | 70daf0b18defd88e97f5f9ebcc9486c22898b66b (patch) | |
tree | 71e5c7cd9c7733ecafd589e8be81a67c67b2070a | |
parent | 47b950579d39dc79127c1dc69f44c33c8ad269b0 (diff) | |
download | chromium_src-70daf0b18defd88e97f5f9ebcc9486c22898b66b.zip chromium_src-70daf0b18defd88e97f5f9ebcc9486c22898b66b.tar.gz chromium_src-70daf0b18defd88e97f5f9ebcc9486c22898b66b.tar.bz2 |
ChromeFrame should honor the host browser's cookie policy. To achieve this we always read the cookies from
the host browser when the renderer requests them. This also cleans up the mess with the host network stack
code parsing cookies from the host looking for persistent cookies.
Fixes bug http://code.google.com/p/chromium/issues/detail?id=34151
Bug=34151
Test=Covered by existing host network stack tests and chrome frame cookie tests.
Review URL: http://codereview.chromium.org/661290
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@40402 0039d316-1c4b-4281-b951-d872f2087c98
27 files changed, 361 insertions, 302 deletions
diff --git a/chrome/browser/automation/automation_profile_impl.cc b/chrome/browser/automation/automation_profile_impl.cc index d09b036..751019a 100644 --- a/chrome/browser/automation/automation_profile_impl.cc +++ b/chrome/browser/automation/automation_profile_impl.cc @@ -3,10 +3,14 @@ // found in the LICENSE file. #include "chrome/browser/automation/automation_profile_impl.h" + +#include <map> + #include "chrome/browser/automation/automation_resource_message_filter.h" #include "chrome/browser/chrome_thread.h" #include "chrome/browser/net/chrome_url_request_context.h" #include "chrome/browser/profile.h" +#include "net/base/net_errors.h" #include "net/url_request/url_request_context.h" #include "chrome/test/automation/automation_messages.h" @@ -18,15 +22,21 @@ namespace AutomationRequestContext { class AutomationURLRequestContext : public ChromeURLRequestContext { public: AutomationURLRequestContext(ChromeURLRequestContext* original_context, - net::CookieStore* automation_cookie_store) + net::CookieStore* automation_cookie_store, + net::CookiePolicy* automation_cookie_policy) : ChromeURLRequestContext(original_context), // We must hold a reference to |original_context|, since many // of the dependencies that ChromeURLRequestContext(original_context) // copied are scoped to |original_context|. original_context_(original_context) { + cookie_policy_ = automation_cookie_policy; cookie_store_ = automation_cookie_store; } + virtual bool IsExternal() const { + return true; + } + private: virtual ~AutomationURLRequestContext() { // Clear out members before calling base class dtor since we don't @@ -57,31 +67,33 @@ class AutomationCookieStore : public net::CookieStore { tab_handle_(tab_handle) { } + virtual ~AutomationCookieStore() { + DLOG(INFO) << "In " << __FUNCTION__; + } + // CookieStore implementation. virtual bool SetCookieWithOptions(const GURL& url, const std::string& cookie_line, const net::CookieOptions& options) { - bool cookie_set = - original_cookie_store_->SetCookieWithOptions(url, cookie_line, - options); - if (cookie_set) { - // TODO(eroman): Should NOT be accessing the profile from here, as this - // is running on the IO thread. - SendIPCMessageOnIOThread(new AutomationMsg_SetCookieAsync(0, - tab_handle_, url, cookie_line)); - } - return cookie_set; + // The cookie_string_ is available only once, i.e. once it is read by + // it is invalidated. + cookie_string_ = cookie_line; + return true; } + virtual std::string GetCookiesWithOptions(const GURL& url, const net::CookieOptions& options) { - return original_cookie_store_->GetCookiesWithOptions(url, options); + return cookie_string_; } + virtual void DeleteCookie(const GURL& url, const std::string& cookie_name) { - return original_cookie_store_->DeleteCookie(url, cookie_name); + NOTREACHED() << "Should not get called for an automation profile"; } + virtual net::CookieMonster* GetCookieMonster() { - return original_cookie_store_->GetCookieMonster(); + NOTREACHED() << "Should not get called for an automation profile"; + return NULL; } protected: @@ -98,11 +110,53 @@ class AutomationCookieStore : public net::CookieStore { net::CookieStore* original_cookie_store_; scoped_refptr<AutomationResourceMessageFilter> automation_client_; int tab_handle_; + std::string cookie_string_; private: DISALLOW_COPY_AND_ASSIGN(AutomationCookieStore); }; +// CookiePolicy specialization for automation specific cookie policies. +class AutomationCookiePolicy : public net::CookiePolicy { + public: + AutomationCookiePolicy(AutomationResourceMessageFilter* automation_client, + int tab_handle, net::CookieStore* cookie_store) + : automation_client_(automation_client), + tab_handle_(tab_handle), + cookie_store_(cookie_store) {} + + virtual int CanGetCookies(const GURL& url, + const GURL& first_party_for_cookies, + net::CompletionCallback* callback) { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); + + if (automation_client_.get()) { + automation_client_->GetCookiesForUrl(tab_handle_, url, callback, + cookie_store_.get()); + return net::ERR_IO_PENDING; + } + return net::ERR_ACCESS_DENIED; + } + + virtual int CanSetCookie(const GURL& url, + const GURL& first_party_for_cookies, + const std::string& cookie_line, + net::CompletionCallback* callback) { + if (automation_client_.get()) { + automation_client_->Send(new AutomationMsg_SetCookieAsync(0, + tab_handle_, url, cookie_line)); + } + return net::ERR_ACCESS_DENIED; + } + + private: + scoped_refptr<AutomationResourceMessageFilter> automation_client_; + int tab_handle_; + scoped_refptr<net::CookieStore> cookie_store_; + + DISALLOW_COPY_AND_ASSIGN(AutomationCookiePolicy); +}; + class Factory : public ChromeURLRequestContextFactory { public: Factory(ChromeURLRequestContextGetter* original_context_getter, @@ -125,8 +179,15 @@ class Factory : public ChromeURLRequestContextFactory { automation_client_, tab_handle_); + // Create an automation cookie policy. + AutomationCookiePolicy* automation_cookie_policy = + new AutomationCookiePolicy(automation_client_, + tab_handle_, + automation_cookie_store); + return new AutomationURLRequestContext(original_context, - automation_cookie_store); + automation_cookie_store, + automation_cookie_policy); } private: diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc index b7311b7..d0cb97e 100644 --- a/chrome/browser/automation/automation_provider.cc +++ b/chrome/browser/automation/automation_provider.cc @@ -1030,8 +1030,12 @@ void AutomationProvider::GetCookies(const GURL& url, int handle, NavigationController* tab = tab_tracker_->GetResource(handle); // Since we are running on the UI thread don't call GetURLRequestContext(). - net::CookieStore* cookie_store = - tab->profile()->GetRequestContext()->GetCookieStore(); + scoped_refptr<URLRequestContextGetter> request_context = + tab->tab_contents()->request_context(); + if (!request_context.get()) + request_context = tab->profile()->GetRequestContext(); + + net::CookieStore* cookie_store = request_context->GetCookieStore(); *value = cookie_store->GetCookies(url); *value_size = static_cast<int>(value->size()); diff --git a/chrome/browser/automation/automation_resource_message_filter.cc b/chrome/browser/automation/automation_resource_message_filter.cc index 77c47a9..90a4555 100644 --- a/chrome/browser/automation/automation_resource_message_filter.cc +++ b/chrome/browser/automation/automation_resource_message_filter.cc @@ -16,9 +16,11 @@ #include "chrome/common/chrome_paths.h" #include "chrome/browser/chrome_thread.h" #include "chrome/test/automation/automation_messages.h" +#include "googleurl/src/gurl.h" +#include "net/base/cookie_store.h" +#include "net/base/net_errors.h" #include "net/url_request/url_request_filter.h" - AutomationResourceMessageFilter::RenderViewMap AutomationResourceMessageFilter::filtered_render_views_; @@ -87,7 +89,8 @@ bool AutomationResourceMessageFilter::OnMessageReceived( OnGetFilteredInetHitCount) IPC_MESSAGE_HANDLER(AutomationMsg_RecordHistograms, OnRecordHistograms) - + IPC_MESSAGE_HANDLER(AutomationMsg_GetCookiesHostResponse, + OnGetCookiesHostResponse) IPC_MESSAGE_UNHANDLED(handled = false) IPC_END_MESSAGE_MAP() @@ -311,6 +314,54 @@ void AutomationResourceMessageFilter::OnRecordHistograms( } } +void AutomationResourceMessageFilter::GetCookiesForUrl( + int tab_handle, const GURL& url, net::CompletionCallback* callback, + net::CookieStore* cookie_store) { + DCHECK(callback != NULL); + DCHECK(cookie_store != NULL); + + int completion_callback_id = GetNextCompletionCallbackId(); + DCHECK(!ContainsKey(completion_callback_map_, completion_callback_id)); + + CookieCompletionInfo cookie_info; + cookie_info.completion_callback = callback; + cookie_info.cookie_store = cookie_store; + + completion_callback_map_[completion_callback_id] = cookie_info; + + channel_->Send(new AutomationMsg_GetCookiesFromHost(0, + tab_handle, url, completion_callback_id)); +} + +void AutomationResourceMessageFilter::OnGetCookiesHostResponse( + int tab_handle, bool success, const GURL& url, const std::string& cookies, + int cookie_id) { + CompletionCallbackMap::iterator index = + completion_callback_map_.find(cookie_id); + if (index != completion_callback_map_.end()) { + net::CompletionCallback* callback = index->second.completion_callback; + scoped_refptr<net::CookieStore> cookie_store = index->second.cookie_store; + + DCHECK(callback != NULL); + DCHECK(cookie_store.get() != NULL); + + completion_callback_map_.erase(index); + + // Set the cookie in the cookie store so that the callback can read it. + cookie_store->SetCookieWithOptions(url, cookies, net::CookieOptions()); + + Tuple1<int> params; + params.a = success ? net::OK : net::ERR_ACCESS_DENIED; + callback->RunWithParams(params); + + // The cookie for this URL is only valid until it is read by the callback. + cookie_store->SetCookieWithOptions(url, "", net::CookieOptions()); + } else { + NOTREACHED() << "Received invalid completion callback id:" + << cookie_id; + } +} + // static void AutomationResourceMessageFilter::ResumeJobsForPendingView( int tab_handle, diff --git a/chrome/browser/automation/automation_resource_message_filter.h b/chrome/browser/automation/automation_resource_message_filter.h index 9d0e8018..df7eca1 100644 --- a/chrome/browser/automation/automation_resource_message_filter.h +++ b/chrome/browser/automation/automation_resource_message_filter.h @@ -11,8 +11,14 @@ #include "base/lock.h" #include "base/platform_thread.h" #include "ipc/ipc_channel_proxy.h" +#include "net/base/completion_callback.h" class URLRequestAutomationJob; +class GURL; + +namespace net { +class CookieStore; +} // namespace net // This class filters out incoming automation IPC messages for network // requests and processes them on the IPC thread. As a result, network @@ -89,6 +95,20 @@ class AutomationResourceMessageFilter bool SendDownloadRequestToHost(int routing_id, int tab_handle, int request_id); + // Retrieves cookies for the url passed in from the external host. The + // callback passed in is notified on success or failure asynchronously. + void GetCookiesForUrl(int tab_handle, const GURL& url, + net::CompletionCallback* callback, + net::CookieStore* cookie_store); + + // This function gets invoked when we receive a response from the external + // host for the cookie request sent in GetCookiesForUrl above. It sets the + // cookie temporarily on the cookie store and executes the completion + // callback which reads the cookie from the store. The cookie value is reset + // after the callback finishes executing. + void OnGetCookiesHostResponse(int tab_handle, bool success, const GURL& url, + const std::string& cookies, int cookie_id); + protected: // Retrieves the automation request id for the passed in chrome request // id and returns it in the automation_request_id parameter. @@ -116,6 +136,10 @@ class AutomationResourceMessageFilter AutomationResourceMessageFilter* old_filter, AutomationResourceMessageFilter* new_filter); + int GetNextCompletionCallbackId() { + return ++next_completion_callback_id_; + } + // A unique renderer id is a combination of renderer process id and // it's routing id. struct RendererId { @@ -150,6 +174,25 @@ class AutomationResourceMessageFilter // Map of render views interested in diverting url requests over automation. static RenderViewMap filtered_render_views_; + // Contains information used for completing the request to read cookies from + // the host coming in from the renderer. + struct CookieCompletionInfo { + net::CompletionCallback* completion_callback; + scoped_refptr<net::CookieStore> cookie_store; + }; + + // Map of completion callback id to CookieCompletionInfo, which contains the + // actual callback which is invoked on successful retrieval of cookies from + // host. The mapping is setup when GetCookiesForUrl is invoked to retrieve + // cookies from the host and is removed when we receive a response from the + // host. Please see the OnGetCookiesHostResponse function. + typedef std::map<int, CookieCompletionInfo> CompletionCallbackMap; + CompletionCallbackMap completion_callback_map_; + + // Contains the id of the next completion callback. This is passed to the the + // external host as a cookie referring to the completion callback. + int next_completion_callback_id_; + DISALLOW_COPY_AND_ASSIGN(AutomationResourceMessageFilter); }; diff --git a/chrome/browser/automation/url_request_automation_job.cc b/chrome/browser/automation/url_request_automation_job.cc index 8c6efa5..d6ebda3 100644 --- a/chrome/browser/automation/url_request_automation_job.cc +++ b/chrome/browser/automation/url_request_automation_job.cc @@ -49,24 +49,6 @@ URLRequest::ProtocolFactory* URLRequestAutomationJob::old_http_factory_ URLRequest::ProtocolFactory* URLRequestAutomationJob::old_https_factory_ = NULL; -namespace { - -// Returns true if the cookie passed in exists in the list of cookies -// parsed from the HTTP response header. -bool IsParsedCookiePresentInCookieHeader( - const net::CookieMonster::ParsedCookie& parsed_cookie, - const std::vector<std::string>& header_cookies) { - for (size_t i = 0; i < header_cookies.size(); ++i) { - net::CookieMonster::ParsedCookie parsed_header_cookie(header_cookies[i]); - if (parsed_header_cookie.Name() == parsed_cookie.Name()) - return true; - } - - return false; -} - -} // end namespace - URLRequestAutomationJob::URLRequestAutomationJob(URLRequest* request, int tab, int request_id, AutomationResourceMessageFilter* filter, bool is_pending) : URLRequestJob(request), @@ -296,79 +278,11 @@ void URLRequestAutomationJob::OnRequestStarted(int tab, int id, DCHECK(redirect_status_ == 0 || redirect_status_ == 200 || (redirect_status_ >= 300 && redirect_status_ < 400)); - GURL url_for_cookies = - GURL(redirect_url_.empty() ? request_->url().spec().c_str() : - redirect_url_.c_str()); - - URLRequestContext* ctx = request_->context(); - std::vector<std::string> response_cookies; - - // NOTE: We ignore Chrome's cookie policy to allow the automation to - // decide what cookies should be set. - if (!response.headers.empty()) { headers_ = new net::HttpResponseHeaders( net::HttpUtil::AssembleRawHeaders(response.headers.data(), response.headers.size())); - // Parse and set HTTP cookies. - const std::string name = "Set-Cookie"; - std::string value; - - void* iter = NULL; - while (headers_->EnumerateHeader(&iter, name, &value)) { - if (request_->context()->InterceptResponseCookie(request_, value)) - response_cookies.push_back(value); - } - - if (response_cookies.size() && ctx && ctx->cookie_store()) { - net::CookieOptions options; - options.set_include_httponly(); - ctx->cookie_store()->SetCookiesWithOptions(url_for_cookies, - response_cookies, - options); - } - } - - if (!response.persistent_cookies.empty() && ctx && ctx->cookie_store()) { - StringTokenizer cookie_parser(response.persistent_cookies, ";"); - - net::CookieMonster::CookieList existing_cookies; - net::CookieMonster* monster = ctx->cookie_store()->GetCookieMonster(); - DCHECK(monster); - if (monster) - existing_cookies = monster->GetAllCookiesForURL(url_for_cookies); - - while (cookie_parser.GetNext()) { - std::string cookie_string = cookie_parser.token(); - // Only allow cookies with valid name value pairs. - if (cookie_string.find('=') != std::string::npos) { - // Workaround for not having path information with the persistent - // cookies. When the path information is missing we assume path=/. - // If we don't do this the path of the cookie will be assumed to be - // the directory of url_for_cookies. - SetCookiePathToRootIfNotPresent(&cookie_string); - - // Ignore duplicate cookies, i.e. cookies passed in from the host - // browser which also exist in the response header. - net::CookieMonster::ParsedCookie parsed_cookie(cookie_string); - net::CookieMonster::CookieList::const_iterator i; - for (i = existing_cookies.begin(); i != existing_cookies.end(); ++i) { - if (i->second.Name() == parsed_cookie.Name()) - break; - } - - if (i == existing_cookies.end() && - !IsParsedCookiePresentInCookieHeader(parsed_cookie, - response_cookies)) { - net::CookieOptions options; - ctx->cookie_store()->SetCookieWithOptions(url_for_cookies, - cookie_string, - options); - } - } - } } - NotifyHeadersComplete(); } @@ -536,32 +450,6 @@ void URLRequestAutomationJob::StartPendingJob( Start(); } -// static -bool URLRequestAutomationJob::IsCookiePresentInCookieHeader( - const std::string& cookie_name, - const std::vector<std::string>& header_cookies) { - net::CookieMonster::ParsedCookie parsed_cookie(cookie_name); - return IsParsedCookiePresentInCookieHeader(parsed_cookie, header_cookies); -} - -// static -void URLRequestAutomationJob::SetCookiePathToRootIfNotPresent( - std::string* cookie_string) { - DCHECK(cookie_string); - TrimWhitespace(*cookie_string, TRIM_ALL, cookie_string); - - // First check if path is already specified. - net::CookieMonster::ParsedCookie parsed(*cookie_string); - if (parsed.IsValid() && !parsed.HasPath()) { - DCHECK(cookie_string->length() > 0); - - // The path is not specified, add it at the end. - if ((*cookie_string)[cookie_string->length() - 1] != ';') - *cookie_string += ';'; - cookie_string->append(" path=/"); - } -} - void URLRequestAutomationJob::NotifyJobCompletionTask() { if (!is_done()) { NotifyDone(request_status_); diff --git a/chrome/browser/automation/url_request_automation_job.h b/chrome/browser/automation/url_request_automation_job.h index 0e04d7c..b8c7442 100644 --- a/chrome/browser/automation/url_request_automation_job.h +++ b/chrome/browser/automation/url_request_automation_job.h @@ -55,16 +55,6 @@ class URLRequestAutomationJob : public URLRequestJob { return request_id_; } - // Returns true if the cookie passed in exists in the list of cookies - // parsed from the HTTP response header. - static bool IsCookiePresentInCookieHeader( - const std::string& cookie_name, - const std::vector<std::string>& header_cookies); - - // Parses a cookie string and if there's no path specified for the cookie - // it appends "; path=/" to the cookie string. - static void SetCookiePathToRootIfNotPresent(std::string* cookie_string); - bool is_pending() const { return is_pending_; } diff --git a/chrome/browser/net/chrome_url_request_context.h b/chrome/browser/net/chrome_url_request_context.h index 1aea2b0..07e2f0e 100644 --- a/chrome/browser/net/chrome_url_request_context.h +++ b/chrome/browser/net/chrome_url_request_context.h @@ -121,6 +121,12 @@ class ChromeURLRequestContext : public URLRequestContext { // False only if cookies are globally blocked without exception. bool AreCookiesEnabled() const; + // Returns true if this context is an external request context, like + // ChromeFrame. + virtual bool IsExternal() const { + return false; + } + protected: // Copies the dependencies from |other| into |this|. If you use this // constructor, then you should hold a reference to |other|, as we diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc index 94f60a8..5506ffd 100644 --- a/chrome/browser/renderer_host/resource_message_filter.cc +++ b/chrome/browser/renderer_host/resource_message_filter.cc @@ -158,7 +158,7 @@ class SetCookieCompletion : public net::CompletionCallback { int render_view_id, const GURL& url, const std::string& cookie_line, - URLRequestContext* context) + ChromeURLRequestContext* context) : render_process_id_(render_process_id), render_view_id_(render_view_id), url_(url), @@ -176,10 +176,12 @@ class SetCookieCompletion : public net::CompletionCallback { context_->cookie_store()->SetCookieWithOptions(url_, cookie_line_, options); } else { - CallRenderViewHostResourceDelegate( - render_process_id_, render_view_id_, - &RenderViewHostDelegate::Resource::OnContentBlocked, - CONTENT_SETTINGS_TYPE_COOKIES); + if (!context_->IsExternal()) { + CallRenderViewHostResourceDelegate( + render_process_id_, render_view_id_, + &RenderViewHostDelegate::Resource::OnContentBlocked, + CONTENT_SETTINGS_TYPE_COOKIES); + } } delete this; } @@ -189,7 +191,7 @@ class SetCookieCompletion : public net::CompletionCallback { int render_view_id_; GURL url_; std::string cookie_line_; - scoped_refptr<URLRequestContext> context_; + scoped_refptr<ChromeURLRequestContext> context_; }; class GetCookiesCompletion : public net::CompletionCallback { @@ -641,8 +643,15 @@ void ResourceMessageFilter::OnGetRawCookies( const GURL& url, const GURL& first_party_for_cookies, IPC::Message* reply_msg) { - // Only return raw cookies to trusted renderers. - if (!ChildProcessSecurityPolicy::GetInstance()->CanReadRawCookies(id())) { + + ChromeURLRequestContext* context = GetRequestContextForURL(url); + + // Only return raw cookies to trusted renderers or if this request is + // not targeted to an an external host like ChromeFrame. + // TODO(ananta) We need to support retreiving raw cookies from external + // hosts. + if (!ChildProcessSecurityPolicy::GetInstance()->CanReadRawCookies(id()) || + context->IsExternal()) { ViewHostMsg_GetRawCookies::WriteReplyParams( reply_msg, std::vector<webkit_glue::WebCookie>()); @@ -650,8 +659,6 @@ void ResourceMessageFilter::OnGetRawCookies( return; } - URLRequestContext* context = GetRequestContextForURL(url); - GetRawCookiesCompletion* callback = new GetRawCookiesCompletion(url, reply_msg, this, context); diff --git a/chrome/test/automation/automation_messages.h b/chrome/test/automation/automation_messages.h index a4501aa..49014dc 100644 --- a/chrome/test/automation/automation_messages.h +++ b/chrome/test/automation/automation_messages.h @@ -306,7 +306,6 @@ struct AutomationURLResponse { std::string headers; int64 content_length; base::Time last_modified; - std::string persistent_cookies; std::string redirect_url; int redirect_status; }; @@ -320,7 +319,6 @@ struct ParamTraits<AutomationURLResponse> { WriteParam(m, p.headers); WriteParam(m, p.content_length); WriteParam(m, p.last_modified); - WriteParam(m, p.persistent_cookies); WriteParam(m, p.redirect_url); WriteParam(m, p.redirect_status); } @@ -329,7 +327,6 @@ struct ParamTraits<AutomationURLResponse> { ReadParam(m, iter, &p->headers) && ReadParam(m, iter, &p->content_length) && ReadParam(m, iter, &p->last_modified) && - ReadParam(m, iter, &p->persistent_cookies) && ReadParam(m, iter, &p->redirect_url) && ReadParam(m, iter, &p->redirect_status); } @@ -343,8 +340,6 @@ struct ParamTraits<AutomationURLResponse> { l->append(L", "); LogParam(p.last_modified, l); l->append(L", "); - LogParam(p.persistent_cookies, l); - l->append(L", "); LogParam(p.redirect_url, l); l->append(L", "); LogParam(p.redirect_status, l); diff --git a/chrome/test/automation/automation_messages_internal.h b/chrome/test/automation/automation_messages_internal.h index eb74875..ae68136 100644 --- a/chrome/test/automation/automation_messages_internal.h +++ b/chrome/test/automation/automation_messages_internal.h @@ -1199,4 +1199,17 @@ IPC_BEGIN_MESSAGES(Automation) int /* tab handle */) #endif + // Used to get cookies for the given URL. + IPC_MESSAGE_ROUTED3(AutomationMsg_GetCookiesFromHost, + int /* tab_handle */, + GURL /* url */, + int /* opaque_cookie_id */) + + IPC_MESSAGE_ROUTED5(AutomationMsg_GetCookiesHostResponse, + int /* tab_handle */, + bool /* success */, + GURL /* url */, + std::string /* cookies */, + int /* opaque_cookie_id */) + IPC_END_MESSAGES(Automation) diff --git a/chrome/test/automation/automation_proxy_uitest.cc b/chrome/test/automation/automation_proxy_uitest.cc index c828638..b4b68b4 100644 --- a/chrome/test/automation/automation_proxy_uitest.cc +++ b/chrome/test/automation/automation_proxy_uitest.cc @@ -927,9 +927,6 @@ TEST_F(ExternalTabUITest, IncognitoMode) { GURL url("http://anatomyofmelancholy.net"); std::string cookie = "robert=burton; expires=Thu, 13 Oct 2011 05:04:03 UTC;"; - EXPECT_CALL(*mock_, OnSetCookieAsync(1, url, StrEq(cookie))) - .Times(1) - .WillOnce(QUIT_LOOP(&loop)); EXPECT_CALL(*mock_, HandleClosed(1)).Times(1); IPC::ExternalTabSettings incognito = @@ -942,9 +939,6 @@ TEST_F(ExternalTabUITest, IncognitoMode) { std::string value_result; EXPECT_TRUE(tab->SetCookie(url, cookie)); - loop.RunFor(action_max_timeout_ms()); - ASSERT_FALSE(loop.WasTimedOut()); // Expect QuitLoop from OnSetCookieAsync. - EXPECT_TRUE(tab->GetCookieByName(url, "robert", &value_result)); EXPECT_EQ("burton", value_result); mock_->DestroyHostWindow(); diff --git a/chrome_frame/chrome_frame_activex_base.h b/chrome_frame/chrome_frame_activex_base.h index 6d27604..487a3f6 100644 --- a/chrome_frame/chrome_frame_activex_base.h +++ b/chrome_frame/chrome_frame_activex_base.h @@ -492,9 +492,14 @@ END_MSG_MAP() // Verify if the cookie is being deleted. The cookie format is as below // value[; expires=date][; domain=domain][; path=path][; secure] // If the first semicolon appears immediately after the name= string, - // it means that the cookie is being deleted. - if (!parsed_cookie.Value().empty()) + // it means that the cookie is being deleted, in which case we should + // pass the data as is to the InternetSetCookie function. + if (!parsed_cookie.Value().empty()) { + name.clear(); + data = cookie; + } else { data = cookie.substr(name_end + 1); + } } else { data = cookie; } @@ -504,6 +509,34 @@ END_MSG_MAP() DCHECK(ret) << "InternetSetcookie failed. Error: " << GetLastError(); } + virtual void OnGetCookiesFromHost(int tab_handle, const GURL& url, + int cookie_id) { + DWORD cookie_size = 0; + bool success = true; + std::string cookie_string; + InternetGetCookieA(url.spec().c_str(), NULL, NULL, &cookie_size); + if (cookie_size) { + scoped_array<char> cookies(new char[cookie_size + 1]); + if (!InternetGetCookieA(url.spec().c_str(), NULL, cookies.get(), + &cookie_size)) { + success = false; + NOTREACHED() << "InternetGetCookie failed. Error: " << GetLastError(); + } else { + cookie_string = cookies.get(); + } + } else { + success = false; + DLOG(INFO) << "InternetGetCookie failed. Error: " << GetLastError(); + } + + if (automation_client_->automation_server()) { + automation_client_->automation_server()->Send( + new AutomationMsg_GetCookiesHostResponse(0, tab_handle, success, + url, cookie_string, + cookie_id)); + } + } + virtual void OnAttachExternalTab(int tab_handle, intptr_t cookie, int disposition) { diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc index 746340d..321d09d 100644 --- a/chrome_frame/chrome_frame_automation.cc +++ b/chrome_frame/chrome_frame_automation.cc @@ -1130,14 +1130,13 @@ void ChromeFrameAutomationClient::SetPageFontSize( void ChromeFrameAutomationClient::OnResponseStarted(int request_id, const char* mime_type, const char* headers, int size, - base::Time last_modified, const std::string& peristent_cookies, - const std::string& redirect_url, int redirect_status) { + base::Time last_modified, const std::string& redirect_url, + int redirect_status) { const IPC::AutomationURLResponse response = { mime_type, headers ? headers : "", size, last_modified, - peristent_cookies, redirect_url, redirect_status }; diff --git a/chrome_frame/chrome_frame_automation.h b/chrome_frame/chrome_frame_automation.h index f9e37c4..9c5b186 100644 --- a/chrome_frame/chrome_frame_automation.h +++ b/chrome_frame/chrome_frame_automation.h @@ -257,6 +257,10 @@ class ChromeFrameAutomationClient void SetPageFontSize(enum AutomationPageFontSize); + ChromeFrameAutomationProxy* automation_server() { + return automation_server_; + } + protected: // ChromeFrameAutomationProxy::LaunchDelegate implementation. virtual void LaunchComplete(ChromeFrameAutomationProxy* proxy, @@ -346,8 +350,7 @@ class ChromeFrameAutomationClient // as parameter and forwards to Chrome via IPC. virtual void OnResponseStarted(int request_id, const char* mime_type, const char* headers, int size, base::Time last_modified, - const std::string& peristent_cookies, const std::string& redirect_url, - int redirect_status); + const std::string& redirect_url, int redirect_status); virtual void OnReadComplete(int request_id, const void* buffer, int len); virtual void OnResponseEnd(int request_id, const URLRequestStatus& status); diff --git a/chrome_frame/chrome_frame_delegate.cc b/chrome_frame/chrome_frame_delegate.cc index eb97186..f286b08 100644 --- a/chrome_frame/chrome_frame_delegate.cc +++ b/chrome_frame/chrome_frame_delegate.cc @@ -26,6 +26,7 @@ bool ChromeFrameDelegateImpl::IsTabMessage(const IPC::Message& message, IPC_MESSAGE_HANDLER_GENERIC(AutomationMsg_SetCookieAsync, ) IPC_MESSAGE_HANDLER_GENERIC(AutomationMsg_AttachExternalTab, ) IPC_MESSAGE_HANDLER_GENERIC(AutomationMsg_RequestGoToHistoryEntryOffset, ) + IPC_MESSAGE_HANDLER_GENERIC(AutomationMsg_GetCookiesFromHost, ) IPC_MESSAGE_UNHANDLED(is_tab_message = false); IPC_END_MESSAGE_MAP() @@ -69,5 +70,7 @@ void ChromeFrameDelegateImpl::OnMessageReceived(const IPC::Message& msg) { IPC_MESSAGE_HANDLER(AutomationMsg_AttachExternalTab, OnAttachExternalTab) IPC_MESSAGE_HANDLER(AutomationMsg_RequestGoToHistoryEntryOffset, OnGoToHistoryEntryOffset) + IPC_MESSAGE_HANDLER(AutomationMsg_GetCookiesFromHost, + OnGetCookiesFromHost) IPC_END_MESSAGE_MAP() } diff --git a/chrome_frame/chrome_frame_delegate.h b/chrome_frame/chrome_frame_delegate.h index 93d4645..d577dfd 100644 --- a/chrome_frame/chrome_frame_delegate.h +++ b/chrome_frame/chrome_frame_delegate.h @@ -113,6 +113,9 @@ class ChromeFrameDelegateImpl : public ChromeFrameDelegate { virtual void OnAttachExternalTab(int tab_handle, intptr_t cookie, int disposition) {} virtual void OnGoToHistoryEntryOffset(int tab_handle, int offset) {} + + virtual void OnGetCookiesFromHost(int tab_handle, const GURL& url, + int cookie_id) {} }; // This interface enables tasks to be marshalled to desired threads. diff --git a/chrome_frame/chrome_frame_npapi.cc b/chrome_frame/chrome_frame_npapi.cc index fe734a8c..d147a40 100644 --- a/chrome_frame/chrome_frame_npapi.cc +++ b/chrome_frame/chrome_frame_npapi.cc @@ -529,6 +529,55 @@ void ChromeFrameNPAPI::OnSetCookieAsync(int tab_handle, const GURL& url, } } +void ChromeFrameNPAPI::OnGetCookiesFromHost(int tab_handle, const GURL& url, + int cookie_id) { + std::string cookie_string; + bool success = true; + + if (npapi::VersionMinor() >= NPVERS_HAS_URL_AND_AUTH_INFO) { + char* cookies = NULL; + unsigned int cookie_length = 0; + NPError ret = npapi::GetValueForURL(instance_, NPNURLVCookie, + url.spec().c_str(), &cookies, + &cookie_length); + if (ret == NPERR_NO_ERROR) { + DLOG(INFO) << "Obtained cookies:" << cookies << " from host"; + cookie_string.append(cookies, cookie_length); + npapi::MemFree(cookies); + } else { + success = false; + } + } else { + DLOG(INFO) << "Host does not support NPVERS_HAS_URL_AND_AUTH_INFO."; + if (url == GURL(document_url_)) { + DLOG(INFO) << "Reading document.cookie"; + NPVariant cookies = {}; + ExecuteScript("javascript:document.cookie", &cookies); + if (cookies.type == NPVariantType_String) { + cookie_string.append(cookies.value.stringValue.UTF8Characters, + cookies.value.stringValue.UTF8Length); + DLOG(INFO) << "Obtained cookies:" << cookie_string.c_str() + << " from host"; + npapi::ReleaseVariantValue(&cookies); + } else { + success = false; + } + } else { + success = false; + } + } + + if (!success) + DLOG(INFO) << "Failed to return cookies for url:" << url.spec().c_str(); + + if (automation_client_->automation_server()) { + automation_client_->automation_server()->Send( + new AutomationMsg_GetCookiesHostResponse(0, tab_handle, success, + url, cookie_string, + cookie_id)); + } +} + bool ChromeFrameNPAPI::HasMethod(NPObject* obj, NPIdentifier name) { for (int i = 0; i < arraysize(plugin_methods_); ++i) { if (name == plugin_method_identifiers_[i]) diff --git a/chrome_frame/chrome_frame_npapi.h b/chrome_frame/chrome_frame_npapi.h index 6ba130e..cc21ff5 100644 --- a/chrome_frame/chrome_frame_npapi.h +++ b/chrome_frame/chrome_frame_npapi.h @@ -139,6 +139,9 @@ END_MSG_MAP() virtual void OnSetCookieAsync(int tab_handle, const GURL& url, const std::string& cookie); + virtual void OnGetCookiesFromHost(int tab_handle, const GURL& url, + int cookie_id); + // ChromeFrameDelegate overrides virtual void OnLoadFailed(int error_code, const std::string& url); virtual void OnAutomationServerReady(); diff --git a/chrome_frame/npapi_url_request.cc b/chrome_frame/npapi_url_request.cc index d551901..c71659a 100644 --- a/chrome_frame/npapi_url_request.cc +++ b/chrome_frame/npapi_url_request.cc @@ -1,4 +1,4 @@ -// Copyright 2009, Google Inc. +// Copyright 2010, Google Inc. // All rights reserved. // // Redistribution and use in source and binary forms, with or without @@ -134,8 +134,7 @@ NPError NPAPIUrlRequest::OnStreamCreated(const char* mime_type, // Add support for passing persistent cookies and information about any URL // redirects to Chrome. delegate_->OnResponseStarted(id(), mime_type, stream->headers, stream->end, - base::Time::FromTimeT(stream->lastmodified), std::string(), - std::string(), 0); + base::Time::FromTimeT(stream->lastmodified), std::string(), 0); return NPERR_NO_ERROR; } @@ -250,10 +249,10 @@ void NPAPIUrlRequestManager::StopAll() { // Callbacks from NPAPIUrlRequest. Simply forward to the delegate. void NPAPIUrlRequestManager::OnResponseStarted(int request_id, const char* mime_type, const char* headers, int size, - base::Time last_modified, const std::string& peristent_cookies, - const std::string& redirect_url, int redirect_status) { + base::Time last_modified, const std::string& redirect_url, + int redirect_status) { delegate_->OnResponseStarted(request_id, mime_type, headers, size, - last_modified, peristent_cookies, redirect_url, redirect_status); + last_modified, redirect_url, redirect_status); } void NPAPIUrlRequestManager::OnReadComplete(int request_id, const void* buffer, diff --git a/chrome_frame/npapi_url_request.h b/chrome_frame/npapi_url_request.h index 0fec438..5b82de7 100644 --- a/chrome_frame/npapi_url_request.h +++ b/chrome_frame/npapi_url_request.h @@ -48,8 +48,7 @@ class NPAPIUrlRequestManager : public PluginUrlRequestManager, // PluginUrlRequestDelegate implementation. Forwards back to delegate. virtual void OnResponseStarted(int request_id, const char* mime_type, - const char* headers, int size, - base::Time last_modified, const std::string& peristent_cookies, + const char* headers, int size, base::Time last_modified, const std::string& redirect_url, int redirect_status); virtual void OnReadComplete(int request_id, const void* buffer, int len); virtual void OnResponseEnd(int request_id, const URLRequestStatus& status); diff --git a/chrome_frame/plugin_url_request.h b/chrome_frame/plugin_url_request.h index efd6741..ffe0c73 100644 --- a/chrome_frame/plugin_url_request.h +++ b/chrome_frame/plugin_url_request.h @@ -23,18 +23,9 @@ class PluginUrlRequestManager; class DECLSPEC_NOVTABLE PluginUrlRequestDelegate { public: - // Persistent cookies are read from the host browser and passed off to Chrome - // These cookies are sent when we receive a response for every URL request - // initiated by Chrome. Ideally we should only send cookies for the top level - // URL and any subframes. However we don't receive information from Chrome - // about the context for a URL, i.e. whether it is a subframe, etc. - // Additionally cookies for a URL should be sent once for the page. This - // is not done now as it is difficult to track URLs, specifically if they - // are redirected, etc. virtual void OnResponseStarted(int request_id, const char* mime_type, const char* headers, int size, base::Time last_modified, - const std::string& peristent_cookies, const std::string& redirect_url, - int redirect_status) = 0; + const std::string& redirect_url, int redirect_status) = 0; virtual void OnReadComplete(int request_id, const void* buffer, int len) = 0; virtual void OnResponseEnd(int request_id, const URLRequestStatus& status) = 0; protected: diff --git a/chrome_frame/test/automation_client_mock.h b/chrome_frame/test/automation_client_mock.h index da51759..b81a582 100644 --- a/chrome_frame/test/automation_client_mock.h +++ b/chrome_frame/test/automation_client_mock.h @@ -65,7 +65,7 @@ struct MockCFDelegate : public ChromeFrameDelegateImpl { void ReplyStarted(int request_id, const char* headers) { request_delegate_->OnResponseStarted(request_id, "text/html", headers, - 0, base::Time::Now(), EmptyString(), EmptyString(), 0); + 0, base::Time::Now(), EmptyString(), 0); } void ReplyData(int request_id, const std::string* data) { diff --git a/chrome_frame/test/html_util_unittests.cc b/chrome_frame/test/html_util_unittests.cc index b993129..7248b80 100644 --- a/chrome_frame/test/html_util_unittests.cc +++ b/chrome_frame/test/html_util_unittests.cc @@ -362,45 +362,3 @@ TEST(HttpUtils, HasFrameBustingHeader) { "X-Frame-Options: ALLOWall\r\n")); } -TEST(HttpCookieTest, IdentifyDuplicateCookieTest) { - std::vector<std::string> header_cookies; - header_cookies.push_back("BLAHHH; Path=/;"); - - EXPECT_FALSE(URLRequestAutomationJob::IsCookiePresentInCookieHeader( - "BLAHHH=1", header_cookies)); - - header_cookies.clear(); - - header_cookies.push_back("BLAHHH=1; Path=/;"); - - EXPECT_TRUE(URLRequestAutomationJob::IsCookiePresentInCookieHeader( - "BLAHHH=1", header_cookies)); - - header_cookies.clear(); - - header_cookies.push_back("BLAH=1; Path=/blah;"); - - EXPECT_FALSE(URLRequestAutomationJob::IsCookiePresentInCookieHeader( - "BLAH", header_cookies)); -} - -TEST(HttpCookieTest, SetCookiePathToRootIfNotPresentTest) { - struct TestCase { - std::string input; - std::string expected; - } test_cases[] = { - { "", "" }, - { "Cookie=value", "Cookie=value; path=/" }, - { "Cookie=value;", "Cookie=value; path=/" }, - { "Cookie=value; ", "Cookie=value; path=/" }, - { " Cookie=value; ", "Cookie=value; path=/" }, - { "Cookie=", "Cookie=; path=/" }, - { "Cookie=foo; path=/bar", "Cookie=foo; path=/bar" }, - }; - - for (int i = 0; i < arraysize(test_cases); ++i) { - std::string& cookie(test_cases[i].input); - URLRequestAutomationJob::SetCookiePathToRootIfNotPresent(&cookie); - EXPECT_EQ(cookie, test_cases[i].expected); - } -} diff --git a/chrome_frame/test/test_mock_with_web_server.cc b/chrome_frame/test/test_mock_with_web_server.cc index d4a045e..c376638 100644 --- a/chrome_frame/test/test_mock_with_web_server.cc +++ b/chrome_frame/test/test_mock_with_web_server.cc @@ -345,6 +345,10 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_BackForward) { CloseIeAtEndOfScope last_resort_close_ie; chrome_frame_test::TimedMsgLoop loop; ComStackObjectWithUninitialize<MockWebBrowserEventSink> mock; + + EXPECT_CALL(mock, OnFileDownload(VARIANT_TRUE, _)) + .Times(testing::AnyNumber()).WillRepeatedly(testing::Return()); + ::testing::InSequence sequence; // Everything in sequence // When the onhttpequiv patch is enabled, we will get two @@ -359,9 +363,6 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_BackForward) { _, _, _, _, _)) .WillOnce(testing::Return(S_OK)); - EXPECT_CALL(mock, OnFileDownload(VARIANT_TRUE, _)) - .WillOnce(testing::Return()); - EXPECT_CALL(mock, OnNavigateComplete2(_, _)) .WillOnce(testing::Return()); @@ -371,9 +372,6 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_BackForward) { _, _, _, _, _)) .Times(testing::AnyNumber()).WillRepeatedly(testing::Return(S_OK)); - EXPECT_CALL(mock, OnFileDownload(VARIANT_TRUE, _)) - .Times(testing::AnyNumber()).WillOnce(testing::Return()); - EXPECT_CALL(mock, OnNavigateComplete2(_, _)) .Times(testing::AnyNumber()).WillRepeatedly(testing::Return()); @@ -391,9 +389,6 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_BackForward) { _, _, _, _, _)) .WillOnce(testing::Return(S_OK)); - EXPECT_CALL(mock, OnFileDownload(VARIANT_TRUE, _)) - .Times(testing::AnyNumber()).WillOnce(testing::Return()); - EXPECT_CALL(mock, OnNavigateComplete2(_, _)) .WillOnce(testing::Return()); @@ -403,9 +398,6 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_BackForward) { _, _, _, _, _)) .Times(testing::AnyNumber()).WillRepeatedly(testing::Return(S_OK)); - EXPECT_CALL(mock, OnFileDownload(VARIANT_TRUE, _)) - .Times(testing::AnyNumber()).WillOnce(testing::Return()); - EXPECT_CALL(mock, OnNavigateComplete2(_, _)) .Times(testing::AnyNumber()).WillRepeatedly(testing::Return()); @@ -424,9 +416,6 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_BackForward) { _, _, _, _, _)) .WillOnce(testing::Return(S_OK)); - EXPECT_CALL(mock, OnFileDownload(VARIANT_TRUE, _)) - .Times(testing::AnyNumber()).WillOnce(testing::Return()); - EXPECT_CALL(mock, OnNavigateComplete2(_, _)) .WillOnce(testing::Return()); @@ -436,9 +425,6 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_BackForward) { _, _, _, _, _)) .Times(testing::AnyNumber()).WillRepeatedly(testing::Return(S_OK)); - EXPECT_CALL(mock, OnFileDownload(VARIANT_TRUE, _)) - .Times(testing::AnyNumber()).WillOnce(testing::Return()); - EXPECT_CALL(mock, OnNavigateComplete2(_, _)) .Times(testing::AnyNumber()).WillRepeatedly(testing::Return()); @@ -459,6 +445,15 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_BackForward) { EXPECT_CALL(mock, OnNavigateComplete2(_, _)) .WillOnce(testing::Return()); + EXPECT_CALL(mock, + OnBeforeNavigate2(_, testing::Field(&VARIANT::bstrVal, + testing::StrCaseEq(kSubFrameUrl2)), + _, _, _, _, _)) + .Times(testing::AnyNumber()).WillRepeatedly(testing::Return(S_OK)); + + EXPECT_CALL(mock, OnNavigateComplete2(_, _)) + .Times(testing::AnyNumber()).WillRepeatedly(testing::Return()); + EXPECT_CALL(mock, OnLoad(testing::StrEq(kSubFrameUrl2))) .WillOnce(testing::IgnoreResult(testing::InvokeWithoutArgs( CreateFunctor(ReceivePointer(mock.web_browser2_), @@ -474,6 +469,15 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_BackForward) { EXPECT_CALL(mock, OnNavigateComplete2(_, _)) .WillOnce(testing::Return()); + EXPECT_CALL(mock, + OnBeforeNavigate2(_, testing::Field(&VARIANT::bstrVal, + testing::StrCaseEq(kSubFrameUrl1)), + _, _, _, _, _)) + .Times(testing::AnyNumber()).WillRepeatedly(testing::Return(S_OK)); + + EXPECT_CALL(mock, OnNavigateComplete2(_, _)) + .Times(testing::AnyNumber()).WillRepeatedly(testing::Return()); + EXPECT_CALL(mock, OnLoad(testing::StrEq(kSubFrameUrl1))) .WillOnce(testing::IgnoreResult(testing::InvokeWithoutArgs( CreateFunctor(&mock, @@ -927,20 +931,19 @@ TEST_F(ChromeFrameTestWithWebServer, FLAKY_FullTabModeIE_ContextMenuBackForward) chrome_frame_test::TimedMsgLoop loop; ComStackObjectWithUninitialize<MockWebBrowserEventSink> mock; + EXPECT_CALL(mock, OnFileDownload(VARIANT_TRUE, _)) + .Times(testing::AnyNumber()).WillRepeatedly(testing::Return()); + ::testing::InSequence sequence; // Everything in sequence EXPECT_CALL(mock, OnBeforeNavigate2(_, testing::Field(&VARIANT::bstrVal, testing::StrCaseEq(kSubFrameUrl1)), _, _, _, _, _)) .WillOnce(testing::Return(S_OK)); - EXPECT_CALL(mock, OnFileDownload(VARIANT_TRUE, _)) - .Times(testing::AnyNumber()).WillRepeatedly(testing::Return()); EXPECT_CALL(mock, OnNavigateComplete2(_, _)).WillOnce(testing::Return()); EXPECT_CALL(mock, OnBeforeNavigate2(_, testing::Field(&VARIANT::bstrVal, testing::StrCaseEq(kSubFrameUrl1)), _, _, _, _, _)) .Times(testing::AnyNumber()).WillRepeatedly(testing::Return(S_OK)); - EXPECT_CALL(mock, OnFileDownload(VARIANT_TRUE, _)) - .Times(testing::AnyNumber()).WillRepeatedly(testing::Return()); EXPECT_CALL(mock, OnNavigateComplete2(_, _)) .Times(testing::AnyNumber()).WillRepeatedly(testing::Return()); @@ -955,15 +958,11 @@ TEST_F(ChromeFrameTestWithWebServer, FLAKY_FullTabModeIE_ContextMenuBackForward) testing::StrCaseEq(kSubFrameUrl2)), _, _, _, _, _)) .WillOnce(testing::Return(S_OK)); - EXPECT_CALL(mock, OnFileDownload(VARIANT_TRUE, _)) - .Times(testing::AnyNumber()).WillRepeatedly(testing::Return()); EXPECT_CALL(mock, OnNavigateComplete2(_, _)).WillOnce(testing::Return()); EXPECT_CALL(mock, OnBeforeNavigate2(_, testing::Field(&VARIANT::bstrVal, testing::StrCaseEq(kSubFrameUrl2)), _, _, _, _, _)) .Times(testing::AnyNumber()).WillRepeatedly(testing::Return(S_OK)); - EXPECT_CALL(mock, OnFileDownload(VARIANT_TRUE, _)) - .Times(testing::AnyNumber()).WillRepeatedly(testing::Return()); EXPECT_CALL(mock, OnNavigateComplete2(_, _)) .Times(testing::AnyNumber()).WillRepeatedly(testing::Return()); @@ -1038,6 +1037,7 @@ TEST_F(ChromeFrameTestWithWebServer, FLAKY_FullTabModeIE_ContextMenuBackForward) CreateFunctor(&mock, &MockWebBrowserEventSink::CloseWebBrowser)))); EXPECT_CALL(mock, OnQuit()).WillOnce(QUIT_LOOP(loop)); + HRESULT hr = mock.LaunchIEAndNavigate(kSubFrameUrl1); ASSERT_HRESULT_SUCCEEDED(hr); if (hr == S_FALSE) diff --git a/chrome_frame/test/url_request_test.cc b/chrome_frame/test/url_request_test.cc index aad95a9..03428e5 100644 --- a/chrome_frame/test/url_request_test.cc +++ b/chrome_frame/test/url_request_test.cc @@ -18,9 +18,8 @@ const int kChromeFrameLongNavigationTimeoutInSeconds = 10; class MockUrlDelegate : public PluginUrlRequestDelegate { public: - MOCK_METHOD8(OnResponseStarted, void(int request_id, const char* mime_type, - const char* headers, int size, - base::Time last_modified, const std::string& peristent_cookies, + MOCK_METHOD7(OnResponseStarted, void(int request_id, const char* mime_type, + const char* headers, int size, base::Time last_modified, const std::string& redirect_url, int redirect_status)); MOCK_METHOD3(OnReadComplete, void(int request_id, const void* buffer, int len)); @@ -65,8 +64,7 @@ TEST(UrlmonUrlRequestTest, Simple1) { testing::InSequence s; EXPECT_CALL(mock, OnResponseStarted(1, testing::_, testing::_, testing::_, - testing::_, testing::_, testing::_, - testing::_)) + testing::_, testing::_, testing::_)) .Times(1) .WillOnce(testing::IgnoreResult(testing::InvokeWithoutArgs(CreateFunctor( &request, &UrlmonUrlRequest::Read, 512)))); @@ -133,8 +131,7 @@ TEST(UrlmonUrlRequestTest, ZeroLengthResponse) { // Expect headers EXPECT_CALL(mock, OnResponseStarted(1, testing::_, testing::_, testing::_, - testing::_, testing::_, testing::_, - testing::_)) + testing::_, testing::_, testing::_)) .Times(1) .WillOnce(QUIT_LOOP(loop)); @@ -168,7 +165,7 @@ TEST(UrlmonUrlRequestManagerTest, Simple1) { server.Resolve(L"files/chrome_frame_window_open.html").spec(), "get" }; EXPECT_CALL(mock, OnResponseStarted(1, testing::_, testing::_, testing::_, - testing::_, testing::_, testing::_, testing::_)) + testing::_, testing::_, testing::_)) .Times(1) .WillOnce(testing::InvokeWithoutArgs(CreateFunctor(mgr.get(), &PluginUrlRequestManager::ReadUrlRequest, 0, 1, 512))); @@ -199,7 +196,7 @@ TEST(UrlmonUrlRequestManagerTest, Abort1) { server.Resolve(L"files/chrome_frame_window_open.html").spec(), "get" }; EXPECT_CALL(mock, OnResponseStarted(1, testing::_, testing::_, testing::_, - testing::_, testing::_, testing::_, testing::_)) + testing::_, testing::_, testing::_)) .Times(1) .WillOnce(testing::DoAll( testing::InvokeWithoutArgs(CreateFunctor(mgr.get(), diff --git a/chrome_frame/urlmon_url_request.cc b/chrome_frame/urlmon_url_request.cc index 149c94b..dbb50d6 100644 --- a/chrome_frame/urlmon_url_request.cc +++ b/chrome_frame/urlmon_url_request.cc @@ -477,34 +477,6 @@ STDMETHODIMP UrlmonUrlRequest::OnResponse(DWORD dwResponseCode, } } - std::string url_for_persistent_cookies; - std::string persistent_cookies; - - if (status_.was_redirected()) - url_for_persistent_cookies = status_.get_redirection().utf8_url; - - if (url_for_persistent_cookies.empty()) - url_for_persistent_cookies = url(); - - // Grab cookies for the specific Url from WININET. - { - DWORD cookie_size = 0; // NOLINT - std::wstring url = UTF8ToWide(url_for_persistent_cookies); - - // Note that there's really no way for us here to distinguish session - // cookies from persistent cookies here. Session cookies should get - // filtered out on the chrome side as to not be added again. - InternetGetCookie(url.c_str(), NULL, NULL, &cookie_size); - if (cookie_size) { - scoped_array<wchar_t> cookies(new wchar_t[cookie_size + 1]); - if (!InternetGetCookie(url.c_str(), NULL, cookies.get(), &cookie_size)) { - NOTREACHED() << "InternetGetCookie failed. Error: " << GetLastError(); - } else { - persistent_cookies = WideToUTF8(cookies.get()); - } - } - } - // Inform the delegate. headers_received_ = true; delegate_->OnResponseStarted(id(), @@ -512,7 +484,6 @@ STDMETHODIMP UrlmonUrlRequest::OnResponse(DWORD dwResponseCode, raw_headers.c_str(), // headers 0, // size base::Time(), // last_modified - persistent_cookies, status_.get_redirection().utf8_url, status_.get_redirection().http_code); return S_OK; @@ -991,12 +962,12 @@ void UrlmonUrlRequestManager::StopAllWorker() { void UrlmonUrlRequestManager::OnResponseStarted(int request_id, const char* mime_type, const char* headers, int size, - base::Time last_modified, const std::string& peristent_cookies, - const std::string& redirect_url, int redirect_status) { + base::Time last_modified, const std::string& redirect_url, + int redirect_status) { DCHECK_EQ(worker_thread_.thread_id(), PlatformThread::CurrentId()); DCHECK(LookupRequest(request_id).get() != NULL); delegate_->OnResponseStarted(request_id, mime_type, headers, size, - last_modified, peristent_cookies, redirect_url, redirect_status); + last_modified, redirect_url, redirect_status); } void UrlmonUrlRequestManager::OnReadComplete(int request_id, const void* buffer, diff --git a/chrome_frame/urlmon_url_request.h b/chrome_frame/urlmon_url_request.h index ebb9f56..6fbc147 100644 --- a/chrome_frame/urlmon_url_request.h +++ b/chrome_frame/urlmon_url_request.h @@ -19,9 +19,9 @@ class UrlmonUrlRequest; -class UrlmonUrlRequestManager : - public PluginUrlRequestManager, - public PluginUrlRequestDelegate { +class UrlmonUrlRequestManager + : public PluginUrlRequestManager, + public PluginUrlRequestDelegate { public: UrlmonUrlRequestManager(); ~UrlmonUrlRequestManager(); @@ -59,8 +59,7 @@ class UrlmonUrlRequestManager : // PluginUrlRequestDelegate implementation virtual void OnResponseStarted(int request_id, const char* mime_type, const char* headers, int size, base::Time last_modified, - const std::string& peristent_cookies, const std::string& redirect_url, - int redirect_status); + const std::string& redirect_url, int redirect_status); virtual void OnReadComplete(int request_id, const void* buffer, int len); virtual void OnResponseEnd(int request_id, const URLRequestStatus& status); |