[ABANDONED] Make sure that threaded-data-provider doesn't cause a use-after-free.

Make sure that threaded-data-provider doesn't cause a use-after-free.

As ThreadedDataProvider::OnReceivedDataOnForegroundThread passes the given
pointer to another thread, it requires the pointer to be valid until it is
used. On ResourceDispatcher::OnReceivedData, the pointer comes from shared
memory buffer, or from std::string data (alternative data). As written above,
it is dangerous to pass the alternative data pointer because it is stack
allocated.

BUG=None
R=oysteine@chromium.org

[yhirano, 2015-04-28 06:17:57]

yhirano@chromium.org changed reviewers:
+ oysteine@chromium.org
- oystein@chromium.org

[yhirano, 2015-04-28 06:23:39]

Hi Oystein, I am writing a CL[1] and I'm not certain about
ThreadedDataProvider behavior. As written in the description, it looks dangerous
because you retain a pointer that can be freed synchronously.
If it is actually safe, I think adding a DCHECK is good for readability.

Can you take a look?

1: https://codereview.chromium.org/1103813002/

[oystein (OOO til 10th of July), 2015-04-28 17:14:45]

On 2015/04/28 06:23:39, yhirano wrote:
> Hi Oystein, I am writing a CL[1] and I'm not certain about
> ThreadedDataProvider behavior. As written in the description, it looks
dangerous
> because you retain a pointer that can be freed synchronously.
> If it is actually safe, I think adding a DCHECK is good for readability.
> 
> Can you take a look?
> 
> 1: https://codereview.chromium.org/1103813002/

Looks like you're right, yep! The site_isolation_policy.cc isn't turned on in
practice, but this'll be dangerous if/when it does. Since all it does is use the
"alternate data" to insert a space (and only when it's blocked), this could do
with some refactoring regardless.

[yhirano, 2015-04-30 01:16:44]

On 2015/04/28 17:14:45, Oystein wrote:
> On 2015/04/28 06:23:39, yhirano wrote:
> > Hi Oystein, I am writing a CL[1] and I'm not certain about
> > ThreadedDataProvider behavior. As written in the description, it looks
> dangerous
> > because you retain a pointer that can be freed synchronously.
> > If it is actually safe, I think adding a DCHECK is good for readability.
> > 
> > Can you take a look?
> > 
> > 1: https://codereview.chromium.org/1103813002/
> 
> Looks like you're right, yep! The site_isolation_policy.cc isn't turned on in
> practice, but this'll be dangerous if/when it does. Since all it does is use
the
> "alternate data" to insert a space (and only when it's blocked), this could do
> with some refactoring regardless.

Thanks, I will add a TODO in https://codereview.chromium.org/1103813002/.