New issue
Advanced search Search tips

Issue 737191 link

Starred by 1 user
Status: Fixed
Owner:
mmenke@chromium.org
Closed: Jun 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Proj-Servicification

Blocking:
issue 736906



Sign in to add a comment

ResourceRequestBodyImpl should not inherit from base::SupportsUserData

Project Member Reported by mmenke@chromium.org, Jun 27 2017 
ResourceRequestBodyImpl is the data type used to pass upload bodies to the NetworkService.  It's fundamentally weird to have a data type used by Mojo inherit from SupportsUserData, since that data can't be passed via Mojo, and external consumers should not be setting UserData on the object.

It looks like this is only used by blobs, as a convenient way to keep them alive from before a URLRequest starts until it's destroyed, so need to find another way to do that.
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 28 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ed44e6c201f6e31ab4dc71b7914628545a551eda

commit ed44e6c201f6e31ab4dc71b7914628545a551eda
Author: mmenke <mmenke@chromium.org>
Date: Wed Jun 28 21:13:32 2017

Remove SupportsUserData from ResourceRequestBodyImpl.

ResourceRequestBodyImpl is sent between processes via IPC and Mojo, and
since we can't send the UserData along with it, it doesn't make sense to
inherit from SupportsUserData. It was only being used to stash upload blob
handles, so this CL juggles around ownership of those.

BUG= 737191 

Review-Url: https://codereview.chromium.org/2960023002
Cr-Commit-Position: refs/heads/master@{#483140}

[modify] https://crrev.com/ed44e6c201f6e31ab4dc71b7914628545a551eda/content/browser/blob_storage/chrome_blob_storage_context.cc
[modify] https://crrev.com/ed44e6c201f6e31ab4dc71b7914628545a551eda/content/browser/blob_storage/chrome_blob_storage_context.h
[modify] https://crrev.com/ed44e6c201f6e31ab4dc71b7914628545a551eda/content/browser/loader/DEPS
[modify] https://crrev.com/ed44e6c201f6e31ab4dc71b7914628545a551eda/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/ed44e6c201f6e31ab4dc71b7914628545a551eda/content/browser/loader/resource_dispatcher_host_impl.cc
[modify] https://crrev.com/ed44e6c201f6e31ab4dc71b7914628545a551eda/content/browser/loader/resource_dispatcher_host_impl.h
[modify] https://crrev.com/ed44e6c201f6e31ab4dc71b7914628545a551eda/content/browser/loader/resource_request_info_impl.cc
[modify] https://crrev.com/ed44e6c201f6e31ab4dc71b7914628545a551eda/content/browser/loader/resource_request_info_impl.h
[modify] https://crrev.com/ed44e6c201f6e31ab4dc71b7914628545a551eda/content/common/resource_request_body_impl.h

Comment 2 by mmenke@chromium.org, Jun 28 2017

Status: Fixed (was: Assigned)

Comment 3 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
►

Sign in to add a comment