Turned UiStrings into a singleton so that the continue window does not depend on ChromotingHost.

Moved the localized strings from ChromotinHost to HostUserInterface, closer to the UI objects that consume them. Removed a dependency on ChromotingHost from the continue window UI wrapper.

BUG=104544
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=177207

[alexeypa (please no reviews), 2013-01-15 20:43:17]

PTAL.

[Wez, 2013-01-15 20:58:01]

On 2013/01/15 20:43:17, alexeypa wrote:
> PTAL.

Why does it need to be a singleton, rather than simply being passed to
ContinueWindow as a parameter?

[Lambros, 2013-01-15 21:07:57]

On 2013/01/15 20:58:01, Wez wrote:
> On 2013/01/15 20:43:17, alexeypa wrote:
> > PTAL.
> 
> Why does it need to be a singleton, rather than simply being passed to
> ContinueWindow as a parameter?

+1
Maybe add a SetUiStrings() method to the disconnect/continue window interfaces
(or modify one of the existing methods)?

[alexeypa (please no reviews), 2013-01-15 21:09:52]

On 2013/01/15 20:58:01, Wez wrote:
> On 2013/01/15 20:43:17, alexeypa wrote:
> > PTAL.
> 
> Why does it need to be a singleton, rather than simply being passed to
> ContinueWindow as a parameter?

I tried that. It turned out to be very inconvenient due to multiple layers of
calls. Since it is used as a singleton anyway it seems to be reasonable to make
it official.

[Wez, 2013-01-15 21:16:40]

On 2013/01/15 21:09:52, alexeypa wrote:
> On 2013/01/15 20:58:01, Wez wrote:
> > On 2013/01/15 20:43:17, alexeypa wrote:
> > > PTAL.
> > 
> > Why does it need to be a singleton, rather than simply being passed to
> > ContinueWindow as a parameter?
> 
> I tried that. It turned out to be very inconvenient due to multiple layers of
> calls. Since it is used as a singleton anyway it seems to be reasonable to
make
> it official.

Strictly speaking the strings might be translated to different languages in
different plugin instances (though I doubt it).

How about making it ref-counted-non-thread-safe and having the plugin instance
create it and share it w/ ContinueWindow, etc?

[Lambros, 2013-01-15 21:27:19]

If there really are multiple layers of calls to deal with, I think it's
better to keep the object dependencies explicit, rather than hiding them
inside a global singleton.

DisconnectWindow already had a UiStrings parameter before this change.
Sounds like we just want to add it to ContinueWindow also?


On Tue, Jan 15, 2013 at 1:16 PM, <wez@chromium.org> wrote:

> On 2013/01/15 21:09:52, alexeypa wrote:
>
>> On 2013/01/15 20:58:01, Wez wrote:
>> > On 2013/01/15 20:43:17, alexeypa wrote:
>> > > PTAL.
>> >
>> > Why does it need to be a singleton, rather than simply being passed to
>> > ContinueWindow as a parameter?
>>
>
>  I tried that. It turned out to be very inconvenient due to multiple
>> layers of
>> calls. Since it is used as a singleton anyway it seems to be reasonable to
>>
> make
>
>> it official.
>>
>
> Strictly speaking the strings might be translated to different languages in
> different plugin instances (though I doubt it).
>
> How about making it ref-counted-non-thread-safe and having the plugin
> instance
> create it and share it w/ ContinueWindow, etc?
>
>
https://chromiumcodereview.**appspot.com/11886051/<https://chromiumcodereview...
>

[alexeypa (please no reviews), 2013-01-16 00:05:50]

Got rid of singleton. PTAL.

[Lambros, 2013-01-16 02:30:47]

Please update the description.

LGTM with nits, though I'm not sure about use of const *

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/conti...
File remoting/host/continue_window.h (right):

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/conti...
remoting/host/continue_window.h:31: static scoped_ptr<ContinueWindow>
Create(const UiStrings* ui_strings);
nit: Does it make sense to pass by const ref everywhere?

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/conti...
File remoting/host/continue_window_gtk.cc (right):

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/conti...
remoting/host/continue_window_gtk.cc:34: const UiStrings* ui_strings_;
optional: This could be const ref as well.

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/ui_st...
File remoting/host/ui_strings.cc (right):

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/ui_st...
remoting/host/ui_strings.cc:34: }  // anmespace remoting
typo

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/ui_st...
File remoting/host/ui_strings.h (right):

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/ui_st...
remoting/host/ui_strings.h:22: public:
Do we normally add public: at the top of struct?

[alexeypa (please no reviews), 2013-01-16 16:55:35]

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/conti...
File remoting/host/continue_window.h (right):

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/conti...
remoting/host/continue_window.h:31: static scoped_ptr<ContinueWindow>
Create(const UiStrings* ui_strings);
On 2013/01/16 02:30:47, Lambros wrote:
> nit: Does it make sense to pass by const ref everywhere?

The reason for using a pointer that it implies that the pointed value can be
used after the method returns stronger than a reference.

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/conti...
File remoting/host/continue_window_gtk.cc (right):

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/conti...
remoting/host/continue_window_gtk.cc:34: const UiStrings* ui_strings_;
On 2013/01/16 02:30:47, Lambros wrote:
> optional: This could be const ref as well.

See my previous comment.

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/ui_st...
File remoting/host/ui_strings.cc (right):

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/ui_st...
remoting/host/ui_strings.cc:34: }  // anmespace remoting
On 2013/01/16 02:30:47, Lambros wrote:
> typo

Done.

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/ui_st...
File remoting/host/ui_strings.h (right):

https://chromiumcodereview.appspot.com/11886051/diff/8001/remoting/host/ui_st...
remoting/host/ui_strings.h:22: public:
On 2013/01/16 02:30:47, Lambros wrote:
> Do we normally add public: at the top of struct?

Done.

[commit-bot: I haz the power, 2013-01-16 16:55:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alexeypa@chromium.org/11886051/16001

[commit-bot: I haz the power, 2013-01-16 17:28:28]

Retried try job too often on win for step(s) compile

[commit-bot: I haz the power, 2013-01-16 18:09:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alexeypa@chromium.org/11886051/8003

[commit-bot: I haz the power, 2013-01-16 20:30:10]

Change committed as 177207