Skip to content

Commit

Permalink
Fix chromium#1 for multiple request issue (both POST and GET) after a…
Browse files Browse the repository at this point in the history 
…ctivating the onhttpequiv approach.

There's a ton going on here. Brief summary: I'm introducing a caching layer for top level page requests that we then use again when switching the document from mshtml to cf. 

Also in this change list: 
* Removing the previous way of shifting the document moniker 
over to the worker thread when a new chrome document is 
created. Instead we use a request cache object. 
* Refactoring the part of the Bho class that offered access 
to it via TLS. This was needed for better testability but 
also makes (imho) the separation clearer and will possibly 
help in the future if we don't have a Bho object. 
* Added a bit of logging that I'd prefer to keep in there until we're confident enough with onhttpequiv. 
* Enabling two previously disabled tests (the ones I added for multiple requests) 
* Adding several more unit tests for the new code. 
Review URL: http://codereview.chromium.org/668187

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@41495 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
tommi@chromium.org committed Mar 12, 2010
1 parent 507c710 commit 051236f
Show file tree
Hide file tree
Showing 24 changed files with 2,534 additions and 263 deletions.
181 changes: 54 additions & 127 deletions chrome_frame/bho.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,24 @@
#include "chrome_frame/bho.h"

#include <shlguid.h>
#include <shobjidl.h>

#include "base/file_path.h"
#include "base/logging.h"
#include "base/path_service.h"
#include "base/registry.h"
#include "base/scoped_bstr_win.h"
#include "base/scoped_comptr_win.h"
#include "base/scoped_variant_win.h"
#include "base/string_util.h"
#include "chrome_tab.h" // NOLINT
#include "chrome_frame/extra_system_apis.h"
#include "chrome_frame/http_negotiate.h"
#include "chrome_frame/protocol_sink_wrap.h"
#include "chrome_frame/urlmon_moniker.h"
#include "chrome_frame/utils.h"
#include "chrome_frame/vtable_patch_manager.h"
#include "net/http/http_util.h"

const wchar_t kPatchProtocols[] = L"PatchProtocols";
static const int kIBrowserServiceOnHttpEquivIndex = 30;

base::LazyInstance<base::ThreadLocalPointer<Bho> >
Bho::bho_current_thread_instance_(base::LINKER_INITIALIZED);

PatchHelper g_patch_helper;

BEGIN_VTABLE_PATCHES(IBrowserService)
Expand All @@ -47,6 +41,13 @@ _ATL_FUNC_INFO Bho::kBeforeNavigate2Info = {
}
};

_ATL_FUNC_INFO Bho::kNavigateComplete2Info = {
CC_STDCALL, VT_EMPTY, 2, {
VT_DISPATCH,
VT_VARIANT | VT_BYREF
}
};

Bho::Bho() {
}

Expand Down Expand Up @@ -82,9 +83,9 @@ STDMETHODIMP Bho::SetSite(IUnknown* site) {
// our active document/activex instances to query referrer and other
// information for a URL.
AddRef();
bho_current_thread_instance_.Pointer()->Set(this);
RegisterThreadInstance();
} else {
bho_current_thread_instance_.Pointer()->Set(NULL);
UnregisterThreadInstance();
Release();
}

Expand Down Expand Up @@ -114,62 +115,19 @@ STDMETHODIMP Bho::BeforeNavigate2(IDispatch* dispatch, VARIANT* url,
if (!browser_service || !CheckForCFNavigation(browser_service, false)) {
referrer_.clear();
}
url_ = url->bstrVal;
ProcessOptInUrls(web_browser2, url->bstrVal);
return S_OK;
}

HRESULT Bho::NavigateToCurrentUrlInCF(IBrowserService* browser) {
DCHECK(browser);
MarkBrowserOnThreadForCFNavigation(browser);
ScopedComPtr<IBindCtx> bind_context;
ScopedComPtr<IMoniker> moniker;
HRESULT hr = ::CreateBindCtx(0, bind_context.Receive());
DCHECK(bind_context);
if (SUCCEEDED(hr) &&
SUCCEEDED(hr = ::CreateURLMonikerEx(NULL, url_.c_str(), moniker.Receive(),
URL_MK_UNIFORM))) {
DCHECK(SUCCEEDED(hr));
if (SUCCEEDED(hr)) {
#ifndef NDEBUG
// We've just created a new moniker for a URL that has just been fetched.
// However, the moniker used for the current navigation should still
// be running.
// The documentation for IMoniker::IsRunning() states that if the
// moniker is already running (i.e. an equal moniker already exists),
// then the return value from IsRunning will be S_OK (or 0) and
// S_FALSE (1) when the moniker is not running.
// http://msdn.microsoft.com/en-us/library/ms678475(VS.85).aspx
// However, knowing that the IsRunning implementation relies on
// the bind context and that the bind context uses ole32's
// IRunningObjectTable::IsRunning to do its bidding, the return value
// is actually TRUE (or 1) when the moniker is running and FALSE (0)
// when it is not running. Yup, the opposite of what you'd expect :-)
// http://msdn.microsoft.com/en-us/library/ms682169(VS.85).aspx
HRESULT running = moniker->IsRunning(bind_context, NULL, NULL);
DCHECK(running == TRUE) << "Moniker not already running?";
#endif

// If there's a referrer, preserve it.
std::wstring headers;
if (!referrer_.empty()) {
headers = StringPrintf(L"Referer: %ls\r\n\r\n",
ASCIIToWide(referrer_).c_str());
}

// Pass in URL fragments if applicable.
std::wstring fragment;
GURL parsed_moniker_url(url_);
if (parsed_moniker_url.has_ref()) {
fragment = UTF8ToWide(parsed_moniker_url.ref());
}

hr = NavigateBrowserToMoniker(browser, moniker, headers.c_str(),
bind_context, fragment.c_str());
}
VARIANT_BOOL is_top_level = VARIANT_FALSE;
web_browser2->get_TopLevelContainer(&is_top_level);
if (is_top_level) {
set_url(url->bstrVal);
ProcessOptInUrls(web_browser2, url->bstrVal);
}

return hr;
return S_OK;
}

STDMETHODIMP_(void) Bho::NavigateComplete2(IDispatch* dispatch, VARIANT* url) {
DLOG(INFO) << __FUNCTION__;
}

namespace {
Expand Down Expand Up @@ -239,11 +197,11 @@ HRESULT Bho::OnHttpEquiv(IBrowserService_OnHttpEquiv_Fn original_httpequiv,
// The embedded items should only be created once the top level
// doc has been created.
if (!DocumentHasEmbeddedItems(browser)) {
Bho* bho = Bho::GetCurrentThreadBhoInstance();
DCHECK(bho);
DLOG(INFO) << "Found tag in page. Marking browser." << bho->url() <<
NavigationManager* mgr = NavigationManager::GetThreadInstance();
DCHECK(mgr);
DLOG(INFO) << "Found tag in page. Marking browser." <<
StringPrintf(" tid=0x%08X", ::GetCurrentThreadId());
if (bho) {
if (mgr) {
// TODO(tommi): See if we can't figure out a cleaner way to avoid
// this. For some documents we can hit a problem here. When we
// attempt to navigate the document again in CF, mshtml can "complete"
Expand All @@ -253,82 +211,42 @@ HRESULT Bho::OnHttpEquiv(IBrowserService_OnHttpEquiv_Fn original_httpequiv,
// To work around this, we clear the contents of the document before
// opening it up in CF.
ClearDocumentContents(browser);
bho->NavigateToCurrentUrlInCF(browser);
mgr->NavigateToCurrentUrlInCF(browser);
}
}
}
} else if (done) {
DLOG(INFO) << "Releasing cached data.";
NavigationManager* mgr = NavigationManager::GetThreadInstance();
mgr->ReleaseRequestData();
}

return original_httpequiv(browser, shell_view, done, in_arg, out_arg);
}

void Bho::OnBeginningTransaction(IWebBrowser2* browser, const wchar_t* url,
const wchar_t* headers,
const wchar_t* additional_headers) {
if (!browser)
return;

if (url_.compare(url) != 0) {
DLOG(INFO) << __FUNCTION__ << " not processing headers for url: " << url
<< " current url is: " << url_;
return;
}

VARIANT_BOOL is_top_level = VARIANT_FALSE;
browser->get_TopLevelContainer(&is_top_level);
if (is_top_level) {
ScopedComPtr<IBrowserService> browser_service;
DoQueryService(SID_SShellBrowser, browser, browser_service.Receive());
if (!browser_service || !CheckForCFNavigation(browser_service, false)) {
// Save away the referrer in case our active document needs it to initiate
// navigation in chrome.
referrer_.clear();
const wchar_t* both_headers[] = { headers, additional_headers };
for (int i = 0; referrer_.empty() && i < arraysize(both_headers); ++i) {
if (!both_headers[i])
continue;
std::string raw_headers_utf8 = WideToUTF8(both_headers[i]);
std::string http_headers =
net::HttpUtil::AssembleRawHeaders(raw_headers_utf8.c_str(),
raw_headers_utf8.length());
net::HttpUtil::HeadersIterator it(http_headers.begin(),
http_headers.end(), "\r\n");
while (it.GetNext()) {
if (LowerCaseEqualsASCII(it.name(), "referer")) {
referrer_ = it.values();
break;
}
}
}
}
}
}

Bho* Bho::GetCurrentThreadBhoInstance() {
DCHECK(bho_current_thread_instance_.Pointer()->Get() != NULL);
return bho_current_thread_instance_.Pointer()->Get();
}

// static
void Bho::ProcessOptInUrls(IWebBrowser2* browser, BSTR url) {
if (!browser || !url) {
NOTREACHED();
return;
}

#ifndef NDEBUG
// This check must already have been made.
VARIANT_BOOL is_top_level = VARIANT_FALSE;
browser->get_TopLevelContainer(&is_top_level);
if (is_top_level) {
std::wstring current_url(url, SysStringLen(url));
if (IsValidUrlScheme(current_url, false)) {
bool cf_protocol = StartsWith(current_url, kChromeProtocolPrefix, false);
if (!cf_protocol && IsOptInUrl(current_url.c_str())) {
DLOG(INFO) << "Opt-in URL. Switching to cf.";
ScopedComPtr<IBrowserService> browser_service;
DoQueryService(SID_SShellBrowser, browser, browser_service.Receive());
DCHECK(browser_service) << "DoQueryService - SID_SShellBrowser failed.";
MarkBrowserOnThreadForCFNavigation(browser_service);
}
DCHECK(is_top_level);
#endif

std::wstring current_url(url, SysStringLen(url));
if (IsValidUrlScheme(current_url, false)) {
bool cf_protocol = StartsWith(current_url, kChromeProtocolPrefix, false);
if (!cf_protocol && IsOptInUrl(current_url.c_str())) {
DLOG(INFO) << "Opt-in URL. Switching to cf.";
ScopedComPtr<IBrowserService> browser_service;
DoQueryService(SID_SShellBrowser, browser, browser_service.Receive());
DCHECK(browser_service) << "DoQueryService - SID_SShellBrowser failed.";
MarkBrowserOnThreadForCFNavigation(browser_service);
}
}
}
Expand Down Expand Up @@ -362,12 +280,20 @@ bool PatchHelper::InitializeAndPatchProtocolsIfNeeded() {

HttpNegotiatePatch::Initialize();

bool patch_protocol = GetConfigBool(false, kPatchProtocols);
if (patch_protocol) {
ProtocolPatchMethod patch_method =
static_cast<ProtocolPatchMethod>(
GetConfigInt(PATCH_METHOD_IBROWSER, kPatchProtocols));

if (patch_method == PATCH_METHOD_INET_PROTOCOL) {
ProtocolSinkWrap::PatchProtocolHandlers();
state_ = PATCH_PROTOCOL;
} else {
DCHECK(patch_method == PATCH_METHOD_IBROWSER ||
patch_method == PATCH_METHOD_IBROWSER_AND_MONIKER);
state_ = PATCH_IBROWSER;
if (patch_method == PATCH_METHOD_IBROWSER_AND_MONIKER) {
MonikerPatch::Initialize();
}
}
ret = true;
}
Expand All @@ -390,6 +316,7 @@ void PatchHelper::UnpatchIfNeeded() {
ProtocolSinkWrap::UnpatchProtocolHandlers();
} else if (state_ == PATCH_IBROWSER) {
vtable_patch::UnpatchInterfaceMethods(IBrowserService_PatchInfo);
MonikerPatch::Uninitialize();
}

HttpNegotiatePatch::Uninitialize();
Expand Down
41 changes: 10 additions & 31 deletions chrome_frame/bho.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@

#include <string>

#include "base/lazy_instance.h"
#include "base/thread_local.h"
#include "base/scoped_comptr_win.h"
#include "chrome_tab.h" // NOLINT
#include "chrome_frame/resource.h"
#include "chrome_frame/urlmon_moniker.h"
#include "chrome_frame/urlmon_url_request.h"
#include "grit/chrome_frame_resources.h"

class PatchHelper {
Expand Down Expand Up @@ -47,7 +48,8 @@ class ATL_NO_VTABLE Bho
: public CComObjectRootEx<CComSingleThreadModel>,
public CComCoClass<Bho, &CLSID_ChromeFrameBHO>,
public IObjectWithSiteImpl<Bho>,
public IDispEventSimpleImpl<0, Bho, &DIID_DWebBrowserEvents2> {
public IDispEventSimpleImpl<0, Bho, &DIID_DWebBrowserEvents2>,
public NavigationManager {
public:
typedef HRESULT (STDMETHODCALLTYPE* IBrowserService_OnHttpEquiv_Fn)(
IBrowserService* browser, IShellView* shell_view, BOOL done,
Expand All @@ -64,9 +66,10 @@ END_COM_MAP()
BEGIN_SINK_MAP(Bho)
SINK_ENTRY_INFO(0, DIID_DWebBrowserEvents2, DISPID_BEFORENAVIGATE2,
BeforeNavigate2, &kBeforeNavigate2Info)
SINK_ENTRY_INFO(0, DIID_DWebBrowserEvents2, DISPID_NAVIGATECOMPLETE2,
NavigateComplete2, &kNavigateComplete2Info)
END_SINK_MAP()

// Lifetime management methods
Bho();

HRESULT FinalConstruct();
Expand All @@ -77,51 +80,27 @@ END_SINK_MAP()
STDMETHOD(BeforeNavigate2)(IDispatch* dispatch, VARIANT* url, VARIANT* flags,
VARIANT* target_frame_name, VARIANT* post_data, VARIANT* headers,
VARIANT_BOOL* cancel);

HRESULT NavigateToCurrentUrlInCF(IBrowserService* browser);
STDMETHOD_(void, NavigateComplete2)(IDispatch* dispatch, VARIANT* url);

// mshtml sends an IOleCommandTarget::Exec of OLECMDID_HTTPEQUIV
// (and OLECMDID_HTTPEQUIV_DONE) as soon as it parses a meta tag.
// It also sends contents of the meta tag as an argument. IEFrame
// handles this in IBrowserService::OnHttpEquiv. So this allows
// us to sniff the META tag by simply patching it. The renderer
// switching can be achieved by cancelling original navigation
// switching can be achieved by canceling original navigation
// and issuing a new one using IWebBrowser2->Navigate2.
static HRESULT STDMETHODCALLTYPE OnHttpEquiv(
IBrowserService_OnHttpEquiv_Fn original_httpequiv,
IBrowserService* browser, IShellView* shell_view, BOOL done,
VARIANT* in_arg, VARIANT* out_arg);

const std::string& referrer() const {
return referrer_;
}

const std::wstring& url() const {
return url_;
}

// Called from HttpNegotiatePatch::BeginningTransaction when a request is
// being issued. We check the url and headers and see if there is a referrer
// header that we need to cache.
void OnBeginningTransaction(IWebBrowser2* browser, const wchar_t* url,
const wchar_t* headers,
const wchar_t* additional_headers);

// Returns the Bho instance for the current thread. This is returned from
// TLS.
static Bho* GetCurrentThreadBhoInstance();

static void ProcessOptInUrls(IWebBrowser2* browser, BSTR url);

protected:
bool PatchProtocolHandler(const CLSID& handler_clsid);

std::string referrer_;
std::wstring url_;

static base::LazyInstance<base::ThreadLocalPointer<Bho> >
bho_current_thread_instance_;
static _ATL_FUNC_INFO kBeforeNavigate2Info;
static _ATL_FUNC_INFO kNavigateComplete2Info;
};

#endif // CHROME_FRAME_BHO_H_
Expand Down
Loading

0 comments on commit 051236f

Please sign in to comment.