Add GetPluginRefererURL to PPB_URLUtil_Dev interface to get the 'Referer' HTTP header value that wa…

Add GetPluginRefererURL to PPB_URLUtil_Dev interface to get the 'Referer' HTTP header value that was sent by request that loaded plug-in. 
Sometimes Pepper plug-in needs to create additional HTTP request to its source URL (for example, PDF plug-in downloads parts of the file using HTTP Range requests). These requests will fail for servers that check Referer header (example: http://fs51.www.ex.ua/get/96354baf7547114fa116c56ecdbdb3ee/66511913/FRoziner_Iskusstvo_Ciurlionisa_1993.pdf).

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221920

[bbudge, 2013-08-27 18:16:07]

https://codereview.chromium.org/23444004/diff/1/ppapi/api/dev/ppb_url_util_de...
File ppapi/api/dev/ppb_url_util_dev.idl (right):

https://codereview.chromium.org/23444004/diff/1/ppapi/api/dev/ppb_url_util_de...
ppapi/api/dev/ppb_url_util_dev.idl:11: M17 = 0.6
You should version the interface (create a 0.7). For an example of this see
ppb_file_chooser_dev.idl:
https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/api/dev/ppb_...

https://codereview.chromium.org/23444004/diff/1/ppapi/api/dev/ppb_url_util_de...
ppapi/api/dev/ppb_url_util_dev.idl:151: * that loaded plug-in. Undefined value
means 'Referer' header is absent.
Wording is a bit awkward. How about this?
Returns the Referer URL of the HTTP request that loaded the plugin. This is the
value of the 'Referer' header of the request. An undefined value means the
'Referer' header was absent.

[omakovski, 2013-08-27 19:33:26]

Fixed.

[bbudge, 2013-08-27 22:36:30]

https://codereview.chromium.org/23444004/diff/14001/ppapi/api/dev/ppb_url_uti...
File ppapi/api/dev/ppb_url_util_dev.idl (right):

https://codereview.chromium.org/23444004/diff/14001/ppapi/api/dev/ppb_url_uti...
ppapi/api/dev/ppb_url_util_dev.idl:12: M30 = 0.7
Don't forget to re-run the generator.

[piman, 2013-08-27 22:48:21]

https://codereview.chromium.org/23444004/diff/14001/content/renderer/pepper/p...
File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):

https://codereview.chromium.org/23444004/diff/14001/content/renderer/pepper/p...
content/renderer/pepper/pepper_plugin_instance_impl.cc:2508: const
WebURLRequest& request = frame->dataSource()->originalRequest();
Mmh, this is the referrer of the frame containing the plugin, which is only
going to be the referrer of the plugin if it is loaded as a full-frame. Should
this be named GetContainingFrameReferrerURL? Or should it be limited to
full-frame plugins (and otherwise return the frame URL, maybe?)

https://codereview.chromium.org/23444004/diff/14001/ppapi/api/dev/ppb_url_uti...
File ppapi/api/dev/ppb_url_util_dev.idl (right):

https://codereview.chromium.org/23444004/diff/14001/ppapi/api/dev/ppb_url_uti...
ppapi/api/dev/ppb_url_util_dev.idl:151: * Returns the Referer URL of the HTTP
request that loaded the plugin. This is
nit: s/Referer/Referrer/ when you're not explicitly specifying the HTTP header
name. (also in every other file)

[yzshen1, 2013-08-28 17:44:29]

https://codereview.chromium.org/23444004/diff/14001/ppapi/api/dev/ppb_url_uti...
File ppapi/api/dev/ppb_url_util_dev.idl (right):

https://codereview.chromium.org/23444004/diff/14001/ppapi/api/dev/ppb_url_uti...
ppapi/api/dev/ppb_url_util_dev.idl:12: M30 = 0.7
Now the earliest milestone is M31.

https://codereview.chromium.org/23444004/diff/14001/ppapi/thunk/ppb_url_util_...
File ppapi/thunk/ppb_url_util_thunk.cc (right):

https://codereview.chromium.org/23444004/diff/14001/ppapi/thunk/ppb_url_util_...
ppapi/thunk/ppb_url_util_thunk.cc:73: &GetPluginRefererURL
You need to have a different struct for 0.7 interface.

[omakovski, 2013-08-28 19:29:22]

Thank you all for pointing out problems in my patch. I hope I have done it right
this time.

There was an error during presubmit check:
** Presubmit ERRORS **
Missing PPAPI IDL for stable interface:

***************
  ppapi/c/pp_macros.h
***************
I didn't know how to deal with it so I used --no_presubmit parameter.

I have changed PepperPluginInstanceImpl::GetPluginReferrerURL() to return the
same result as PepperPluginInstanceImpl::GetDocumentURL() for plugin that is not
full-frame.

[piman, 2013-08-28 19:47:27]

LGTM for content/

[bbudge, 2013-08-28 20:35:35]

https://codereview.chromium.org/23444004/diff/51001/ppapi/api/dev/ppb_url_uti...
File ppapi/api/dev/ppb_url_util_dev.idl (right):

https://codereview.chromium.org/23444004/diff/51001/ppapi/api/dev/ppb_url_uti...
ppapi/api/dev/ppb_url_util_dev.idl:9: 
If you put this here:
[generate_thunk]

Can the thunk be auto-generated?

[omakovski, 2013-08-29 10:28:31]

On 2013/08/28 20:35:35, bbudge1 wrote:
>
https://codereview.chromium.org/23444004/diff/51001/ppapi/api/dev/ppb_url_uti...
> File ppapi/api/dev/ppb_url_util_dev.idl (right):
> 
>
https://codereview.chromium.org/23444004/diff/51001/ppapi/api/dev/ppb_url_uti...
> ppapi/api/dev/ppb_url_util_dev.idl:9: 
> If you put this here:
> [generate_thunk]
> 
> Can the thunk be auto-generated?

Yes, it was generated after adding this directive. Contents stayed the same thus
thunk is not updated by Patch Set 5.

[bbudge, 2013-08-29 13:15:19]

LGTM
You will need a security review for ppapi_messages.h
Probably from one of tsepez, palmer, or jschuh

[Tom Sepez, 2013-08-29 20:07:56]

https://codereview.chromium.org/23444004/diff/66001/content/renderer/pepper/p...
File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):

https://codereview.chromium.org/23444004/diff/66001/content/renderer/pepper/p...
content/renderer/pepper/pepper_plugin_instance_impl.cc:2507: return
ppapi::PPB_URLUtil_Shared::GenerateURLReturn(document.url(),
Does this leak info that would have otherwise been hidden, given that referrers
are suppressed for HTTPS to HTTP or HTTPS to HTTPS (different origin) requests?

[omakovski, 2013-08-29 21:22:09]

On 2013/08/29 20:07:56, Tom Sepez wrote:
>
https://codereview.chromium.org/23444004/diff/66001/content/renderer/pepper/p...
> File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):
> 
>
https://codereview.chromium.org/23444004/diff/66001/content/renderer/pepper/p...
> content/renderer/pepper/pepper_plugin_instance_impl.cc:2507: return
> ppapi::PPB_URLUtil_Shared::GenerateURLReturn(document.url(),
> Does this leak info that would have otherwise been hidden, given that
referrers
> are suppressed for HTTPS to HTTP or HTTPS to HTTPS (different origin)
requests?

This is exactly the same information that is returned by
PepperPluginInstanceImpl::GetDocumentURL(). That means it is already available
to plugins.

[Tom Sepez, 2013-08-29 21:26:10]

lgtm

[omakovski, 2013-09-02 16:26:50]

yzshen1, brettw, binji, please let me know if there are any more issues with
this patch.

[yzshen1, 2013-09-03 16:50:41]

LGTM

[bbudge, 2013-09-06 20:35:44]

https://codereview.chromium.org/23444004/diff/66001/content/renderer/pepper/p...
File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):

https://codereview.chromium.org/23444004/diff/66001/content/renderer/pepper/p...
content/renderer/pepper/pepper_plugin_instance_impl.cc:2507: return
ppapi::PPB_URLUtil_Shared::GenerateURLReturn(document.url(),
On 2013/08/29 20:07:56, Tom Sepez wrote:
> Does this leak info that would have otherwise been hidden, given that
referrers
> are suppressed for HTTPS to HTTP or HTTPS to HTTPS (different origin)
requests?

It's a Dev interface, so can't be used by apps from the web store. It's used
mostly for testing. We might need a note that this shouldn't ever be exposed to
real apps.

Also, it's not proxied to NaCl and likely won't ever be, since the
implementation pulls in suprising things, including ICU.

[noelallen1, 2013-09-06 22:51:38]

IDL lgtm

[commit-bot: I haz the power, 2013-09-06 23:48:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/omakovski@yandex-team.ru/23444004/66001

[commit-bot: I haz the power, 2013-09-06 23:48:57]

Failed to apply patch for ppapi/c/pp_macros.h:
While running patch -p0 --forward --force --no-backup-if-mismatch;
  patching file ppapi/c/pp_macros.h
  Hunk #1 FAILED at 3.
  1 out of 1 hunk FAILED -- saving rejects to file ppapi/c/pp_macros.h.rej

Patch:       ppapi/c/pp_macros.h
Index: ppapi/c/pp_macros.h
===================================================================
--- ppapi/c/pp_macros.h	(revision 219784)
+++ ppapi/c/pp_macros.h	(working copy)
@@ -3,13 +3,13 @@
  * found in the LICENSE file.
  */
 
-/* From pp_macros.idl modified Tue Jul  2 14:15:05 2013. */
+/* From pp_macros.idl modified Wed Mar 06 13:00:44 2013. */
 
 #ifndef PPAPI_C_PP_MACROS_H_
 #define PPAPI_C_PP_MACROS_H_
 
 
-#define PPAPI_RELEASE 30
+#define PPAPI_RELEASE 31
 
 /**
  * @file

[commit-bot: I haz the power, 2013-09-07 09:16:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/omakovski@yandex-team.ru/23444004/84001

[commit-bot: I haz the power, 2013-09-07 15:47:51]

Change committed as 221920