Initialize ios/web_view translate with a system-wide URLRequestContext.

ios/web_view/internal/translate/web_view_translate_service.h

+// Copyright 2017 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 IOS_WEB_VIEW_INTERNAL_TRANSLATE_WEB_VIEW_TRANSLATE_SERVICE_H_
+#define IOS_WEB_VIEW_INTERNAL_TRANSLATE_WEB_VIEW_TRANSLATE_SERVICE_H_
+
+#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "components/web_resource/resource_request_allowed_notifier.h"
+
+namespace base {
+template <typename T>
+struct DefaultSingletonTraits;
+}
+
+namespace ios_web_view {
+
+// Singleton managing the resources required for Translate.
+class WebViewTranslateService {
+ public:
+  static WebViewTranslateService* GetInstance();
+
+  // Must be called before the Translate feature can be used.
+  void Initialize();
+
+  // Must be called to shut down the Translate feature.
+  void Shutdown();
+
+ private:
+  // Manages enabling translate requests only when resource requests are
+  // allowed.
+  // NOTE: This must be a separate class from WebViewTranslateService becuase

[sdefresne, 2017/05/31 08:52:46]

becuase -> because

[michaeldo, 2017/05/31 21:03:43]

Done.

+  // base/memory/singleton.h won't see that the OnResourceRequestsAllowed

[sdefresne, 2017/05/31 08:52:46]

I don't understand this comment at all. Is this because you want the method
OnResourceRequestsAllowed to be invoked in the constructor/destructor? Then yes,
it is not possible to do that, but this is not a problem with
base/memory/singleton.h, just how C++ virtual methods are implemented.

Since we have many singleton classes that do implements virtual base class
(random example: all the KeyedService factories), I do not think you *need* a
separate class, unless you are trying to call the virtual method from the
destructor/constructor, which you do not appear to be doing (at least not in a
different way in TranslateRequestsAllowedListener than it would be done in
WebViewTranslateService AFAICT).

Maybe the problem is that web_resource::ResourceRequestAllowedNotifier::Observer
destructor is not virtual.

[michaeldo, 2017/05/31 21:03:43]

I updated the comment. IIUC then we need to fix the Observer class? A brief
search for "class Observer {" shows that many of them do not have virtual
destructors. Should they all? I looked into this a little and it seems this
could lead to undefined deconstructor behavior and potential leaks!?

[sdefresne, 2017/06/01 08:36:52]

If a class is written to be sub-classed and it is possible to delete an instance
through a pointer to base class (i.e. the destructor is public), then the base
class destructor needs to be virtual otherwise the result is undefined
behaviour. So Observer class destructor needs to be changed to be virtual or
protected.

+  // virtual method has been implemented otherwise.
+  class TranslateRequestsAllowedListener
+      : public web_resource::ResourceRequestAllowedNotifier::Observer {
+   public:
+    TranslateRequestsAllowedListener();
+    ~TranslateRequestsAllowedListener();

[sdefresne, 2017/05/31 08:52:46]

This destructor should be virtual in the base class :-/

[michaeldo, 2017/05/31 21:03:43]

Acknowledged.

+
+    // ResourceRequestAllowedNotifier::Observer methods.
+    void OnResourceRequestsAllowed() override;
+
+   private:
+    // Notifier class to know if it's allowed to make network resource requests.
+    web_resource::ResourceRequestAllowedNotifier
+        resource_request_allowed_notifier_;
+
+    DISALLOW_COPY_AND_ASSIGN(TranslateRequestsAllowedListener);
+  };
+
+  WebViewTranslateService();
+  ~WebViewTranslateService();
+
+  friend struct base::DefaultSingletonTraits<WebViewTranslateService>;
+
+  // Listener which manages when translate requests can occur.
+  std::unique_ptr<TranslateRequestsAllowedListener>

[sdefresne, 2017/05/31 08:52:46]

You can change this to
"std::unique_ptr<web_resource::ResourceRequestAllowedNotifier::Observer>
translate_requests_allowed_listener_;" and move the declaration of the class
TranslateRequestsAllowedListener to the implementation file. But I've just seen
that the destructor in the base class is not virtual ... That is bad.

Or you can also change this to "TranslateRequestsAllowedListener
translate_requests_allowed_listener_;" to avoid a separate allocation.

[michaeldo, 2017/05/31 21:03:43]

I used your second suggestion.

+      translate_requests_allowed_listener_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebViewTranslateService);
+};
+
+}  // namespace ios_web_view
+
+#endif  // IOS_WEB_VIEW_INTERNAL_TRANSLATE_WEB_VIEW_TRANSLATE_SERVICE_H_