Explicitly naming the data type for requestBody

Explicitly naming the data type for requestBody

In order to get the full documentation generated from JSON for
"details.requestBody" in the onBeforeRequest, the object type of
requestBody has to be a named reference.
This CL moves the inline definition to a named reference.


BUG=91191
TEST=N/A (No code change, documentation only.)

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=157615

[vabr (Chromium), 2012-09-19 08:16:08]

Hi Matt,

Could you please review this copy&paste in one JSON file?

What to look for:
1. General idea of introducing the named type RequestBody OK?
2. "description" for details.requestBody and for RequestBody makes sense after
splitting them?
3. NOTRY=true a good idea when no code is changed?

Thanks!
Vaclav

https://codereview.chromium.org/10948033/diff/1/chrome/common/extensions/api/...
File chrome/common/extensions/api/web_request.json (left):

https://codereview.chromium.org/10948033/diff/1/chrome/common/extensions/api/...
chrome/common/extensions/api/web_request.json:137: "description": "Experimental
feature, only available in DEV or CANARY channels. Container for request body
data. Only provided if extraInfoSpec contains 'requestBody'. Currently only
request body elements containing byte data or files are reported.",
I removed "Currently only request body elements containing byte data or files
are reported" entirely, because there are no other upload data elements than
TYPE_FILE or TYPE_BYTES.

[Matt Perry, 2012-09-19 19:07:58]

> 1. General idea of introducing the named type RequestBody OK?

Seems fine, though it's weird that you needed to do this.

> 2. "description" for details.requestBody and for RequestBody makes sense after
> splitting them?

With updated description. See comments.

> 3. NOTRY=true a good idea when no code is changed?

No, the .json files definitely affect compilation. Even if you think you're
making a small change, it could break stuff since our code depends on them.

https://codereview.chromium.org/10948033/diff/1/chrome/common/extensions/api/...
File chrome/common/extensions/api/web_request.json (right):

https://codereview.chromium.org/10948033/diff/1/chrome/common/extensions/api/...
chrome/common/extensions/api/web_request.json:111: "description": "This is a
container for request body data.",
I would copy part of the description of the variable: "Contains the HTTP request
body data." (or whatever you go with - see below).

https://codereview.chromium.org/10948033/diff/1/chrome/common/extensions/api/...
chrome/common/extensions/api/web_request.json:161: "description": "Contains the
upload data of the request. Experimental feature, only available in DEV or
CANARY channels. Only provided if extraInfoSpec contains 'requestBody'."
"Contains the upload data of the request." ? Makes it sound like UploadData. How
about "Contains the HTTP request body data."

[Matt Perry, 2012-09-19 19:08:12]

LGTM with comments addressed.

[vabr (Chromium), 2012-09-19 19:18:15]

Thanks, Matt!

Commit tests enabled, and in-line comments addressed.

Sending to CQ.

Vaclav

https://codereview.chromium.org/10948033/diff/1/chrome/common/extensions/api/...
File chrome/common/extensions/api/web_request.json (right):

https://codereview.chromium.org/10948033/diff/1/chrome/common/extensions/api/...
chrome/common/extensions/api/web_request.json:111: "description": "This is a
container for request body data.",
On 2012/09/19 19:07:58, Matt Perry wrote:
> I would copy part of the description of the variable: "Contains the HTTP
request
> body data." (or whatever you go with - see below).

Done.

https://codereview.chromium.org/10948033/diff/1/chrome/common/extensions/api/...
chrome/common/extensions/api/web_request.json:161: "description": "Contains the
upload data of the request. Experimental feature, only available in DEV or
CANARY channels. Only provided if extraInfoSpec contains 'requestBody'."
On 2012/09/19 19:07:58, Matt Perry wrote:
> "Contains the upload data of the request." ? Makes it sound like UploadData.
How
> about "Contains the HTTP request body data."

Done.

[commit-bot: I haz the power, 2012-09-19 19:18:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vabr@chromium.org/10948033/6001

[commit-bot: I haz the power, 2012-09-19 21:49:39]

Change committed as 157615