Pass SessionConfig by reference instead of pointer.

Pass SessionConfig by reference instead of pointer.

BUG=88600
TEST=Unittests

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

[Wez, 2011-09-10 01:04:29]

LGTM

http://codereview.chromium.org/7796026/diff/1/remoting/host/chromoting_host.cc
File remoting/host/chromoting_host.cc (right):

http://codereview.chromium.org/7796026/diff/1/remoting/host/chromoting_host.c...
remoting/host/chromoting_host.cc:457: DCHECK_EQ(context_->main_message_loop(),
MessageLoop::current());
We don't need this, surely, since the preceding lines ensure that we're on the
main message loop?

http://codereview.chromium.org/7796026/diff/1/remoting/protocol/session.h
File remoting/protocol/session.h (right):

http://codereview.chromium.org/7796026/diff/1/remoting/protocol/session.h#new...
remoting/protocol/session.h:94: //
ChromotocolServer::IncomingConnectionCallback. Ownership of |config| is
nit: This comment is out-of-date; consider updating.

http://codereview.chromium.org/7796026/diff/1/remoting/protocol/session_confi...
File remoting/protocol/session_config.cc (right):

http://codereview.chromium.org/7796026/diff/1/remoting/protocol/session_confi...
remoting/protocol/session_config.cc:191: *result->mutable_initial_resolution() =
(config.initial_resolution());
Typo!

[Sergey Ulanov, 2011-09-10 06:45:58]

http://codereview.chromium.org/7796026/diff/1/remoting/host/chromoting_host.cc
File remoting/host/chromoting_host.cc (right):

http://codereview.chromium.org/7796026/diff/1/remoting/host/chromoting_host.c...
remoting/host/chromoting_host.cc:457: DCHECK_EQ(context_->main_message_loop(),
MessageLoop::current());
On 2011/09/10 01:04:29, Wez wrote:
> We don't need this, surely, since the preceding lines ensure that we're on the
> main message loop?
Right. Added it by mistake.

http://codereview.chromium.org/7796026/diff/1/remoting/protocol/session.h
File remoting/protocol/session.h (right):

http://codereview.chromium.org/7796026/diff/1/remoting/protocol/session.h#new...
remoting/protocol/session.h:94: //
ChromotocolServer::IncomingConnectionCallback. Ownership of |config| is
On 2011/09/10 01:04:29, Wez wrote:
> nit: This comment is out-of-date; consider updating.

Done.

http://codereview.chromium.org/7796026/diff/1/remoting/protocol/session_confi...
File remoting/protocol/session_config.cc (right):

http://codereview.chromium.org/7796026/diff/1/remoting/protocol/session_confi...
remoting/protocol/session_config.cc:191: *result->mutable_initial_resolution() =
(config.initial_resolution());
On 2011/09/10 01:04:29, Wez wrote:
> Typo!
Where? The star in the beginning is needed. Removed parentheses, as they are
redundant.

[commit-bot: I haz the power, 2011-09-10 16:44:21]

Can't process patch for file remoting/protocol/session.h.
File's status is None, patchset upload is incomplete.

[Wez, 2011-09-11 01:17:40]

http://codereview.chromium.org/7796026/diff/1/remoting/protocol/session_confi...
File remoting/protocol/session_config.cc (right):

http://codereview.chromium.org/7796026/diff/1/remoting/protocol/session_confi...
remoting/protocol/session_config.cc:191: *result->mutable_initial_resolution() =
(config.initial_resolution());
On 2011/09/10 06:45:58, sergeyu wrote:
> On 2011/09/10 01:04:29, Wez wrote:
> > Typo!
> Where? The star in the beginning is needed. Removed parentheses, as they are
> redundant.

|result| is a pointer-to-CandidateSessionConfig, so -> should be sufficient, and
is what you used on previous lines.  What am I missing here?

[Sergey Ulanov, 2011-09-11 01:42:42]

On Sep 10, 2011 6:17 PM, <wez@chromium.org> wrote:
>
>
>
http://codereview.chromium.org/7796026/diff/1/remoting/protocol/session_confi...
> File remoting/protocol/session_config.cc (right):
>
>
http://codereview.chromium.org/7796026/diff/1/remoting/protocol/session_confi...
> remoting/protocol/session_config.cc:191:
> *result->mutable_initial_resolution() = (config.initial_resolution());
> On 2011/09/10 06:45:58, sergeyu wrote:
>>
>> On 2011/09/10 01:04:29, Wez wrote:
>> > Typo!
>> Where? The star in the beginning is needed. Removed parentheses, as
>
> they are
>>
>> redundant.
>
>
> |result| is a pointer-to-CandidateSessionConfig, so -> should be
> sufficient, and is what you used on previous lines.  What am I missing
> here?

The star is applied to the result of the function call, and it is a pointer.
Perhaps we need parentheses to make this more readable.

>
>
> http://codereview.chromium.org/7796026/

[Wez, 2011-09-11 01:48:42]

On 2011/09/11 01:42:42, sergeyu wrote:
> The star is applied to the result of the function call, and it is a pointer.
> Perhaps we need parentheses to make this more readable.

You're right, I was reading it wrongly.  Parentheses would definitely make that
clearer!