chrome/browser/media/router/media_route.h - Issue 1202963004: Gets presentation ID from route ID upon route creation, and overrides previous presentation ID with…

Side by Side Diff: chrome/browser/media/router/media_route.h

Issue 1202963004: Gets presentation ID from route ID upon route creation, and overrides previous presentation ID with… (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 5 years, 5 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

« chrome/browser/media/router/create_session_request.cc ('K') | « chrome/browser/media/router/create_session_request.cc ('k') | chrome/browser/media/router/media_route.cc » ('j') | chrome/browser/media/router/media_route.cc » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 // Copyright 2015 The Chromium Authors. All rights reserved.	1 // Copyright 2015 The Chromium Authors. All rights reserved.

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

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 #ifndef CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTE_H_	5 #ifndef CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTE_H_

6 #define CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTE_H_	6 #define CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTE_H_

7	7

8 #include <string>	8 #include <string>

9	9

10 #include "base/gtest_prod_util.h"	10 #include "base/gtest_prod_util.h"

(...skipping 61 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
72	72

73 private:	73 private:

74 MediaRoute::Id media_route_id_;	74 MediaRoute::Id media_route_id_;

75 MediaSource media_source_;	75 MediaSource media_source_;

76 MediaSink media_sink_;	76 MediaSink media_sink_;

77 std::string description_;	77 std::string description_;

78 bool is_local_;	78 bool is_local_;

79 MediaRouteState state_;	79 MediaRouteState state_;

80 };	80 };

81	81

	82 // Return a pair of Presentation ID and URL. If the input route ID is invalid,

	83 // a pair of empty strings is returned.
	Kevin M 2015/06/24 23:02:07 If a caller is going to use half the pair but disc If a caller is going to use half the pair but discard the other half, then this should be split into 2 functions vs. returning a pair to save on unnecessary processing. I recommend just making an additional function that takes a computed presentation URL and returns a StringPiece representing the source substr. Kevin M 2015/06/24 23:02:07 Presentation API support functions should be in pr Presentation API support functions should be in preso API files. Kevin M 2015/06/24 23:02:07 Document the parameters and return type Document the parameters and return type haibinlu 2015/06/24 23:25:12 Ack. Constructing presentation session message is Show quoted text On 2015/06/24 23:02:07, Kevin M wrote: > Presentation API support functions should be in preso API files. Ack. Constructing presentation session message is done here. Moving it to presention_service_delegate_impl makes sense, but incurs extra cost of blinding message_callback to itself, pass media_router.mojo's route message format, etc. To avoid using media_router.mojo's route message format in presentation files, we need to create another class shared by both, an unnecessary cost for carrying message along. haibinlu 2015/06/24 23:25:13 They are obvious in method signature. Not sure abo Show quoted text On 2015/06/24 23:02:07, Kevin M wrote: > Document the parameters and return type They are obvious in method signature. Not sure about the value of adding more doc. haibinlu 2015/06/24 23:25:13 yes, there are lot more fields in the route-id. I Show quoted text On 2015/06/24 23:02:07, Kevin M wrote: > If a caller is going to use half the pair but discard the other half, then this > should be split into 2 functions vs. returning a pair to save on unnecessary > processing. > > I recommend just making an additional function that takes a computed > presentation URL and returns a StringPiece representing the source substr. yes, there are lot more fields in the route-id. I do not want to do it in the CL. Prefer a separate CL for refactoring source/sink/route URN.
	84 std::pair<std::string, std::string> GetPresentationIdAndUrl(

	85 const MediaRoute::Id& id);

	86

82 } // namespace media_router	87 } // namespace media_router

83	88

84 #endif // CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTE_H_	89 #endif // CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTE_H_

OLD	NEW