Stop using ResourceController in ResourceThrottle

Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

On the other hand, the AsyncRevalidationDriver and
ThrottlingResourceHandler are diverting the ResourceController
interface for defining their own interface to allow the throttles they
have to resume handlers deferred due to throttling. Not for
communication with the next handler.

Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by:
- adding methods on ResourceThrottle that hides the details of
  communication with the associated handler,
- introduce a dedicated interface ResourceThrottle::Delegate between
  the ResourceThrottle and the handlers than diverting the
  ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

R=reillyg@chromium.org,boliu@chromium.org,jochen@chromium.org
BUG=646277
Committed: https://crrev.com/64fa092bc4ccdcb1c0cc08b93d6c1bd49363adb0
Cr-Commit-Position: refs/heads/master@{#436889}

[tyoshino (SeeGerritForStatus), 2016-11-30 06:26:38]

The CQ bit was checked by tyoshino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 06:27:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 06:42:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-30 06:42:46]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[tyoshino (SeeGerritForStatus), 2016-11-30 06:45:38]

The CQ bit was checked by tyoshino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 06:46:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 07:04:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-30 07:04:38]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tyoshino (SeeGerritForStatus), 2016-11-30 10:03:58]

The CQ bit was checked by tyoshino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 10:04:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 10:23:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-30 10:23:11]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[tyoshino (SeeGerritForStatus), 2016-11-30 10:53:34]

The CQ bit was checked by tyoshino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 10:53:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 11:06:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-30 11:06:35]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tyoshino (SeeGerritForStatus), 2016-11-30 11:40:38]

The CQ bit was checked by tyoshino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 11:40:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 12:01:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-30 12:01:56]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[tyoshino (SeeGerritForStatus), 2016-11-30 13:34:33]

Patchset #1 (id:1) has been deleted

[tyoshino (SeeGerritForStatus), 2016-11-30 13:34:42]

Patchset #5 (id:100001) has been deleted

[tyoshino (SeeGerritForStatus), 2016-11-30 13:34:50]

Patchset #1 (id:20001) has been deleted

[tyoshino (SeeGerritForStatus), 2016-11-30 13:34:59]

Patchset #1 (id:40001) has been deleted

[tyoshino (SeeGerritForStatus), 2016-11-30 13:35:07]

Patchset #1 (id:60001) has been deleted

[tyoshino (SeeGerritForStatus), 2016-11-30 13:35:14]

Patchset #1 (id:80001) has been deleted

[tyoshino (SeeGerritForStatus), 2016-11-30 13:35:20]

The CQ bit was checked by tyoshino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 13:35:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tyoshino (SeeGerritForStatus), 2016-11-30 13:43:33]

Description was changed from

==========
Limit use of ResourceController to only inside ResourceHandler implementations

ResourceController is directly touched especially by ResourceThrottle
implementations. It's making it hard to understand the semantics of
ResourceController. Let ResourceHandler have methods which access
the ResourceController and hide the ResourceController from the
ResourceHandler implementations.

BUG=none
==========

to

==========
Limit use of ResourceController to only inside ResourceHandler implementations

ResourceController is directly touched especially by ResourceThrottle
implementations. It's making it hard to understand the semantics of
ResourceController. Let ResourceHandler have methods which access
the ResourceController and hide the ResourceController from the
ResourceHandler implementations.

Renamed ThrottlineResourceHandler::OnRequestDeferred() to
OnRequestDeferredForThrottle() to make it clear that this deferring

BUG=none
==========

[tyoshino (SeeGerritForStatus), 2016-11-30 14:40:53]

Description was changed from

==========
Limit use of ResourceController to only inside ResourceHandler implementations

ResourceController is directly touched especially by ResourceThrottle
implementations. It's making it hard to understand the semantics of
ResourceController. Let ResourceHandler have methods which access
the ResourceController and hide the ResourceController from the
ResourceHandler implementations.

Renamed ThrottlineResourceHandler::OnRequestDeferred() to
OnRequestDeferredForThrottle() to make it clear that this deferring

BUG=none
==========

to

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers and let the left most handler
to propagate cancel/resume signal to the right most handler (it's
ResourceLoader) possibly intercepted by the 4 classes.

However, AsyncRevalidationDriver and ThrottlingResourceHandler are just
diverting the ResourceController interface for defining their own
interface to allow the throttles they have to resume deferred handlers.
Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by adding methods on ResourceThrottle
that hides the details of communication with the associated handler,
and introduce a dedicated interface ResourceThrottle::Delegate between
the ResourceThrottle and the handlers than diverting the
ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

BUG=none
==========

[tyoshino (SeeGerritForStatus), 2016-11-30 14:41:26]

Description was changed from

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers and let the left most handler
to propagate cancel/resume signal to the right most handler (it's
ResourceLoader) possibly intercepted by the 4 classes.

However, AsyncRevalidationDriver and ThrottlingResourceHandler are just
diverting the ResourceController interface for defining their own
interface to allow the throttles they have to resume deferred handlers.
Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by adding methods on ResourceThrottle
that hides the details of communication with the associated handler,
and introduce a dedicated interface ResourceThrottle::Delegate between
the ResourceThrottle and the handlers than diverting the
ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

BUG=none
==========

to

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers and let the left most handler
to propagate cancel/resume signal to the right most handler (it's
ResourceLoader) possibly intercepted by the 4 classes.

However, AsyncRevalidationDriver and ThrottlingResourceHandler are just
diverting the ResourceController interface for defining their own
interface to allow the throttles they have to resume deferred handlers.
Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by adding methods on ResourceThrottle
that hides the details of communication with the associated handler,
and introduce a dedicated interface ResourceThrottle::Delegate between
the ResourceThrottle and the handlers than diverting the
ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

BUG=none
==========

[tyoshino (SeeGerritForStatus), 2016-11-30 14:50:09]

Description was changed from

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers and let the left most handler
to propagate cancel/resume signal to the right most handler (it's
ResourceLoader) possibly intercepted by the 4 classes.

However, AsyncRevalidationDriver and ThrottlingResourceHandler are just
diverting the ResourceController interface for defining their own
interface to allow the throttles they have to resume deferred handlers.
Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by adding methods on ResourceThrottle
that hides the details of communication with the associated handler,
and introduce a dedicated interface ResourceThrottle::Delegate between
the ResourceThrottle and the handlers than diverting the
ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

BUG=none
==========

to

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

However, AsyncRevalidationDriver and ThrottlingResourceHandler are just
diverting the ResourceController interface for defining their own
interface to allow the throttles they have to resume deferred handlers.
Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by adding methods on ResourceThrottle
that hides the details of communication with the associated handler,
and introduce a dedicated interface ResourceThrottle::Delegate between
the ResourceThrottle and the handlers than diverting the
ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

BUG=none
==========

[tyoshino (SeeGerritForStatus), 2016-11-30 14:50:36]

Description was changed from

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

However, AsyncRevalidationDriver and ThrottlingResourceHandler are just
diverting the ResourceController interface for defining their own
interface to allow the throttles they have to resume deferred handlers.
Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by adding methods on ResourceThrottle
that hides the details of communication with the associated handler,
and introduce a dedicated interface ResourceThrottle::Delegate between
the ResourceThrottle and the handlers than diverting the
ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

BUG=none
==========

to

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

However, AsyncRevalidationDriver and ThrottlingResourceHandler are
diverting the ResourceController interface for defining their own
interface to allow the throttles they have to resume deferred handlers.
Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by adding methods on ResourceThrottle
that hides the details of communication with the associated handler,
and introduce a dedicated interface ResourceThrottle::Delegate between
the ResourceThrottle and the handlers than diverting the
ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

BUG=none
==========

[tyoshino (SeeGerritForStatus), 2016-11-30 14:59:03]

Description was changed from

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

However, AsyncRevalidationDriver and ThrottlingResourceHandler are
diverting the ResourceController interface for defining their own
interface to allow the throttles they have to resume deferred handlers.
Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by adding methods on ResourceThrottle
that hides the details of communication with the associated handler,
and introduce a dedicated interface ResourceThrottle::Delegate between
the ResourceThrottle and the handlers than diverting the
ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

BUG=none
==========

to

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

On the other hand, the AsyncRevalidationDriver and
ThrottlingResourceHandler are diverting the ResourceController
interface for defining their own interface to allow the throttles they
have to resume handlers deferred due to throttling. Not for
communication with the next handler.

Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by:
- adding methods on ResourceThrottle that hides the details of
  communication with the associated handler,
- introduce a dedicated interface ResourceThrottle::Delegate between
  the ResourceThrottle and the handlers than diverting the
  ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

BUG=none
==========

[tyoshino (SeeGerritForStatus), 2016-11-30 14:59:19]

The CQ bit was checked by tyoshino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 14:59:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tyoshino (SeeGerritForStatus), 2016-11-30 15:05:26]

Description was changed from

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

On the other hand, the AsyncRevalidationDriver and
ThrottlingResourceHandler are diverting the ResourceController
interface for defining their own interface to allow the throttles they
have to resume handlers deferred due to throttling. Not for
communication with the next handler.

Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by:
- adding methods on ResourceThrottle that hides the details of
  communication with the associated handler,
- introduce a dedicated interface ResourceThrottle::Delegate between
  the ResourceThrottle and the handlers than diverting the
  ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

BUG=none
==========

to

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

On the other hand, the AsyncRevalidationDriver and
ThrottlingResourceHandler are diverting the ResourceController
interface for defining their own interface to allow the throttles they
have to resume handlers deferred due to throttling. Not for
communication with the next handler.

Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by:
- adding methods on ResourceThrottle that hides the details of
  communication with the associated handler,
- introduce a dedicated interface ResourceThrottle::Delegate between
  the ResourceThrottle and the handlers than diverting the
  ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

BUG=none
==========

[tyoshino (SeeGerritForStatus), 2016-11-30 15:05:27]

tyoshino@chromium.org changed reviewers:
+ jochen@chromium.org

[mmenke, 2016-11-30 15:15:35]

Note that I have a pretty significant refactor of how ResourceControllers are
passed through the ResourceHandler chain in the works.  Doesn't conflict with
this CL significantly (In fact, only having ResourceHandlers use
ResourceControllers makes the change in behavior cleaner), but worth being aware
of:  I'm changing it so that instead of a bool* defer argument eveywhere, we
pass around unique_ptr<ResourceController>, so only one class has a given
ResourceController at a time.

This prevents things like out-of-band Resumes, which is nice, but the big thing
I want to get rid of is out-of-order cancels.  It can result in two
ResourceHandlers thinking they're the unique object responsible for continuing
the request at once, since we can defer teardown, which can result in weird,
unexpected states.  Out-of-band cancels will follow another path that doesn't
allow deferral.

[commit-bot: I haz the power, 2016-11-30 16:39:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-30 16:39:33]

Dry run: This issue passed the CQ dry run.

[tyoshino (SeeGerritForStatus), 2016-12-01 12:17:38]

On 2016/11/30 15:15:35, mmenke wrote:
> Note that I have a pretty significant refactor of how ResourceControllers are
> passed through the ResourceHandler chain in the works.  Doesn't conflict with
> this CL significantly (In fact, only having ResourceHandlers use
> ResourceControllers makes the change in behavior cleaner), but worth being
aware
> of:  I'm changing it so that instead of a bool* defer argument eveywhere, we
> pass around unique_ptr<ResourceController>, so only one class has a given
> ResourceController at a time.

Thanks for the heads up!

> 
> This prevents things like out-of-band Resumes, which is nice, but the big
thing
> I want to get rid of is out-of-order cancels.  It can result in two
> ResourceHandlers thinking they're the unique object responsible for continuing
> the request at once, since we can defer teardown, which can result in weird,
> unexpected states.  Out-of-band cancels will follow another path that doesn't
> allow deferral.

OK

[tyoshino (SeeGerritForStatus), 2016-12-05 10:58:31]

The CQ bit was checked by tyoshino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-05 10:58:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-05 11:43:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-05 11:43:58]

Dry run: This issue passed the CQ dry run.

[jochen (gone - plz use gerrit), 2016-12-05 15:25:03]

lgtm with comments added

https://codereview.chromium.org/2535723005/diff/160001/content/public/browser...
File content/public/browser/resource_throttle.h (right):

https://codereview.chromium.org/2535723005/diff/160001/content/public/browser...
content/public/browser/resource_throttle.h:29: virtual void Cancel() = 0;
please add comments explaining the delegate and every virtual method

https://codereview.chromium.org/2535723005/diff/160001/content/public/browser...
content/public/browser/resource_throttle.h:70: void Cancel();
please add comments here as well

[jochen (gone - plz use gerrit), 2016-12-05 15:25:13]

please also add a tracking bug

[tyoshino (SeeGerritForStatus), 2016-12-05 16:05:14]

tyoshino@chromium.org changed reviewers:
+ boliu@chromium.org

[tyoshino (SeeGerritForStatus), 2016-12-05 16:07:00]

tyoshino@chromium.org changed reviewers:
+ reillyg@chromium.org

[tyoshino (SeeGerritForStatus), 2016-12-05 16:07:08]

On 2016/12/05 15:25:13, jochen wrote:
> please also add a tracking bug

I'll do. Thank you for review.

+boliu, reillyg

boliu, reillyg, please take a look at the changes in android_webview/ dir. I'll
address jochen@'s comments in #48 tomorrow.

[tyoshino (SeeGerritForStatus), 2016-12-05 16:07:33]

On 2016/12/05 16:07:08, tyoshino wrote:
> On 2016/12/05 15:25:13, jochen wrote:
> > please also add a tracking bug
> 
> I'll do. Thank you for review.
> 
> +boliu, reillyg
> 
> boliu, reillyg, please take a look at the changes in android_webview/ dir.
I'll
> address jochen@'s comments in #48 tomorrow.

Correction: reillyg, please take a look at the changes in extensions/. Thanks!

[boliu, 2016-12-05 17:01:31]

rs lgtm

[tyoshino (SeeGerritForStatus), 2016-12-06 10:59:57]

Description was changed from

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

On the other hand, the AsyncRevalidationDriver and
ThrottlingResourceHandler are diverting the ResourceController
interface for defining their own interface to allow the throttles they
have to resume handlers deferred due to throttling. Not for
communication with the next handler.

Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by:
- adding methods on ResourceThrottle that hides the details of
  communication with the associated handler,
- introduce a dedicated interface ResourceThrottle::Delegate between
  the ResourceThrottle and the handlers than diverting the
  ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

BUG=none
==========

to

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

On the other hand, the AsyncRevalidationDriver and
ThrottlingResourceHandler are diverting the ResourceController
interface for defining their own interface to allow the throttles they
have to resume handlers deferred due to throttling. Not for
communication with the next handler.

Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by:
- adding methods on ResourceThrottle that hides the details of
  communication with the associated handler,
- introduce a dedicated interface ResourceThrottle::Delegate between
  the ResourceThrottle and the handlers than diverting the
  ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

BUG=646277
==========

[tyoshino (SeeGerritForStatus), 2016-12-06 11:01:22]

On 2016/12/05 16:07:08, tyoshino wrote:
> On 2016/12/05 15:25:13, jochen wrote:
> > please also add a tracking bug
> 
> I'll do. Thank you for review.

Just put http://crbug.com/646277 to BUG line which is about the original
security issue.

[tyoshino (SeeGerritForStatus), 2016-12-06 11:25:31]

https://codereview.chromium.org/2535723005/diff/160001/content/public/browser...
File content/public/browser/resource_throttle.h (right):

https://codereview.chromium.org/2535723005/diff/160001/content/public/browser...
content/public/browser/resource_throttle.h:29: virtual void Cancel() = 0;
On 2016/12/05 15:25:03, jochen wrote:
> please add comments explaining the delegate and every virtual method

Done.

https://codereview.chromium.org/2535723005/diff/160001/content/public/browser...
content/public/browser/resource_throttle.h:70: void Cancel();
On 2016/12/05 15:25:03, jochen wrote:
> please add comments here as well

Done.

[tyoshino (SeeGerritForStatus), 2016-12-06 11:26:37]

The CQ bit was checked by tyoshino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-06 11:26:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-06 13:00:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-06 13:00:37]

Dry run: This issue passed the CQ dry run.

[Reilly Grant (use Gerrit), 2016-12-06 18:53:44]

*/extensions/* lgtm with nits

https://codereview.chromium.org/2535723005/diff/200001/chrome/browser/extensi...
File chrome/browser/extensions/user_script_listener_unittest.cc (right):

https://codereview.chromium.org/2535723005/diff/200001/chrome/browser/extensi...
chrome/browser/extensions/user_script_listener_unittest.cc:166:
request->SetUserData(NULL, new ThrottleDelegate(request.get(), throttle));
nit: nullptr

https://codereview.chromium.org/2535723005/diff/200001/chrome/browser/extensi...
chrome/browser/extensions/user_script_listener_unittest.cc:333:
request->SetUserData(NULL, new ThrottleDelegate(request.get(), throttle));
nit: nullptr

[mmenke, 2016-12-06 20:16:41]

mmenke@chromium.org changed reviewers:
+ mmenke@chromium.org

[mmenke, 2016-12-06 20:16:42]

https://codereview.chromium.org/2535723005/diff/200001/content/public/browser...
File content/public/browser/resource_throttle.h (right):

https://codereview.chromium.org/2535723005/diff/200001/content/public/browser...
content/public/browser/resource_throttle.h:91: Delegate* delegate_;
With the ResourceController dependency removed, you should remove
resource_controller from content/public, and move it into
content/browser/loader, since ResourceHandler isn't in content/public.

[mmenke, 2016-12-06 20:17:20]

mmenke@chromium.org changed reviewers:
- mmenke@chromium.org

[tyoshino (SeeGerritForStatus), 2016-12-07 05:51:28]

Description was changed from

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

On the other hand, the AsyncRevalidationDriver and
ThrottlingResourceHandler are diverting the ResourceController
interface for defining their own interface to allow the throttles they
have to resume handlers deferred due to throttling. Not for
communication with the next handler.

Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by:
- adding methods on ResourceThrottle that hides the details of
  communication with the associated handler,
- introduce a dedicated interface ResourceThrottle::Delegate between
  the ResourceThrottle and the handlers than diverting the
  ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

BUG=646277
==========

to

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

On the other hand, the AsyncRevalidationDriver and
ThrottlingResourceHandler are diverting the ResourceController
interface for defining their own interface to allow the throttles they
have to resume handlers deferred due to throttling. Not for
communication with the next handler.

Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by:
- adding methods on ResourceThrottle that hides the details of
  communication with the associated handler,
- introduce a dedicated interface ResourceThrottle::Delegate between
  the ResourceThrottle and the handlers than diverting the
  ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

R=reillyg@chromium.org,boliu@chromium.org,jochen@chromium.org
BUG=646277
==========

[tyoshino (SeeGerritForStatus), 2016-12-07 05:51:37]

The CQ bit was checked by tyoshino@chromium.org

[tyoshino (SeeGerritForStatus), 2016-12-07 05:51:38]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
boliu@chromium.org, reillyg@chromium.org
Link to the patchset: https://codereview.chromium.org/2535723005/#ps220001
(title: "Addressed #62")

[commit-bot: I haz the power, 2016-12-07 05:51:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tyoshino (SeeGerritForStatus), 2016-12-07 05:52:16]

https://codereview.chromium.org/2535723005/diff/200001/chrome/browser/extensi...
File chrome/browser/extensions/user_script_listener_unittest.cc (right):

https://codereview.chromium.org/2535723005/diff/200001/chrome/browser/extensi...
chrome/browser/extensions/user_script_listener_unittest.cc:166:
request->SetUserData(NULL, new ThrottleDelegate(request.get(), throttle));
On 2016/12/06 18:53:44, Reilly Grant wrote:
> nit: nullptr

Done.

https://codereview.chromium.org/2535723005/diff/200001/chrome/browser/extensi...
chrome/browser/extensions/user_script_listener_unittest.cc:333:
request->SetUserData(NULL, new ThrottleDelegate(request.get(), throttle));
On 2016/12/06 18:53:44, Reilly Grant wrote:
> nit: nullptr

Done.

https://codereview.chromium.org/2535723005/diff/200001/content/public/browser...
File content/public/browser/resource_throttle.h (right):

https://codereview.chromium.org/2535723005/diff/200001/content/public/browser...
content/public/browser/resource_throttle.h:91: Delegate* delegate_;
On 2016/12/06 20:16:42, mmenke wrote:
> With the ResourceController dependency removed, you should remove
> resource_controller from content/public, and move it into
> content/browser/loader, since ResourceHandler isn't in content/public.

OK. Let me do it in the next CL.

[commit-bot: I haz the power, 2016-12-07 07:40:14]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1481089897584040,
"parent_rev": "009142190cfd6c3ea118a5e056707f430fa9fa43", "commit_rev":
"fd1e4808f53698fc72396ee70c66adec088941c7"}

[commit-bot: I haz the power, 2016-12-07 07:40:42]

Description was changed from

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

On the other hand, the AsyncRevalidationDriver and
ThrottlingResourceHandler are diverting the ResourceController
interface for defining their own interface to allow the throttles they
have to resume handlers deferred due to throttling. Not for
communication with the next handler.

Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by:
- adding methods on ResourceThrottle that hides the details of
  communication with the associated handler,
- introduce a dedicated interface ResourceThrottle::Delegate between
  the ResourceThrottle and the handlers than diverting the
  ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

R=reillyg@chromium.org,boliu@chromium.org,jochen@chromium.org
BUG=646277
==========

to

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

On the other hand, the AsyncRevalidationDriver and
ThrottlingResourceHandler are diverting the ResourceController
interface for defining their own interface to allow the throttles they
have to resume handlers deferred due to throttling. Not for
communication with the next handler.

Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by:
- adding methods on ResourceThrottle that hides the details of
  communication with the associated handler,
- introduce a dedicated interface ResourceThrottle::Delegate between
  the ResourceThrottle and the handlers than diverting the
  ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

R=reillyg@chromium.org,boliu@chromium.org,jochen@chromium.org
BUG=646277
==========

[commit-bot: I haz the power, 2016-12-07 07:40:43]

Committed patchset #6 (id:220001)

[commit-bot: I haz the power, 2016-12-07 07:42:39]

Description was changed from

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

On the other hand, the AsyncRevalidationDriver and
ThrottlingResourceHandler are diverting the ResourceController
interface for defining their own interface to allow the throttles they
have to resume handlers deferred due to throttling. Not for
communication with the next handler.

Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by:
- adding methods on ResourceThrottle that hides the details of
  communication with the associated handler,
- introduce a dedicated interface ResourceThrottle::Delegate between
  the ResourceThrottle and the handlers than diverting the
  ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

R=reillyg@chromium.org,boliu@chromium.org,jochen@chromium.org
BUG=646277
==========

to

==========
Stop using ResourceController in ResourceThrottle

The following classes are subclasses of the ResourceController
interface:
- AsyncRevalidationDriver
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader
- ThrottlingResourceHandler

The following classes out of them sets themselves as the controller for
the next handler:
- DetachableResourceHandler
- InterceptingResourceHandler
- MimeSniffingResourceHandler
- ResourceLoader

Classes implementing LayeredResourceHandler pass the controller given
to SetController() call through to the next handler.

With this mechanism, we can chain handlers, let each handler
communicate defer signal via the ResourceHandler interface methods,
and let the left most handler to propagate cancel/resume signal to the
right most handler (it's ResourceLoader. possibly intercepted by the
4 classes). This is easy to understand.

On the other hand, the AsyncRevalidationDriver and
ThrottlingResourceHandler are diverting the ResourceController
interface for defining their own interface to allow the throttles they
have to resume handlers deferred due to throttling. Not for
communication with the next handler.

Moreover, the ResourceThrottle is exposing the ResourceController
instance to its subclasses and letting them directly operate on the
instance. This is making the ResourceController interface and handler
chain hard to understand e.g. by making the cross reference on
codesearch list a lot of callers.

I'd like to make this cleaner by:
- adding methods on ResourceThrottle that hides the details of
  communication with the associated handler,
- introduce a dedicated interface ResourceThrottle::Delegate between
  the ResourceThrottle and the handlers than diverting the
  ResourceController interface.

Motivation:
- I plan to change AsyncResourceHandler::OnFollowRedirect() to take
  more arguments to reflect changes in parameters made by Blink (See
  crbug.com/646277 and crbug.com/665766 for the details why we need to
  do this).
- I want to construct a path to propagate it to the URLRequest, but
  ResourceController::Resume() is used for various kinds of resuming.
  I want to separate redirect handling logic in Resume() into a separate
  method on ResourceController than adding arguments to Resume().
- Found that ResourceController is used for various purposes. Before
  fixing the issue, I wanted to clean it up for readability.

R=reillyg@chromium.org,boliu@chromium.org,jochen@chromium.org
BUG=646277

Committed: https://crrev.com/64fa092bc4ccdcb1c0cc08b93d6c1bd49363adb0
Cr-Commit-Position: refs/heads/master@{#436889}
==========

[commit-bot: I haz the power, 2016-12-07 07:42:41]

Patchset 6 (id:??) landed as
https://crrev.com/64fa092bc4ccdcb1c0cc08b93d6c1bd49363adb0
Cr-Commit-Position: refs/heads/master@{#436889}

[tyoshino (SeeGerritForStatus), 2016-12-07 08:34:24]

https://codereview.chromium.org/2535723005/diff/200001/content/public/browser...
File content/public/browser/resource_throttle.h (right):

https://codereview.chromium.org/2535723005/diff/200001/content/public/browser...
content/public/browser/resource_throttle.h:91: Delegate* delegate_;
On 2016/12/07 05:52:15, tyoshino wrote:
> On 2016/12/06 20:16:42, mmenke wrote:
> > With the ResourceController dependency removed, you should remove
> > resource_controller from content/public, and move it into
> > content/browser/loader, since ResourceHandler isn't in content/public.
> 
> OK. Let me do it in the next CL.

Created https://codereview.chromium.org/2554273002/