Create MetricsPrivateDelegate for metricsPrivate behavior

chrome/browser/extensions/api/metrics_private/metrics_private_api.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_EXTENSIONS_API_METRICS_PRIVATE_METRICS_PRIVATE_API_H_
 #define CHROME_BROWSER_EXTENSIONS_API_METRICS_PRIVATE_METRICS_PRIVATE_API_H_
 
 #include <stddef.h>
 
 #include <string>
 
 #include "base/metrics/histogram.h"
+#include "extensions/browser/browser_context_keyed_api_factory.h"
 #include "extensions/browser/extension_function.h"
 
+namespace content {
+class BrowserContext;
+}  // namespace content
+
 namespace extensions {
 
+class MetricsPrivateDelegate;
+
 class MetricsPrivateGetIsCrashReportingEnabledFunction
     : public UIThreadExtensionFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("metricsPrivate.getIsCrashReportingEnabled",
                              METRICSPRIVATE_GETISCRASHRECORDINGENABLED)
 
  protected:
   ~MetricsPrivateGetIsCrashReportingEnabledFunction() override {}
 
   // ExtensionFunction:
@@ skipping 158 matching lines @@
   DECLARE_EXTENSION_FUNCTION("metricsPrivate.recordLongTime",
                              METRICSPRIVATE_RECORDLONGTIME)
 
  protected:
   ~MetricsPrivateRecordLongTimeFunction() override {}
 
   // ExtensionFunction:
   ResponseAction Run() override;
 };
 
+class MetricsPrivateAPI : public BrowserContextKeyedAPI {

[Rahul Chaturvedi, 2016/09/16 23:11:04]

Is this needed? Our delegate isn't context specific but global.
Could we instead have [Chrome|Shell]ExtensionsApiClient keep the pointer to the
delegate instead and return that to us when we ask for it?

[michaelpg, 2016/09/16 23:51:56]

Good point. We could make the ExtensionAPIClient impl keep the delegate as a
static or member variable... or just give ownership to the caller.

I wonder if this would make more sense as a static function, with different
implementation files for different platforms. Is there a reason to prefer the
ExtensionApiClient interface over just creating a one-off function?

[Rahul Chaturvedi, 2016/09/19 18:11:09]

ExtensionsAPIClient::Get() returns the the correct subclass of
ExtensionAPIClient for our build ([Chrome|Shell]ExtensionsApiClient). If we use
a static function, how would we automatically get the correct version of our
function?

[michaelpg, 2016/09/19 18:36:32]

We do this a lot for platform-specific functionality: foo.h is implemented in
different files (foo_impl_win.cc, foo_impl_chromeos.cc). The implementation is
conditionally included at compile-time, as opposed to using sub-classing and
virtual methods.

But the examples I've seen are mostly *platform* specific (memory/file
management, set as default browser, etc.). MetricsPrivateDelegate's
functionality is *embedder* specific, which I guess we don't use that pattern
for (idk why). Since ExtensionsAPIClient already exists, I can just continue
using that. It just seems weird to own a pointer to what is essentially static
functionality.

[Rahul Chaturvedi, 2016/09/19 19:13:17]

So in 'this' particular situation, a few reasons to pick this pattern over
conditional compilation are,
(a) xxx_platform.cc files are automatically only compiled for their platform, we
don't have any such functionality for app_shell. We'd have to add separate build
rules for them, which I would rather avoid. In principle, I prefer to avoid
adding build rules since it can very quickly slippery slope into massive build
files with enough conditions and sub-conditions to make one's head hurt.

(b) Adding this pattern would be more consistent with the way we do delegate
functionality in other places. :)

I realize that neither of these are super-compelling reasons, but I don't see
many compelling reasons to go the other way either.

+ public:
+  explicit MetricsPrivateAPI(content::BrowserContext* context);
+  ~MetricsPrivateAPI() override;
+
+  // BrowserContextKeyedAPI:
+  static BrowserContextKeyedAPIFactory<MetricsPrivateAPI>* GetFactoryInstance();
+
+  MetricsPrivateDelegate* delegate() { return delegate_.get(); }
+
+ private:
+  friend class BrowserContextKeyedAPIFactory<MetricsPrivateAPI>;
+
+  // BrowserContextKeyedAPI:
+  static const char* service_name() { return "MetricsPrivateAPI"; }
+
+  std::unique_ptr<MetricsPrivateDelegate> delegate_;
+};
+
 } // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_API_METRICS_PRIVATE_METRICS_PRIVATE_API_H_