third_party/WebKit/Source/modules/mediasession/MojoMediaArtwork.h - Issue 2367393002: Migrating MediaSession messages to mojo

Side by Side Diff

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Side by Side Diff: third_party/WebKit/Source/modules/mediasession/MojoMediaArtwork.h

Issue 2367393002: Migrating MediaSession messages to mojo (Closed)

Patch Set: blink typemap (need to revert blink typemap, only need content/ reivew) Created 4 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« no previous file with comments | « third_party/WebKit/Source/modules/mediasession/MediaSessionStructTraits.h ('k') | third_party/WebKit/Source/modules/mediasession/MojoMediaMetadata.h » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 // Copyright 2016 The Chromium Authors. All rights reserved.

	2 // Use of this source code is governed by a BSD-style license that can be

	3 // found in the LICENSE file.

	4

	5 #ifndef MojoMediaArtwork_h

	6 #define MojoMediaArtwork_h

	7

	8 #include "platform/weborigin/KURL.h"

	9 #include "public/platform/WebSize.h"

	10 #include "public/platform/WebString.h"

	11 #include "wtf/Vector.h"

	12

	13 namespace blink {

	14

	15 struct MojoMediaArtwork {
	Ken Rockot(use gerrit already) 2016/10/03 20:39:33 You should either not use a typemap at all in Blin You should either not use a typemap at all in Blink, or typemap to the existing MediaMetadata and MediaArtwork types. I really don't think we want to introduce new structure types just so we can typemap to them. The only reason to ever do that would be if you needed some extra validation logic for the type, which it seems you don't. Typemapping to the existing types is a bit more involved, because the StructTraits will have to define SetUpContext and TearDownContext in order to build up some intermediate state for serialization (e.g. to create a KURL from the src String), and you may need to provide some better way to construct the MediaArtwork and MediaMetadata types for deserialization. As already mentioned, it would also be reasonable to simply not typemap anything here (yet) and work directly with the generated mojom types for now. It's fine for content to typemap while Blink does not. Zhiqiang Zhang (Slow) 2016/10/03 21:00:20 OK. Thanks for your explanation! I was trying to m Show quoted text On 2016/10/03 20:39:33, Ken Rockot wrote: > You should either not use a typemap at all in Blink, or typemap to the existing > MediaMetadata and MediaArtwork types. I really don't think we want to introduce > new structure types just so we can typemap to them. The only reason to ever do > that would be if you needed some extra validation logic for the type, which it > seems you don't. > > Typemapping to the existing types is a bit more involved, because the > StructTraits will have to define SetUpContext and TearDownContext in order to > build up some intermediate state for serialization (e.g. to create a KURL from > the src String), and you may need to provide some better way to construct the > MediaArtwork and MediaMetadata types for deserialization. > > As already mentioned, it would also be reasonable to simply not typemap anything > here (yet) and work directly with the generated mojom types for now. It's fine > for content to typemap while Blink does not. OK. Thanks for your explanation! I was trying to mapping to blink::MediaMetadata and blink::MediaArtwork but failed (because of v8 dependency) so I came up this solution. As you said it's OK for content to typemap while Blink does not, I 'll revert this patch. No need to PTAL for tsepez now.
	16 KURL src;

	17 WebString type;

	18 Vector<WebSize> sizes;

	19 };

	20

	21 } // namespace blink

	22

	23 #endif // MojoMediaArtwork_h

OLD	NEW