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

Issue 1549793003: [proxies] Improve JSProxyVerify and test most proxy invariants. (Closed)

Created:
4 years, 12 months ago by Camillo Bruni
Modified:
4 years, 12 months ago
Reviewers:
Igor Sheludko
CC:
v8-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[proxies] Improve JSProxyVerify and test most proxy invariants. BUG= Committed: https://crrev.com/f17c1d11408452548564f50c392cb8fa23a2bbdd Cr-Commit-Position: refs/heads/master@{#33032}

Patch Set 1 #

Total comments: 3

Patch Set 2 : addressing nits #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -0 lines) Patch
M src/objects-debug.cc View 1 1 chunk +6 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (5 generated)
Camillo Bruni
PTAL
4 years, 12 months ago (2015-12-23 16:53:29 UTC) #2
Igor Sheludko
lgtm with nits: https://codereview.chromium.org/1549793003/diff/1/src/objects-debug.cc File src/objects-debug.cc (right): https://codereview.chromium.org/1549793003/diff/1/src/objects-debug.cc#newcode823 src/objects-debug.cc:823: if (target()->IsCallable()) { You could probably ...
4 years, 12 months ago (2015-12-23 17:04:52 UTC) #3
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1549793003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1549793003/1
4 years, 12 months ago (2015-12-23 17:14:41 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1549793003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1549793003/20001
4 years, 12 months ago (2015-12-23 17:17:01 UTC) #8
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 12 months ago (2015-12-23 17:37:46 UTC) #9
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/f17c1d11408452548564f50c392cb8fa23a2bbdd Cr-Commit-Position: refs/heads/master@{#33032}
4 years, 12 months ago (2015-12-23 17:38:22 UTC) #11
Camillo Bruni
4 years, 12 months ago (2015-12-23 19:07:01 UTC) #12
Message was sent while issue was closed.
https://codereview.chromium.org/1549793003/diff/1/src/objects-debug.cc
File src/objects-debug.cc (right):

https://codereview.chromium.org/1549793003/diff/1/src/objects-debug.cc#newcod...
src/objects-debug.cc:823: if (target()->IsCallable()) {
On 2015/12/23 at 17:04:52, Igor Sheludko wrote:
> You could probably rewrite these ifs like this:
>   CHECK_EQ(target()->IsCallable(), map()->is_callable());
>   CHECK_EQ(target()->IsConstructor(), map()->is_constructor());

Sometimes I really wonder how I got so far with programming :D

Powered by Google App Engine
This is Rietveld 408576698