Carry over modifications to request object in CefHandler::HandleBeforeResourceLoad()

[GoogleCodeExporter]

Allow specification of header values and character set for the resource
load response. This is necessary if the client wants to specify cookie or
no-cache headers, for instance. If |mimeType| and/or |charSet| are empty
CEF will try to find the appropriate header in |headerMap| and set the
values accordingly.

virtual RetVal HandleBeforeResourceLoad(CefRefPtr<CefBrowser> browser,
                                  CefRefPtr<CefRequest> request,
                                  std::wstring& redirectUrl,
                                  CefRefPtr<CefStreamReader>& resourceStream,
                                  HeaderMap& headerMap,
                                  std::wstring& mimeType,
                                  std::wstring& charSet,
                                  int loadFlags) =0;

Original issue reported on code.google.com by magreenb...@gmail.com on 5 Jul 2009 at 5:49

[GoogleCodeExporter]

Instead of adding additional parameters we should carry over modifications to 
the
|request| object.  That way the user can change the URL, method and post data in
addition to the headers.

Original comment by magreenb...@gmail.com on 21 Feb 2010 at 3:53

• Changed title: Carry over modifications to request object in CefHandler::HandleBeforeResourceLoad()

[GoogleCodeExporter]

Patch attached.

Original comment by esiro...@gmail.com on 24 Feb 2010 at 1:36

Attachments:

• patch.txt

[GoogleCodeExporter]

Your patch looks good generally but there's one thing we need to think about 
further.

Your patch treats changing the request's URL member the same as setting the
|redirectUrl| parameter. However, setting the |redirectUrl| parameter causes the
current request is canceled and the equivalent of a 403 redirect is executed.  
Is
this the behavior that we want when changing the request's URL member?

I see two potential options.

(1) Treat changing the request's URL member the same as setting |redirectUrl| 
(which
your patch does).
(2) Proceed with the modified request, including the changed URL member.

If we go with #1 then the |redirectUrl| parameter becomes redundant.  However, 
#1
also builds the erroneous expectation that the other request values will also 
be sent
to the new URL, which is not the case for |redirectUrl|.  If with go with #2 
then the
user's expectation is preserved and the user can do other cool things like
transparently proxying certain requests.

Consequently, I think #2 might be the clearer and more useful approach.  What 
do you
think?

Original comment by magreenb...@gmail.com on 24 Feb 2010 at 6:07

[GoogleCodeExporter]

I'm glad you took the time to consider the request URL aspect of the change.  I 
was 
venturing a guess as to what the right thing to do was.  I didn't even know 
whether 
changing the request's URL at the point at which the callback is fired would 
work.  
(I guess I could have tried, huh?)

I agree #2 seems like a better approach primarily because it prevents confusion 
with 
respect to |redirectUrl|.

Would you like me to update the patch?  I'm also fine with it if you want to 
take it 
from here to completion.

Original comment by esiro...@gmail.com on 25 Feb 2010 at 5:00

[GoogleCodeExporter]

How about updating the patch and testing that it works? ;-)

Original comment by magreenb...@gmail.com on 25 Feb 2010 at 6:22

[GoogleCodeExporter]

I've updated esiroker's patch to be compatible with r94.  I also incorporated 
the  URL suggestion above, which was done with the following change:

-        if(requestUrl != originalUrl)
-          redirectUrl = requestUrl;

+        if(requestUrl != originalUrl) {
+          params->url = GURL(WideToUTF8(requestUrl));
+          redirectUrl.clear(); // Request URL trumps redirect URL
+        }

With this change, I was able to swap out resources in cefclient by changing the 
request url rather than using resource streams.

Original comment by mikeyk...@gmail.com on 19 Aug 2010 at 4:57

Attachments:

• resource94.txt

[GoogleCodeExporter]

Committed with minor style changes as revision 95.

Original comment by magreenb...@gmail.com on 30 Aug 2010 at 8:27

• Changed state: Fixed