Introduce the ThrottleManager

components/subresource_filter/content/browser/content_subresource_filter_throttle_manager.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 COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_CONTENT_SUBRESOURCE_FILTER_THROTTLE_MANAGER_H_
+#define COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_CONTENT_SUBRESOURCE_FILTER_THROTTLE_MANAGER_H_
+
+#include <memory>
+#include <unordered_map>
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "components/subresource_filter/content/browser/verified_ruleset_dealer.h"
+#include "components/subresource_filter/core/common/activation_state.h"
+#include "content/public/browser/web_contents_observer.h"
+
+namespace content {
+class NavigationHandle;
+class NavigationThrottle;
+class RenderFrameHost;
+}  // namespace content
+
+namespace subresource_filter {
+
+class AsyncDocumentSubresourceFilter;
+class ActivationStateComputingNavigationThrottle;
+
+// Simple struct which contains useful information regarding a frame's
+// activation. This includes its DocumentSubresourceFilter, as well as its
+// ActivationState.
+struct FrameActivationInfo {

[engedy, 2017/02/20 15:58:13]

nit: Can the definition of this struct be moved to the .cc file? Looks like a
forward declaration might be enough here. Would also be nice to make this nested
inside the manager so that we do not pollute the namespace.

nit: Could you please add a TODO to consider storing the ActivationState on the
ADSF instead, and removing this struct?

[Charlie Harrison, 2017/03/01 00:02:58]

Let me just move the ActivationState into ADSF in this patch, it simplifies
things.

+  FrameActivationInfo();
+  ~FrameActivationInfo();
+  std::unique_ptr<AsyncDocumentSubresourceFilter> subresource_filter;
+  ActivationState activation_state;
+};
+
+class ContentSubresourceFilterThrottleManager

[engedy, 2017/02/20 15:58:13]

nit: Let's describe what this is in a comment!

[Charlie Harrison, 2017/03/01 00:02:59]

Done.

+    : public content::WebContentsObserver {
+ public:
+  // It is expected that the Delegate outlives |this|, and manages the lifetime
+  // of this class.
+  class Delegate {
+   public:
+    // The embedder may be interested in displaying UI to the user when the
+    // first load is disallowed for a given page load.
+    virtual void OnFirstSubresourceLoadDisallowed() {}
+
+    // Let the delegate have the last word when it comes to activation. It might
+    // have a specific whitelist.
+    virtual bool ShouldVetoActivation(

[engedy, 2017/02/20 15:58:13]

nit: s/Veto/Suppress/, my understanding is that a method name `ShouldX` is
interpreted as "Does the embedder want the content/ layer to do X", so the
subject is the content/ layer, not the embedder.

[Charlie Harrison, 2017/03/01 00:02:58]

Done.

+        content::NavigationHandle* navigation_handle);
+  };
+
+  ContentSubresourceFilterThrottleManager(
+      Delegate* delegate,
+      VerifiedRulesetDealer::Handle* dealer_handle,
+      content::WebContents* web_contents);
+  ~ContentSubresourceFilterThrottleManager() override;
+
+  // Must be called during a navigation, before the corresponding throttle in

[engedy, 2017/02/20 15:58:13]

Let's make this interface less implementation-oriented and more
embedder-friendly:

// Sets the desired page-level |activation_state| for the currently ongoing page
load, identified by its main-frame |navigation_handle|. To be called by the
embedder at the latest in the WillProcessResponse stage, at latest from a
NavigationThrottle that was registered before the throttles created by this
manager in MaybeAppendNavigationThrottles(). If this method is not called for a
main-frame navigation, the default behavior is no activation for that page load.
  virtual void ActivateForPageLoad(
      NavigationHandle* navigation_handle, ...);

[Charlie Harrison, 2017/03/01 00:02:58]

Done.

+  // |ongoing_activation_throttles_| reaches WillProcessResponse.
+  void NotifyPageActivationComputed(
+      content::NavigationHandle* activated_navigation,

[engedy, 2017/02/20 15:58:13]

nit: s/activated_navigation/navigation/, it's not necessarily going to be
activated.

[Charlie Harrison, 2017/03/01 00:02:56]

I s/activated_navigation/navigation_handle/

+      const ActivationState& activation_state);
+
+  // content::WebContentsObserver:

[engedy, 2017/02/20 15:58:13]

Could we make these protected?

[Charlie Harrison, 2017/03/01 00:02:58]

Done.

+  void RenderFrameDeleted(content::RenderFrameHost* frame_host) override;
+  void ReadyToCommitNavigation(
+      content::NavigationHandle* navigation_handle) override;
+  void DidFinishNavigation(
+      content::NavigationHandle* navigation_handle) override;
+
+  void MaybeAppendNavigationThrottles(

[engedy, 2017/02/20 15:58:13]

nit: Given this is the public API of this class, and of more or less the entire
component, let's add a comment to describe this (and move it above the observer
methods). Also mention the NavigationThrottle ordering constraint in the
comment.

nit: Do you think returning a vector (to be appended by the caller) would be
frowned upon? It's only one more allocation per navigation, which should also be
relatively fast thanks to tc_malloc?

[Charlie Harrison, 2017/03/01 00:02:58]

Done, I mentioned that in fact there are no ordering constraints.
I'm not sure, let's punt this for when we actually use the method in a follow up
and see what people think.

[engedy, 2017/03/10 17:36:20]

Sounds good.

+      content::NavigationHandle* navigation_handle,
+      std::vector<std::unique_ptr<content::NavigationThrottle>>* throttles);
+
+ private:
+  std::unique_ptr<content::NavigationThrottle>
+  MaybeCreateSubframeNavigationFilteringThrottle(
+      content::NavigationHandle* navigation_handle);
+  std::unique_ptr<content::NavigationThrottle>
+  MaybeCreateActivationStateComputingThrottle(
+      content::NavigationHandle* navigation_handle);
+
+  FrameActivationInfo* GetFrameActivationInfoForChildNavigation(
+      content::NavigationHandle* child_frame_navigation);
+
+  // Calls OnFirstSubresourceLoadDisallowed on the Delegate at most once per
+  // committed load.

[engedy, 2017/02/20 15:58:13]

nit: once per committed, non-same-page navigation in the main frame. 

[Charlie Harrison, 2017/03/01 00:02:58]

Done.

+  // TODO(csharrison): Ensure IPCs from the renderer go through this path when
+  // they disallow subresource loads.
+  void MaybeCallFirstDisallowedLoad();
+
+  VerifiedRuleset::Handle* EnsureRulesetHandle();
+  void DestroyRulesetHandleIfNecessary();

[engedy, 2017/02/20 15:58:13]

nit: s/Necessary/NoLongerUsed/

[Charlie Harrison, 2017/03/01 00:02:58]

Done.

+
+  // Map of all RenderFrameHosts that are currently activated, and their

[engedy, 2017/02/20 15:58:13]

Phrasing nit: For each RenderFrameHost where the last committed load has
subresource filtering activated, stores the ActivationState and owns the
corresponding AsyncDocumentSubresourceFilter.

[Charlie Harrison, 2017/03/01 00:02:58]

Done.

+  // associated ActivationState and AsyncDocumentSubresourceFilters.
+  std::unordered_map<content::RenderFrameHost*,
+                     std::unique_ptr<FrameActivationInfo>>
+      activated_frame_hosts_;
+
+  // For each ongoing navigation that requires activation state computation,
+  // keeps track of the throttle that is carrying out that computation, so that
+  // the result can be retrieved when the navigation is ready to commit.
+  std::unordered_map<content::NavigationHandle*,
+                     ActivationStateComputingNavigationThrottle*>
+      ongoing_activation_throttles_;
+
+  // Lazily instantiated in EnsureRulesetHandle when the first page level
+  // activation is triggered. Will go away when there are no more activated
+  // RenderFrameHosts (i.e. activated_frame_hosts_ is empty).
+  std::unique_ptr<VerifiedRuleset::Handle> ruleset_handle_;
+
+  // True if the current committed main frame load in this WebContents has
+  // notified the delegate that a subresource was disallowed. The callback
+  // should only be called at most once per main frame load.
+  bool current_committed_load_has_notified_disallowed_load_ = false;
+
+  // These members outlive this class.
+  VerifiedRulesetDealer::Handle* dealer_handle_;
+  Delegate* delegate_;
+
+  base::WeakPtrFactory<ContentSubresourceFilterThrottleManager>
+      weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(ContentSubresourceFilterThrottleManager);
+};
+
+}  // namespace subresource_filter
+
+#endif  // COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_CONTENT_SUBRESOURCE_FILTER_THROTTLE_MANAGER_H_