Factor DRP PreviewsState logic out of content

Factor DRP PreviewsState logic out of content

Move the Data Reduction Proxy header related logic for updating Previews
state out of content and into a RenderFrameObserver in the Data Reduction
Proxy component.

BUG=737159
Review-Url: https://codereview.chromium.org/2963913002
Cr-Commit-Position: refs/heads/master@{#492458}
Committed: https://chromium.googlesource.com/chromium/src/+/9ee542da746d166a094a705ed3fcd7304dfe61e0

[megjablon, 2017-06-29 01:32:29]

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

[megjablon, 2017-06-29 01:56:45]

Patchset #1 (id:1) has been deleted

[megjablon, 2017-06-29 01:56:52]

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

[commit-bot: I haz the power, 2017-06-29 01:57:10]

Dry run: CQ is trying da patch.

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

[megjablon, 2017-06-29 02:10:54]

megjablon@chromium.org changed reviewers:
+ dougarnett@chromium.org

[megjablon, 2017-06-29 02:10:54]

PTAL, thanks!

[commit-bot: I haz the power, 2017-06-29 03:46:53]

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

[commit-bot: I haz the power, 2017-06-29 03:46:54]

Dry run: This issue passed the CQ dry run.

[dougarnett, 2017-06-30 21:03:36]

cool - would be nice to get moved code linked for revision history though.

https://codereview.chromium.org/2963913002/diff/20001/components/data_reducti...
File
components/data_reduction_proxy/content/renderer/content_previews_render_frame_observer.cc
(right):

https://codereview.chromium.org/2963913002/diff/20001/components/data_reducti...
components/data_reduction_proxy/content/renderer/content_previews_render_frame_observer.cc:23:
bool HasEmptyImageDirective(const blink::WebURLResponse& web_url_response) {
Bummer that git didn't identify the move of this code. Does git have it own
hinting mechanism (like git5 hint)?

https://codereview.chromium.org/2963913002/diff/20001/components/data_reducti...
components/data_reduction_proxy/content/renderer/content_previews_render_frame_observer.cc:79:
if (web_url_response
Consider adding TODO to leverage common code in drp_headers (eg, adapt
ParseResponseTransform if can work out how to deal with different http header
object types).

[megjablon, 2017-07-06 19:14:36]

Patchset #2 (id:40001) has been deleted

[megjablon, 2017-07-06 19:15:00]

https://codereview.chromium.org/2963913002/diff/20001/components/data_reducti...
File
components/data_reduction_proxy/content/renderer/content_previews_render_frame_observer.cc
(right):

https://codereview.chromium.org/2963913002/diff/20001/components/data_reducti...
components/data_reduction_proxy/content/renderer/content_previews_render_frame_observer.cc:23:
bool HasEmptyImageDirective(const blink::WebURLResponse& web_url_response) {
On 2017/06/30 21:03:36, dougarnett wrote:
> Bummer that git didn't identify the move of this code. Does git have it own
> hinting mechanism (like git5 hint)?

I couldn't find a sure way to do this.

https://codereview.chromium.org/2963913002/diff/20001/components/data_reducti...
components/data_reduction_proxy/content/renderer/content_previews_render_frame_observer.cc:79:
if (web_url_response
On 2017/06/30 21:03:36, dougarnett wrote:
> Consider adding TODO to leverage common code in drp_headers (eg, adapt
> ParseResponseTransform if can work out how to deal with different http header
> object types).

Can I put this TODO on you?

[megjablon, 2017-07-06 19:18:53]

megjablon@chromium.org changed reviewers:
+ nasko@chromium.org, ryansturm@chromium.org, thestig@chromium.org

[megjablon, 2017-07-06 19:18:53]

nasko: content/*
thestig: chrome/*
ryansturm: components/drp/*

[megjablon, 2017-07-06 19:19:24]

Description was changed from

==========
Factor DRP PreviewsState logic out of content

Move the Data Reduction Proxy header related logic for updating Previews
state out of content and into a RenderFrameObserver in the Data Reduction
Proxy component.

BUG=737159
==========

to

==========
Factor DRP PreviewsState logic out of content

Move the Data Reduction Proxy header related logic for updating Previews
state out of content and into a RenderFrameObserver in the Data Reduction
Proxy component.

BUG=737159
==========

[megjablon, 2017-07-06 19:19:24]

megjablon@chromium.org changed reviewers:
- nasko@chromium.org

[megjablon, 2017-07-06 19:19:55]

megjablon@chromium.org changed reviewers:
+ creis@chromium.org

[megjablon, 2017-07-06 19:19:56]

+creis for content/* since nasko is out

[Lei Zhang, 2017-07-06 19:43:41]

On 2017/07/06 19:18:53, megjablon wrote:
> thestig: chrome/*

lgtm

[Charlie Reis, 2017-07-06 20:12:21]

content/ LGTM

https://codereview.chromium.org/2963913002/diff/60001/components/data_reducti...
File
components/data_reduction_proxy/content/renderer/content_previews_render_frame_observer.h
(right):

https://codereview.chromium.org/2963913002/diff/60001/components/data_reducti...
components/data_reduction_proxy/content/renderer/content_previews_render_frame_observer.h:15:
class ContentPreviewsRenderFrameObserver : public content::RenderFrameObserver {
Can you add a comment about lifetime and who owns instances of this class? 
(Looks like the class deletes itself when the RenderFrame it is observing goes
away?)

[RyanSturm, 2017-07-06 20:57:02]

lgtm

[dougarnett, 2017-07-06 21:07:07]

lgtm

[megjablon, 2017-07-07 17:32:23]

megjablon@chromium.org changed reviewers:
+ blundell@chromium.org

[megjablon, 2017-07-07 17:32:25]

blundell: components/BUILD.gn

[blundell, 2017-07-09 22:27:30]

lgtm

[megjablon, 2017-07-10 17:37:36]

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

[commit-bot: I haz the power, 2017-07-10 17:37:44]

Dry run: CQ is trying da patch.

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

[megjablon, 2017-07-10 17:38:09]

https://codereview.chromium.org/2963913002/diff/60001/components/data_reducti...
File
components/data_reduction_proxy/content/renderer/content_previews_render_frame_observer.h
(right):

https://codereview.chromium.org/2963913002/diff/60001/components/data_reducti...
components/data_reduction_proxy/content/renderer/content_previews_render_frame_observer.h:15:
class ContentPreviewsRenderFrameObserver : public content::RenderFrameObserver {
On 2017/07/06 20:12:21, Charlie Reis (slow) wrote:
> Can you add a comment about lifetime and who owns instances of this class? 
> (Looks like the class deletes itself when the RenderFrame it is observing goes
> away?)

Done.

[commit-bot: I haz the power, 2017-07-10 17:40:36]

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

[commit-bot: I haz the power, 2017-07-10 17:40:36]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[megjablon, 2017-07-10 17:52:05]

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

[commit-bot: I haz the power, 2017-07-10 17:52:17]

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, 2017-07-10 19:30:23]

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

[commit-bot: I haz the power, 2017-07-10 19:30:24]

Dry run: This issue passed the CQ dry run.

[megjablon, 2017-07-10 20:12:16]

The CQ bit was checked by megjablon@chromium.org

[megjablon, 2017-07-10 20:12:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, creis@chromium.org, ryansturm@chromium.org,
dougarnett@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/2963913002/#ps100001
(title: "gclient sync")

[commit-bot: I haz the power, 2017-07-10 20:12:38]

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, 2017-07-10 20:21:28]

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

[commit-bot: I haz the power, 2017-07-10 20:21:29]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[megjablon, 2017-07-10 20:29:02]

megjablon@chromium.org changed reviewers:
+ foolip@chromium.org

[megjablon, 2017-07-10 20:29:03]

+foolip components/data_reduction_proxy/content/renderer/DEPS

[foolip, 2017-07-31 08:08:45]

On 2017/07/10 20:29:03, megjablon wrote:
> +foolip components/data_reduction_proxy/content/renderer/DEPS

I was OOO, but I'm not an owner in that directory.

[blundell, 2017-07-31 09:31:15]

On 2017/07/31 08:08:45, foolip wrote:
> On 2017/07/10 20:29:03, megjablon wrote:
> > +foolip components/data_reduction_proxy/content/renderer/DEPS
> 
> I was OOO, but I'm not an owner in that directory.

(the request is for a stamp from a //third_party/WebKit/public OWNER for the new
deps in the referenced directory)

[foolip, 2017-07-31 10:00:23]

On 2017/07/31 09:31:15, blundell wrote:
> On 2017/07/31 08:08:45, foolip wrote:
> > On 2017/07/10 20:29:03, megjablon wrote:
> > > +foolip components/data_reduction_proxy/content/renderer/DEPS
> > 
> > I was OOO, but I'm not an owner in that directory.
> 
> (the request is for a stamp from a //third_party/WebKit/public OWNER for the
new
> deps in the referenced directory)

Ah. LGTM.

[megjablon, 2017-08-01 03:54:41]

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

[commit-bot: I haz the power, 2017-08-01 03:54:45]

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, 2017-08-01 05:38:16]

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

[commit-bot: I haz the power, 2017-08-01 05:38:17]

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

[megjablon, 2017-08-01 20:37:21]

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

[commit-bot: I haz the power, 2017-08-01 20:37:31]

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, 2017-08-01 21:45:37]

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

[commit-bot: I haz the power, 2017-08-01 21:45:38]

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

[megjablon, 2017-08-07 18:53:36]

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

[commit-bot: I haz the power, 2017-08-07 18:53:44]

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, 2017-08-07 19:08:36]

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

[commit-bot: I haz the power, 2017-08-07 19:08:37]

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

[megjablon, 2017-08-07 19:19:39]

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

[commit-bot: I haz the power, 2017-08-07 19:19:46]

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, 2017-08-07 19:33:37]

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

[commit-bot: I haz the power, 2017-08-07 19:33:38]

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

[megjablon, 2017-08-07 19:38:21]

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

[commit-bot: I haz the power, 2017-08-07 19:38:26]

Dry run: CQ is trying da patch.

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

[megjablon, 2017-08-07 20:19:36]

Patchset #6 (id:140001) has been deleted

[commit-bot: I haz the power, 2017-08-07 21:30:11]

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

[commit-bot: I haz the power, 2017-08-07 21:30:14]

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

[megjablon, 2017-08-07 21:34:32]

The CQ bit was checked by megjablon@chromium.org

[megjablon, 2017-08-07 21:34:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
ryansturm@chromium.org, thestig@chromium.org, dougarnett@chromium.org,
foolip@chromium.org, creis@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/2963913002/#ps160001
(title: "Change WebDataSource to WebDocumentLoader")

[commit-bot: I haz the power, 2017-08-07 21:34:41]

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, 2017-08-07 23:54:05]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1502141672399420,
"parent_rev": "9c2de32002d1dc7a882404343bb734b0551a67b6", "commit_rev":
"99bda69713b293bd5c2bc33be9170600fe17ae3e"}

[commit-bot: I haz the power, 2017-08-07 23:54:28]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1502141672399420,
"parent_rev": "b79ebf26322a0a9f0af1ecf8bc844cb8a7b218a4", "commit_rev":
"9ee542da746d166a094a705ed3fcd7304dfe61e0"}

[commit-bot: I haz the power, 2017-08-07 23:54:50]

Description was changed from

==========
Factor DRP PreviewsState logic out of content

Move the Data Reduction Proxy header related logic for updating Previews
state out of content and into a RenderFrameObserver in the Data Reduction
Proxy component.

BUG=737159
==========

to

==========
Factor DRP PreviewsState logic out of content

Move the Data Reduction Proxy header related logic for updating Previews
state out of content and into a RenderFrameObserver in the Data Reduction
Proxy component.

BUG=737159

Review-Url: https://codereview.chromium.org/2963913002
Cr-Commit-Position: refs/heads/master@{#492458}
Committed:
https://chromium.googlesource.com/chromium/src/+/9ee542da746d166a094a705ed3fc...
==========

[commit-bot: I haz the power, 2017-08-07 23:54:52]

Committed patchset #6 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/9ee542da746d166a094a705ed3fc...