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

Issue 2169453002: Sprinkle some release asserts over HTMLFrameElementBase (Closed)

Created:
4 years, 5 months ago by jochen (gone - plz use gerrit)
Modified:
4 years, 1 month 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

Sprinkle some release asserts over HTMLFrameElementBase It appears to be difficult to get this right, but it's quite bad if we get it wrong, so rather crash. BUG=628942 R=dcheng@chromium.org Committed: https://crrev.com/b9e3d96067ff8c016d2505ab2a1f6b9d985c6b35 Cr-Commit-Position: refs/heads/master@{#407430}

Patch Set 1 #

Patch Set 2 : updates #

Patch Set 3 : updates #

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

Messages

Total messages: 25 (14 generated)
jochen (gone - plz use gerrit)
4 years, 5 months ago (2016-07-20 10:33:21 UTC) #1
jochen (gone - plz use gerrit)
ptal - now with less redness
4 years, 5 months ago (2016-07-25 07:41:37 UTC) #12
dominicc (has gone to gerrit)
This is a bit subtle but I guess LGTM, seems like a way to find ...
4 years, 5 months ago (2016-07-25 07:43:59 UTC) #13
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/2169453002/40001
4 years, 5 months ago (2016-07-25 07:55:10 UTC) #16
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 5 months ago (2016-07-25 08:19:59 UTC) #17
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/b9e3d96067ff8c016d2505ab2a1f6b9d985c6b35 Cr-Commit-Position: refs/heads/master@{#407430}
4 years, 5 months ago (2016-07-25 08:38:00 UTC) #19
tkent
> Patchset 3 (id:??) landed as https://crrev.com/b9e3d96067ff8c016d2505ab2a1f6b9d985c6b35 > Cr-Commit-Position: refs/heads/master@{#407430} I'm afraid this made telemetry_perf_unittests ...
4 years, 5 months ago (2016-07-26 08:02:02 UTC) #20
jochen (gone - plz use gerrit)
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/2175313005/ by jochen@chromium.org. ...
4 years, 5 months ago (2016-07-26 09:55:38 UTC) #21
jochen (gone - plz use gerrit)
On 2016/07/26 at 09:55:38, jochen wrote: > A revert of this CL (patchset #3 id:40001) ...
4 years, 5 months ago (2016-07-26 09:55:57 UTC) #22
esprehn
lgtm
4 years, 1 month ago (2016-11-12 09:46:53 UTC) #24
dcheng
4 years, 1 month ago (2016-11-12 18:14:11 UTC) #25
Message was sent while issue was closed.
On 2016/11/12 09:46:53, esprehn wrote:
> lgtm

Did you look at https://codereview.chromium.org/2497133002/ as well, which is
the proposed reland? It's a bit more narrowly scoped, but in all the UXSSes I've
seen, the contentFrame() assert is the important one: that's why my reland only
includes it.

Powered by Google App Engine
This is Rietveld 408576698