Add support for 'orientation' in Manifest.

Add support for 'orientation' in Manifest.

BUG=366145
Committed: https://crrev.com/96297a73c48ff299e45eac62b967a8e333569602
Cr-Commit-Position: refs/heads/master@{#295449}

[mlamouri (slow - plz ping), 2014-09-17 13:56:30]

mlamouri@chromium.org changed reviewers:
+ avi@chromium.org, palmer@chromium.org

[mlamouri (slow - plz ping), 2014-09-17 13:56:30]

palmer@, can you look at the changes in:
 content/common/manifest_manager_messages.h

avi@, can you look at the changes in:
 content/

Thanks :)

[palmer, 2014-09-17 19:18:24]

https://codereview.chromium.org/576073004/diff/1/content/common/manifest_mana...
File content/common/manifest_manager_messages.h (right):

https://codereview.chromium.org/576073004/diff/1/content/common/manifest_mana...
content/common/manifest_manager_messages.h:25:
IPC_STRUCT_TRAITS_MEMBER(orientation)
Can you add an IPC_ENUM_TRAITS_MAX_VALUE for orientation?

[mlamouri (slow - plz ping), 2014-09-17 19:20:11]

https://codereview.chromium.org/576073004/diff/1/content/common/manifest_mana...
File content/common/manifest_manager_messages.h (right):

https://codereview.chromium.org/576073004/diff/1/content/common/manifest_mana...
content/common/manifest_manager_messages.h:25:
IPC_STRUCT_TRAITS_MEMBER(orientation)
On 2014/09/17 19:18:24, Chromium Palmer wrote:
> Can you add an IPC_ENUM_TRAITS_MAX_VALUE for orientation?

This is already being passed via IPC by the Screen Orientation API code so the
traits should be there. It wouldn't compile otherwise, would it?

[mlamouri (slow - plz ping), 2014-09-17 20:00:26]

On 2014/09/17 19:20:11, Mounir Lamouri wrote:
>
https://codereview.chromium.org/576073004/diff/1/content/common/manifest_mana...
> File content/common/manifest_manager_messages.h (right):
> 
>
https://codereview.chromium.org/576073004/diff/1/content/common/manifest_mana...
> content/common/manifest_manager_messages.h:25:
> IPC_STRUCT_TRAITS_MEMBER(orientation)
> On 2014/09/17 19:18:24, Chromium Palmer wrote:
> > Can you add an IPC_ENUM_TRAITS_MAX_VALUE for orientation?
> 
> This is already being passed via IPC by the Screen Orientation API code so the
> traits should be there. It wouldn't compile otherwise, would it?

It's set there:
https://code.google.com/p/chromium/codesearch#chromium/src/content/common/scr...

[palmer, 2014-09-17 20:24:38]

> This is already being passed via IPC by the Screen Orientation API code so the
> traits should be there. It wouldn't compile otherwise, would it?

Yep, it would compile. The serializer would just serialize an un-checked int.

I think that unless you declare the MAX_VALUE in both *messages.h files, the
serializer will only apply the restriction for the 1 message type.

[palmer, 2014-09-17 20:25:00]

> I think that unless you declare the MAX_VALUE in both *messages.h files, the
> serializer will only apply the restriction for the 1 message type.

Which is to say, yeah, you should re-state the MAX_VALUE here as well.

[palmer, 2014-09-17 20:29:18]

palmer@chromium.org changed reviewers:
+ tsepez@chromium.org

[palmer, 2014-09-17 20:29:18]

Adding tsepez for greater IPC guts wisdom. There was a notion that you could put
the MAX_VALUE restriction in 1 place, in a foo_param_traits.h file. That would
be nicer than duplicating it.

[palmer, 2014-09-17 20:42:59]

See e.g. content/common/resource_messages.h and its use of
content/common/content_param_traits_macros.h.

[Tom Sepez, 2014-09-17 20:46:55]

Actually, this looks OK, provided it compiles.  I didn't see the duplicate
declaration that palmer was implying.  Are there two places where there are
IPC_ENUM_TRAITS macros for the blink::WebScreenOrientationLockType?

[mlamouri (slow - plz ping), 2014-09-17 20:49:27]

On 2014/09/17 20:46:55, Tom Sepez wrote:
> Actually, this looks OK, provided it compiles.  I didn't see the duplicate
> declaration that palmer was implying.  Are there two places where there are
> IPC_ENUM_TRAITS macros for the blink::WebScreenOrientationLockType?

The IPC_ENUM_TRAITS is only set in content/common/screen_orientation_messages.h

[Tom Sepez, 2014-09-17 20:59:27]

OK. LGTM.

[Avi (use Gerrit), 2014-09-18 01:58:51]

lgtm

[mlamouri (slow - plz ping), 2014-09-18 09:37:12]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2014-09-18 09:37:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/576073004/1

[commit-bot: I haz the power, 2014-09-18 10:33:18]

Committed patchset #1 (id:1) as 5333fe29c9fadb0eec627fc6d31bd87e9ffe0451

[commit-bot: I haz the power, 2014-09-18 10:56:27]

Patchset 1 (id:??) landed as
https://crrev.com/96297a73c48ff299e45eac62b967a8e333569602
Cr-Commit-Position: refs/heads/master@{#295449}