Issue 15859009: Revert 202821 "Protect WebURLLoaderImpl::Context while receiving..."

Issue 15859009: Revert 202821 "Protect WebURLLoaderImpl::Context while receiving..." (Closed)

Created:
7 years, 6 months ago by hashimoto

Modified:
7 years, 6 months ago

Reviewers:
gavinp

CC:
chromium-reviews

Base URL:
svn://svn.chromium.org/chrome/

Visibility:
Public.

More Reviews

Description

Revert 202821 "Protect WebURLLoaderImpl::Context while receiving..." > Protect WebURLLoaderImpl::Context while receiving responses. > > A client's didReceiveResponse can cancel a request; by protecting the > Context we avoid a use after free in this case. > > Interestingly, we really had very good warning about this problem, see > https://codereview.chromium.org/11900002/ back in January. > > R=darin > BUG=241139 > > Review URL: https://chromiumcodereview.appspot.com/15738007 TBR=gavinp@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=202835

Patch Set 1 #

Created: 7 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+0 lines, -22 lines)			Patch
M	trunk/src/content/browser/webkit_browsertest.cc	View	1 chunk	+0 lines, -15 lines	0 comments	Download
D	trunk/src/content/test/data/error-body-no-crash.html	View	1 chunk	+0 lines, -6 lines	0 comments	Download
M	trunk/src/webkit/glue/weburlloader_impl.cc	View	1 chunk	+0 lines, -1 line	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

gavinp

Please add a comment linking to failing tests. On May 29, 2013 7:25 AM, <hashimoto@chromium.org> ...

7 years, 6 months ago (2013-05-29 11:26:43 UTC) #3

hashimoto

Reverted 202821 to fix failing content_browsertests: http://build.chromium.org/p/chromium.mac/builders/Mac%2010.7%20Tests%20%28dbg%29%281%29/builds/11458 http://build.chromium.org/p/chromium.mac/builders/Mac%2010.6%20Tests%20%28dbg%29%281%29/builds/37743 http://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29%281%29/builds/25937 http://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29%281%29%2832%29/builds/1322 http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%282%29/builds/8195

7 years, 6 months ago (2013-05-29 11:27:30 UTC) #4

hashimoto

Oops, URLs are double escaped. Please refer for the following bots. "Mac 10.7 Tests (dbg)(1)", ...

7 years, 6 months ago (2013-05-29 11:30:34 UTC) #5

gavinp

Thanks. I almost think disabling the test would have been a better call. I'll look ...

7 years, 6 months ago (2013-05-29 11:39:36 UTC) #6

hashimoto

7 years, 6 months ago (2013-05-29 12:09:46 UTC) #7

Message was sent while issue was closed.

On 2013/05/29 11:39:36, gavinp wrote:
> Thanks.
> 
> I almost think disabling the test would have been a better call. I'll look
> at this closely today.
Sorry, since I couldn't find any try results without content_browsertests
skipped on the original review page, I had no clue about the reliability of your
change on debug builds and was tempted to choose the most straight-forward
solution.

> On May 29, 2013 7:30 AM, <mailto:hashimoto@chromium.org> wrote:
> 
> > Oops, URLs are double escaped.
> > Please refer for the following bots. "Mac 10.7 Tests (dbg)(1)", "Mac 10.6
> > Tests
> > (dbg)(1)", "Linux Tests (dbg)(1)", "Linux Tests (dbg)(1)(32)", "Linux
> > ChromiumOS
> > Tests (dbg)(2)"
> >
> >
>
https://codereview.chromium.**org/15859009/%3Chttps://codereview.chromium.org...>
> >

Expand Messages | Collapse Messages