services/ui: Create a proper component for the client-lib.

services/ui: Create a proper component for the client-lib.

Instead of a source_set, create a component for the client-lib.

BUG=none

[sadrul, 2016-07-28 03:23:18]

The CQ bit was checked by sadrul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-28 03:23:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sadrul, 2016-07-28 03:30:13]

sadrul@chromium.org changed reviewers:
+ sky@chromium.org, tsepez@chromium.org

[sadrul, 2016-07-28 03:30:14]

tsepez@ Please review //services/ui/public/cpp/property_type_converters.h

sky@ Please review all

[commit-bot: I haz the power, 2016-07-28 03:50:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-28 03:50:26]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sadrul, 2016-07-28 04:16:31]

The CQ bit was checked by sadrul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-28 04:16:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-28 05:08:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-28 05:08:24]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[sadrul, 2016-07-28 05:25:50]

The CQ bit was checked by sadrul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-28 05:26:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-28 06:20:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-28 06:20:15]

Dry run: This issue passed the CQ dry run.

[sadrul, 2016-07-28 17:23:55]

sadrul@chromium.org changed reviewers:
+ ben@chromium.org

[sadrul, 2016-07-28 17:23:56]

+ben@ since sky@ is ooo

[Tom Sepez, 2016-07-28 17:56:45]

On 2016/07/28 03:30:14, sadrul wrote:
> tsepez@ Please review //services/ui/public/cpp/property_type_converters.h
> 
> sky@ Please review all

RS LGTM for adding 'EXPORT keyword to type converters.

[Ben Goodger (Google), 2016-07-28 19:19:46]

Why do you need to do this?

The component build is a vestigial artifact of the chrome build process,
something I hope we'll be able to undo at some future point with //service-based
componentization, & so for the past 2+ years we've avoided leaking it into
//services code.

Client libs tend to be pretty tiny, even complex ones like this - I figured as
long as the targets that depended on it were components it'd be ok.

[sadrul, 2016-07-28 19:44:09]

On 2016/07/28 19:19:46, Ben Goodger (Google) wrote:
> Why do you need to do this?
> 
> The component build is a vestigial artifact of the chrome build process,
> something I hope we'll be able to undo at some future point with
//service-based
> componentization, & so for the past 2+ years we've avoided leaking it into
> //services code.
> 
> Client libs tend to be pretty tiny, even complex ones like this - I figured as
> long as the targets that depended on it were components it'd be ok.

Without this, things like static variables or singletons defined in the
client-lib cannot be used (see for example
https://codereview.chromium.org/2188813003/, which uses a static const
kWindowForAcceleratedWidget, and https://codereview.chromium.org/2184943002/,
which moves a client-side singleton into the client-lib).

[Ben Goodger (Google), 2016-07-28 20:23:39]

On 2016/07/28 19:44:09, sadrul wrote:
> On 2016/07/28 19:19:46, Ben Goodger (Google) wrote:
> > Why do you need to do this?
> > 
> > The component build is a vestigial artifact of the chrome build process,
> > something I hope we'll be able to undo at some future point with
> //service-based
> > componentization, & so for the past 2+ years we've avoided leaking it into
> > //services code.
> > 
> > Client libs tend to be pretty tiny, even complex ones like this - I figured
as
> > long as the targets that depended on it were components it'd be ok.
> 
> Without this, things like static variables or singletons defined in the
> client-lib cannot be used (see for example
> https://codereview.chromium.org/2188813003/, which uses a static const
> kWindowForAcceleratedWidget, and https://codereview.chromium.org/2184943002/,
> which moves a client-side singleton into the client-lib).

Ah right. This happened before and Peng tried to do this, and I think my
response was "don't do that" :-)

[Ben Goodger (Google), 2016-07-28 20:26:31]

statics etcs are the devil. The client lib should be stateless. if you need
to hold some state somewhere, inject it into the client lib.

-Ben

On Thu, Jul 28, 2016 at 1:23 PM, <ben@chromium.org> wrote:

> On 2016/07/28 19:44:09, sadrul wrote:
> > On 2016/07/28 19:19:46, Ben Goodger (Google) wrote:
> > > Why do you need to do this?
> > >
> > > The component build is a vestigial artifact of the chrome build
> process,
> > > something I hope we'll be able to undo at some future point with
> > //service-based
> > > componentization, & so for the past 2+ years we've avoided leaking it
> into
> > > //services code.
> > >
> > > Client libs tend to be pretty tiny, even complex ones like this - I
> figured
> as
> > > long as the targets that depended on it were components it'd be ok.
> >
> > Without this, things like static variables or singletons defined in the
> > client-lib cannot be used (see for example
> > https://codereview.chromium.org/2188813003/, which uses a static const
> > kWindowForAcceleratedWidget, and
> https://codereview.chromium.org/2184943002/,
> > which moves a client-side singleton into the client-lib).
>
> Ah right. This happened before and Peng tried to do this, and I think my
> response was "don't do that" :-)
>
> https://codereview.chromium.org/2189893002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sadrul, 2016-07-30 05:16:23]

On 2016/07/28 20:26:31, Ben Goodger (Google) wrote:
> statics etcs are the devil. The client lib should be stateless. if you need
> to hold some state somewhere, inject it into the client lib.

OK. I am trying to get rid of the GpuService singleton
(https://codereview.chromium.org/2197613003/
https://codereview.chromium.org/2194003002/). That should allow moving the
client-side code out of the common component so it doesn't get included in the
service (https://codereview.chromium.org/2184943002/).

Removing the views dependency from content may be trickier though
(https://codereview.chromium.org/2188813003/), but I will look into that too.

> 
> -Ben
> 
> On Thu, Jul 28, 2016 at 1:23 PM, <mailto:ben@chromium.org> wrote:
> 
> > On 2016/07/28 19:44:09, sadrul wrote:
> > > On 2016/07/28 19:19:46, Ben Goodger (Google) wrote:
> > > > Why do you need to do this?
> > > >
> > > > The component build is a vestigial artifact of the chrome build
> > process,
> > > > something I hope we'll be able to undo at some future point with
> > > //service-based
> > > > componentization, & so for the past 2+ years we've avoided leaking it
> > into
> > > > //services code.
> > > >
> > > > Client libs tend to be pretty tiny, even complex ones like this - I
> > figured
> > as
> > > > long as the targets that depended on it were components it'd be ok.
> > >
> > > Without this, things like static variables or singletons defined in the
> > > client-lib cannot be used (see for example
> > > https://codereview.chromium.org/2188813003/, which uses a static const
> > > kWindowForAcceleratedWidget, and
> > https://codereview.chromium.org/2184943002/,
> > > which moves a client-side singleton into the client-lib).
> >
> > Ah right. This happened before and Peng tried to do this, and I think my
> > response was "don't do that" :-)
> >
> > https://codereview.chromium.org/2189893002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.