New issue
Advanced search Search tips

Issue 750580 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Task



Sign in to add a comment

Hide public ctor of ARC related KeyedService.

Project Member Reported by hidehiko@chromium.org, Jul 31 2017

Issue description

This 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.


 
Labels: -M-63 M-64
Looking 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.
Cc: -lhchavez@chromium.org

Sign in to add a comment