Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(142)

Issue 2190523002: Assert that we never insert an frame element that already has a frame (Closed)

Created:
4 years, 4 months ago by jochen (gone - plz use gerrit)
Modified:
4 years ago
CC:
blink-reviews, blink-reviews-html_chromium.org, chromium-reviews, dglazkov+blink
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Assert that we never insert an frame element that already has a frame BUG=628942, 631151 R=dominicc@chromium.org Committed: https://crrev.com/baf4f1f0cca9c704ff01de23e9360a1deef00cb4 Cr-Commit-Position: refs/heads/master@{#436294}

Patch Set 1 #

Patch Set 2 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -0 lines) Patch
M third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp View 1 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 30 (18 generated)
jochen (gone - plz use gerrit)
4 years, 4 months ago (2016-07-27 08:20:58 UTC) #1
dcheng
lgtm
4 years, 4 months ago (2016-07-27 15:05:48 UTC) #8
jochen (gone - plz use gerrit)
note that this will make clusterfuzz unhappy (issue 631151) should we still land?
4 years, 4 months ago (2016-07-27 15:10:24 UTC) #9
dcheng
On 2016/07/27 15:10:24, jochen wrote: > note that this will make clusterfuzz unhappy (issue 631151) ...
4 years, 4 months ago (2016-07-27 15:11:57 UTC) #10
jochen (gone - plz use gerrit)
any updates?
4 years, 4 months ago (2016-07-29 12:42:35 UTC) #11
dominicc (has gone to gerrit)
On 2016/07/29 at 12:42:35, jochen wrote: > any updates? To summarize, the question as I ...
4 years, 2 months ago (2016-10-14 08:31:22 UTC) #12
jochen (gone - plz use gerrit)
green bots.. so giving this a try
4 years ago (2016-12-05 15:17:19 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2190523002/20001
4 years ago (2016-12-05 15:17:31 UTC) #24
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years ago (2016-12-05 15:21:53 UTC) #26
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/baf4f1f0cca9c704ff01de23e9360a1deef00cb4 Cr-Commit-Position: refs/heads/master@{#436294}
4 years ago (2016-12-05 15:23:16 UTC) #28
dcheng
Is anyone actively working on the known crashers for this? marius pointed out a much ...
4 years ago (2016-12-05 18:23:50 UTC) #29
jochen (gone - plz use gerrit)
4 years ago (2016-12-07 10:41:54 UTC) #30
Message was sent while issue was closed.
On 2016/12/05 at 18:23:50, dcheng wrote:
> Is anyone actively working on the known crashers for this? marius pointed out
a much simpler repro than the CF-reported case:
https://codereview.chromium.org/2497133002/#msg9
> 
> And esprehn@ was against landing due to the very simple snippet required to
trigger this.

ah, sorry, didn't see the other CL. I dunno, so far, I didn't get complaints
about crashes, so maybe it's not that common? OTOH, it's a serious issue
everything we hit that check, and it got ignored so far, so maybe crashing will
help to prioritize fixing the underlying behavior in the DOM parser?

Powered by Google App Engine
This is Rietveld 408576698