Add a X-Requested-With header to URL requests for PPAPI Flash (only).

Add a X-Requested-With header to URL requests for PPAPI Flash (only).

This is a revival of Trung's patch in https://codereview.chromium.org/10886047/. 
The difference is that we provide the header only for the Shockwave Flash
plugin (plus a test plugin).

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

[Tom Sepez, 2014-08-12 00:16:54]

Brett, please review (you were the reviewer on the original VTL patch).

[brettw, 2014-08-18 21:22:48]

lgtm

https://codereview.chromium.org/451923002/diff/40001/content/child/request_ex...
File content/child/request_extra_data.h (right):

https://codereview.chromium.org/451923002/diff/40001/content/child/request_ex...
content/child/request_extra_data.h:99: return x_requested_with_;
I think trung's patch called this just "requested with". Adding the "x_" here I
find difficult to read and possibly confusing. WE we just skip the prefix at
this level?

https://codereview.chromium.org/451923002/diff/40001/content/renderer/pepper/...
File content/renderer/pepper/url_request_info_util.cc (right):

https://codereview.chromium.org/451923002/diff/40001/content/renderer/pepper/...
content/renderer/pepper/url_request_info_util.cc:129: return "";
When returning an empty string, can you do
  return std::string();
instead? This saves generated code in my tests, and is also more precise w.r.t.
types.

[Tom Sepez, 2014-08-18 22:15:24]

https://codereview.chromium.org/451923002/diff/40001/content/child/request_ex...
File content/child/request_extra_data.h (right):

https://codereview.chromium.org/451923002/diff/40001/content/child/request_ex...
content/child/request_extra_data.h:99: return x_requested_with_;
On 2014/08/18 21:22:48, brettw wrote:
> I think trung's patch called this just "requested with". Adding the "x_" here
I
> find difficult to read and possibly confusing. WE we just skip the prefix at
> this level?
done.

https://codereview.chromium.org/451923002/diff/40001/content/renderer/pepper/...
File content/renderer/pepper/url_request_info_util.cc (right):

https://codereview.chromium.org/451923002/diff/40001/content/renderer/pepper/...
content/renderer/pepper/url_request_info_util.cc:129: return "";
On 2014/08/18 21:22:48, brettw wrote:
> When returning an empty string, can you do
>   return std::string();
> instead? This saves generated code in my tests, and is also more precise
w.r.t.
> types.

Done.

[Tom Sepez, 2014-08-19 21:31:57]

Patch set #6 addresses two issues with the tests.  The first is that since only
the PPAPITest sub-class initializes the ppapi library using naming consistent
with the later test for including the x-requested-with header, the result for it
will be different than that obtained when the other sub-classes
(PPAPINaClGLibcTest etc.) run the same test case. So we batch this test under
the "trusted" list to keep the others from running it.

The second issue is that TestGlobals is a liar, in that it's not derived from
HostGlobals, yet its IsHostGlobals() method returns true, and it is then badly
cast into a HostGlobals under HostGlobals::Get(). Thus CreateWebURLRequest() has
to know when this is a test case, so that it can avoid this path altogether. 
Unfortunately, URLRequestInfoTest currently also lies about its instance number,
claiming to be 1234. While the code is trying to handle this special case by
looking for instance 0, it doesn't work to simply change 1234 to 0, since this
triggers other asserts.  Instead, use -1 as a placeholder for the test instance.

[Tom Sepez, 2014-08-19 22:06:15]

+bbudge for chrome/test/ppapi OWNERS review.

[bbudge, 2014-08-19 22:29:15]

chrome/tests/ppapi LGTM with 2 comments.

https://codereview.chromium.org/451923002/diff/100001/chrome/test/ppapi/ppapi...
File chrome/test/ppapi/ppapi_test.cc (right):

https://codereview.chromium.org/451923002/diff/100001/chrome/test/ppapi/ppapi...
chrome/test/ppapi/ppapi_test.cc:300:
pepper_plugin.append(FILE_PATH_LITERAL("#PPAPI Tests"));
This doesn't have to match the value you test for in MakeXRequestedWithValue?
("PPAPITests")
Or does the space get filtered?

https://codereview.chromium.org/451923002/diff/100001/content/renderer/pepper...
File content/renderer/pepper/pepper_url_request_unittest.cc (right):

https://codereview.chromium.org/451923002/diff/100001/content/renderer/pepper...
content/renderer/pepper/pepper_url_request_unittest.cc:56: URLRequestInfoTest()
: pp_instance_(-1) {}
Maybe a comment explaining this unusual instance value.

[Tom Sepez, 2014-08-19 22:30:35]

On 2014/08/19 22:06:15, Tom Sepez wrote:
> +bbudge for chrome/test/ppapi OWNERS review.

Also content/renderer/pepper OWNERS review, Bill.

[Tom Sepez, 2014-08-19 22:31:22]

> Or does the space get filtered?
> 
Yes, exactly.

>
https://codereview.chromium.org/451923002/diff/100001/content/renderer/pepper...
> File content/renderer/pepper/pepper_url_request_unittest.cc (right):
> 
>
https://codereview.chromium.org/451923002/diff/100001/content/renderer/pepper...
> content/renderer/pepper/pepper_url_request_unittest.cc:56:
URLRequestInfoTest()
> : pp_instance_(-1) {}
> Maybe a comment explaining this unusual instance value.
Sure.

[Tom Sepez, 2014-08-20 16:39:53]

The CQ bit was checked by tsepez@chromium.org

[commit-bot: I haz the power, 2014-08-20 16:40:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tsepez@chromium.org/451923002/120001

[commit-bot: I haz the power, 2014-08-20 17:51:57]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-20 19:23:35]

Committed patchset #7 (120001) as 290887