HandleBeforeBrowse wasn't able to modify CefRequest

[heshiming]

Hi,

I'm trying to set some internal stuff in the HeaderMap during HandleBeforeBrowse, so that my custom scheme handler can see them. It looks impossible because even though I called request->SetHeaderMap to provide a new map, the |request| argument in CefSchemeHandler::ProcessRequest still receives the old and untouched one. I traced, and HandleBeforeBrowse was called before ProcessRequest .

Any idea this is happening? How do I fix it? Thanks!

[heshiming]

Well, I'm answering this question myself.

It turned out that the CefRequest object "HandleBeforeBrowse" is seeing, is created only for the sake of this call. It's destroyed afterwards. Modifications to it are hence lost, and not visible to WebKit.

To workaround this problem, in BrowserWebViewDelegate::decidePolicyForNavigation of browser_webview_delegate.cc of "libcef_static". I added:

Code: Select all

    CefRequest::HeaderMap aftermap;
    static_cast<CefRequestImpl*>(req.get())->GetHeaderMap(aftermap);
    for (CefRequest::HeaderMap::iterator it = aftermap.begin();
      it != aftermap.end(); it++)
      ((WebURLRequest&)request).setHTTPHeaderField(webkit_glue::StdStringToWebString(WideToUTF8(it->first)),
        webkit_glue::StdStringToWebString(WideToUTF8(it->second)));


right after the "HandleBeforeBrowse" call. Though obvious this only took care of the header map.

I noticed that this is probably not a good idea because you have a "const" restriction on the |request| argument. But if HandleBeforeBrowse were to be able to modify something, |request| cannot be const.

I'm using this mechanism to add custom headers to keep track of multi-window situations.

[magreenblatt]

I agree that the current documented usage of HandleBeforeBrowse() is inconsistent with the underlying Chromium code. As you say, the 'const'ness of |request| in decidePolicyForNavigation() indicates that we shouldn't allow modification of the CefRequest argument passed to HandleBeforeBrowse(). One valid approach in your code could be to cancel the current load by returning RV_HANDLED from HandleBeforeBrowse() and then call LoadRequest() on the same frame with the modified request information.

[Eric]

I was interested in similar functionality except I wanted to also be able to change headers dynamically for resource loads as well. I'll include my code. I've tried to observe existing style. It would be great if something like this could be merged into CEF.

In browser_resource_loader_bridge.cc, RequestProxy::AsyncStart():

Code: Select all

CefHandler::RetVal rv = handler->HandleBeforeResourceLoad(browser_, request, redirectUrl, resourceStream, mimeType, loadFlags);
// Begin new code
request->GetHeaderMap(headerMap);

CefRequest::HeaderMap::iterator referrer = headerMap.find(L"Referrer");

if(referrer != headerMap.end()) {
  params->referrer = GURL(WideToUTF8(referrer->second));
  headerMap.erase(referrer);
}
params->headers = CefRequestImpl::GenerateHeaderString(headerMap);
// End new code
if(rv == RV_HANDLED) {

In request_impl.cc:

Code: Select all

std::string CefRequestImpl::GenerateHeaderString(const HeaderMap& map)
{
  std::string headerString;

  for(HeaderMap::const_iterator it = map.begin(); it != map.end(); ++it) {
    if(!it->first.empty()) {
      if(!headerString.empty())
        headerString += "\r\n";

      headerString += WideToUTF8(it->first) + ": " + WideToUTF8(it->second);
    }
  }

  return headerString;
}

[magreenblatt]

HandleBeforeResourceLoad() does seem like a more appropriate place than HandleBeforeBrowse() to modify the request. If we allow modification of headers in HandleBeforeResourceLoad() then we should also allow modification of the remaining request information including method, URL and upload data. I've updated issue 41 to reflect this idea. http://code.google.com/p/chromiumembedd ... tail?id=41

[Eric]

I would be happy to implement your spec for this, Marshall. Or, of course, you can. I'm just eager to see this implemented in the near term.

[magreenblatt]

Hi Eric,

Go ahead

- Marshall

[Eric]

I attached a patch to the issue.