Make UnsafeResource hold a WebContents getter instead of RenderFrame ids

Make UnsafeResource hold a WebContents getter instead of RenderFrame ids

This is as a precursor to creating a SafeBrowsingNavigationThrottle,
where there won't be RenderFrame ids available.

R=jam@chromium.org
BUG=575215
Committed: https://crrev.com/22e4f25a4aeaa41742074c9aa5ce7a2c10d14448
Cr-Commit-Position: refs/heads/master@{#412049}

[scottmg, 2016-08-11 23:35:23]

Description was changed from

==========
Make UnsafeResource hold a WebContents instead of RenderFrame ids

BUG=575215
==========

to

==========
Make UnsafeResource hold a WebContents instead of RenderFrame ids

This is as a precursor to creating a SafeBrowsingNavigationThrottle,
where there won't be RenderFrame ids available, only the WebContents.

BUG=575215
==========

[scottmg, 2016-08-11 23:38:21]

Description was changed from

==========
Make UnsafeResource hold a WebContents instead of RenderFrame ids

This is as a precursor to creating a SafeBrowsingNavigationThrottle,
where there won't be RenderFrame ids available, only the WebContents.

BUG=575215
==========

to

==========
Make UnsafeResource hold a WebContents instead of RenderFrame ids

This is as a precursor to creating a SafeBrowsingNavigationThrottle,
where there won't be RenderFrame ids available, only the WebContents.

R=sky@chromium.org
BUG=575215
==========

[scottmg, 2016-08-11 23:38:22]

scottmg@chromium.org changed reviewers:
+ sky@chromium.org

[scottmg, 2016-08-11 23:39:20]

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

[commit-bot: I haz the power, 2016-08-11 23:40:19]

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

[scottmg, 2016-08-11 23:42:31]

Description was changed from

==========
Make UnsafeResource hold a WebContents instead of RenderFrame ids

This is as a precursor to creating a SafeBrowsingNavigationThrottle,
where there won't be RenderFrame ids available, only the WebContents.

R=sky@chromium.org
BUG=575215
==========

to

==========
Make UnsafeResource hold a WebContents instead of RenderFrame ids

This is as a precursor to creating a SafeBrowsingNavigationThrottle,
where there won't be RenderFrame ids available, only the WebContents.

R=jam@chromium.org
BUG=575215
==========

[scottmg, 2016-08-11 23:42:32]

scottmg@chromium.org changed reviewers:
+ jam@chromium.org
- sky@chromium.org

[commit-bot: I haz the power, 2016-08-11 23:53:20]

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

[commit-bot: I haz the power, 2016-08-11 23:53:21]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  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_compile_dbg_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_...)

[scottmg, 2016-08-12 00:32:56]

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

[commit-bot: I haz the power, 2016-08-12 00:33:56]

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-08-12 00:59:05]

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

[commit-bot: I haz the power, 2016-08-12 00:59:06]

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...)

[scottmg, 2016-08-12 01:15:59]

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

[scottmg, 2016-08-12 01:16:15]

Patchset #2 (id:20001) has been deleted

[commit-bot: I haz the power, 2016-08-12 01:16:20]

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-08-12 01:41:01]

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

[commit-bot: I haz the power, 2016-08-12 01:41:02]

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...)

[scottmg, 2016-08-12 01:42:53]

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

[commit-bot: I haz the power, 2016-08-12 01:43:14]

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-08-12 02:44:04]

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

[commit-bot: I haz the power, 2016-08-12 02:44:05]

Dry run: This issue passed the CQ dry run.

[jam, 2016-08-12 17:50:53]

per discussion, storing the raw WebContents pointers isn't safe since there
thread hops. we should store callbacks that dynamically get the WebContents.

[scottmg, 2016-08-12 19:48:37]

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

[commit-bot: I haz the power, 2016-08-12 19:49:15]

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

[scottmg, 2016-08-12 19:50:51]

Description was changed from

==========
Make UnsafeResource hold a WebContents instead of RenderFrame ids

This is as a precursor to creating a SafeBrowsingNavigationThrottle,
where there won't be RenderFrame ids available, only the WebContents.

R=jam@chromium.org
BUG=575215
==========

to

==========
Make UnsafeResource hold a WebContents getter instead of RenderFrame ids

This is as a precursor to creating a SafeBrowsingNavigationThrottle,
where there won't be RenderFrame ids available, only the WebContents.

R=jam@chromium.org
BUG=575215
==========

[scottmg, 2016-08-12 19:51:08]

Description was changed from

==========
Make UnsafeResource hold a WebContents getter instead of RenderFrame ids

This is as a precursor to creating a SafeBrowsingNavigationThrottle,
where there won't be RenderFrame ids available, only the WebContents.

R=jam@chromium.org
BUG=575215
==========

to

==========
Make UnsafeResource hold a WebContents getter instead of RenderFrame ids

This is as a precursor to creating a SafeBrowsingNavigationThrottle,
where there won't be RenderFrame ids available.

R=jam@chromium.org
BUG=575215
==========

[scottmg, 2016-08-12 19:57:30]

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

[commit-bot: I haz the power, 2016-08-12 19:58:11]

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

[scottmg, 2016-08-12 20:00:09]

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

[commit-bot: I haz the power, 2016-08-12 20:00:13]

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

[scottmg, 2016-08-12 20:00:36]

Patchset #6 (id:40002) has been deleted

[scottmg, 2016-08-12 20:00:45]

Patchset #5 (id:100001) has been deleted

[scottmg, 2016-08-12 20:01:28]

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

[commit-bot: I haz the power, 2016-08-12 20:02:07]

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

[scottmg, 2016-08-12 20:06:20]

On 2016/08/12 17:50:53, jam wrote:
> per discussion, storing the raw WebContents pointers isn't safe since there
> thread hops. we should store callbacks that dynamically get the WebContents.

thank you for explaining, ptal.

[commit-bot: I haz the power, 2016-08-12 20:44:31]

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

[commit-bot: I haz the power, 2016-08-12 20:44:32]

Dry run: This issue passed the CQ dry run.

[jam, 2016-08-15 19:49:10]

lgtm

https://codereview.chromium.org/2241673002/diff/130001/chrome/browser/rendere...
File
chrome/browser/renderer_host/data_reduction_proxy_resource_throttle_android.cc
(right):

https://codereview.chromium.org/2241673002/diff/130001/chrome/browser/rendere...
chrome/browser/renderer_host/data_reduction_proxy_resource_throttle_android.cc:124:
int render_frame_id) {
ditto

https://codereview.chromium.org/2241673002/diff/130001/chrome/browser/rendere...
File chrome/browser/renderer_host/safe_browsing_resource_throttle.cc (right):

https://codereview.chromium.org/2241673002/diff/130001/chrome/browser/rendere...
chrome/browser/renderer_host/safe_browsing_resource_throttle.cc:285: if (rfh) {
nit: no need to pass in the two IDs, just call the getter here and null check
the WC instead of the RFH

[scottmg, 2016-08-15 19:53:15]

Patchset #6 (id:150001) has been deleted

[scottmg, 2016-08-15 19:55:58]

Patchset #6 (id:170001) has been deleted

[scottmg, 2016-08-15 19:57:18]

thanks

https://codereview.chromium.org/2241673002/diff/130001/chrome/browser/rendere...
File
chrome/browser/renderer_host/data_reduction_proxy_resource_throttle_android.cc
(right):

https://codereview.chromium.org/2241673002/diff/130001/chrome/browser/rendere...
chrome/browser/renderer_host/data_reduction_proxy_resource_throttle_android.cc:124:
int render_frame_id) {
On 2016/08/15 19:49:10, jam wrote:
> ditto

Done.

https://codereview.chromium.org/2241673002/diff/130001/chrome/browser/rendere...
File chrome/browser/renderer_host/safe_browsing_resource_throttle.cc (right):

https://codereview.chromium.org/2241673002/diff/130001/chrome/browser/rendere...
chrome/browser/renderer_host/safe_browsing_resource_throttle.cc:285: if (rfh) {
On 2016/08/15 19:49:10, jam wrote:
> nit: no need to pass in the two IDs, just call the getter here and null check
> the WC instead of the RFH

Done.

[scottmg, 2016-08-15 19:57:28]

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

[commit-bot: I haz the power, 2016-08-15 19:57:59]

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-08-15 21:00:13]

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

[commit-bot: I haz the power, 2016-08-15 21:00:14]

Dry run: This issue passed the CQ dry run.

[scottmg, 2016-08-15 21:04:44]

The CQ bit was checked by scottmg@chromium.org

[scottmg, 2016-08-15 21:04:45]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2241673002/#ps190001
(title: ".")

[commit-bot: I haz the power, 2016-08-15 21:05:05]

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-08-15 21:09:21]

Committed patchset #6 (id:190001)

[commit-bot: I haz the power, 2016-08-15 21:12:49]

Description was changed from

==========
Make UnsafeResource hold a WebContents getter instead of RenderFrame ids

This is as a precursor to creating a SafeBrowsingNavigationThrottle,
where there won't be RenderFrame ids available.

R=jam@chromium.org
BUG=575215
==========

to

==========
Make UnsafeResource hold a WebContents getter instead of RenderFrame ids

This is as a precursor to creating a SafeBrowsingNavigationThrottle,
where there won't be RenderFrame ids available.

R=jam@chromium.org
BUG=575215

Committed: https://crrev.com/22e4f25a4aeaa41742074c9aa5ce7a2c10d14448
Cr-Commit-Position: refs/heads/master@{#412049}
==========

[commit-bot: I haz the power, 2016-08-15 21:12:50]

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