Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2286323002:
[api] Add documentation for PropertyQueryCallback. (Closed)
|
Created:
4 years, 3 months ago by Franzi Modified:
4 years, 3 months ago Reviewers:
jochen (gone - plz use gerrit) CC:
v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@DocPropertyAttribute Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[api] Add documentation for PropertyQueryCallback.
Also add tests that document the behavior of the PropertyQueryCallback.
BUG=v8:5260
Committed: https://crrev.com/8225465b09118908f4079a19863e174ef60a6f6b
Cr-Commit-Position: refs/heads/master@{#39090}
Patch Set 1 #
Total comments: 3
Patch Set 2 : Improve note about triggering in some cases. #
Depends on Patchset: Messages
Total messages: 20 (10 generated)
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [api] Add documentation for PropertyQueryCallback. Also add tests that document the behavior of the PropertyQueryCallback. BUG=v8:526 ========== to ========== [api] Add documentation for PropertyQueryCallback. Also add tests that document the behavior of the PropertyQueryCallback. BUG=v8:526 ==========
franzih@chromium.org changed reviewers: + jochen@chromium.org
More documentation :)
the BUG= line seems to be incorrect? https://codereview.chromium.org/2286323002/diff/1/include/v8.h File include/v8.h (right): https://codereview.chromium.org/2286323002/diff/1/include/v8.h#newcode4561 include/v8.h:4561: * reasons it is not guaranteed to. wut? :-/
Description was changed from ========== [api] Add documentation for PropertyQueryCallback. Also add tests that document the behavior of the PropertyQueryCallback. BUG=v8:526 ========== to ========== [api] Add documentation for PropertyQueryCallback. Also add tests that document the behavior of the PropertyQueryCallback. BUG=v8:5260 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Hi Jochen, Small documentation change. Can you have another look please? Thanks, Franzi https://codereview.chromium.org/2286323002/diff/1/include/v8.h File include/v8.h (right): https://codereview.chromium.org/2286323002/diff/1/include/v8.h#newcode4561 include/v8.h:4561: * reasons it is not guaranteed to. On 2016/08/29 12:52:35, jochen wrote: > wut? :-/ Is this version better or should we leave out the note?
https://codereview.chromium.org/2286323002/diff/1/include/v8.h File include/v8.h (right): https://codereview.chromium.org/2286323002/diff/1/include/v8.h#newcode4561 include/v8.h:4561: * reasons it is not guaranteed to. On 2016/08/31 at 11:18:36, Franzi wrote: > On 2016/08/29 12:52:35, jochen wrote: > > wut? :-/ > > Is this version better or should we leave out the note? ah, so you mean that hasOwnProperty might or might not call this specific callback, but it will always call some callback on the interceptor?
On 2016/08/31 14:37:50, jochen wrote: > https://codereview.chromium.org/2286323002/diff/1/include/v8.h > File include/v8.h (right): > > https://codereview.chromium.org/2286323002/diff/1/include/v8.h#newcode4561 > include/v8.h:4561: * reasons it is not guaranteed to. > On 2016/08/31 at 11:18:36, Franzi wrote: > > On 2016/08/29 12:52:35, jochen wrote: > > > wut? :-/ > > > > Is this version better or should we leave out the note? > > ah, so you mean that hasOwnProperty might or might not call this specific > callback, Yes. > but it will always call some callback on the interceptor? No, I don't think I'm trying to say that. I noticed that obl.hasOwnProperty('x') calls the query interceptor, if x is not a property of obj. It does not call the interceptor, if x is a property (fast path has SKIP_OWN_INTERCEPTOR set), Since I thought that's confusing, I tried to include it in the documentation.
lgtm
lgtm
The CQ bit was checked by franzih@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [api] Add documentation for PropertyQueryCallback. Also add tests that document the behavior of the PropertyQueryCallback. BUG=v8:5260 ========== to ========== [api] Add documentation for PropertyQueryCallback. Also add tests that document the behavior of the PropertyQueryCallback. BUG=v8:5260 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [api] Add documentation for PropertyQueryCallback. Also add tests that document the behavior of the PropertyQueryCallback. BUG=v8:5260 ========== to ========== [api] Add documentation for PropertyQueryCallback. Also add tests that document the behavior of the PropertyQueryCallback. BUG=v8:5260 Committed: https://crrev.com/8225465b09118908f4079a19863e174ef60a6f6b Cr-Commit-Position: refs/heads/master@{#39090} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/8225465b09118908f4079a19863e174ef60a6f6b Cr-Commit-Position: refs/heads/master@{#39090} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
