Hide public ctor of ARC related KeyedService. |
||
Issue descriptionThis is just a minor clean up tracking issue. We're currently have ctor of ARC related KeyedService classes in public: section. Probably, nice to hide it, to ensure those won't be created unexpectedly.
,
Jan 7
|
||
►
Sign in to add a comment |
||
Comment 1 by hidehiko@chromium.org
, Oct 18 2017Looking into this a bit more details. Luis, there are several minor choices here, so I'd like to make sure the direction before moving forward. 1) Forward declaration. Following the Chromium style guide, we often prefer forward declaration than include. Though, it introduces a bit more boilerplate. namespace arc { namespace internal { template <typename Service, typename Factory> class ArcBrowserContextKeyedServiceFactoryBase; } // namespace internal ... would we like to have this declaration, or #include? 2) Factory declaration. Most of Factory declaration and definition is in anonymous namespace in .cc files. Thus, in such cases, we cannot declare friend with a concrete class as follows. class ArcFooService : ... { ... private: friend internal::ArcBrowserContextkeyedServiceFactoryBase<ArcFooService, ... /* what should be here? */>; }; There are two options. 2-1) Declare Factory in a header. Maybe private nested class of ArcFooService. 2-2) Relax the restriction slightly and declare friend for whole template class. template <typename Service, typename Factory> class internal::ArcBrowserContextKeyedServiceFactoryBase; Another approach is (for both 1) and 2) problem), introduce a method into each factory, and declare the factory as a nested class and friend. namespace arc { class ArcFooService { ... private: class Factory; friend class Factory; ... }; } // namespace arc. And, in .cc namespace arc { class ArcFooService::Factory: public ArcBrowserContextKeyedServiceFactoryBase<ArcFooService, ArcFooService::Factory> { ... KeyedService* BuildArcServiceInstanceFor( content::BrowserContext* context, ArcServiceManager* arc_service_manager) { return new ArcFooService(context, arc_service_manager); } }; ... } // namespace arc WDYT? or, any other suggestion is welcome.