Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2544253002:
Update documentation for [CallWith] and [ConstructorCallWith] (Closed)
DescriptionUpdate documentation for [CallWith] and [ConstructorCallWith]
BUG=669812
Committed: https://crrev.com/efbd06c5569a5a7e6e2dda507e3402bf7eec5675
Cr-Commit-Position: refs/heads/master@{#435879}
Patch Set 1 #Patch Set 2 : temp #
Total comments: 5
Patch Set 3 : temp #Messages
Total messages: 13 (6 generated)
haraken@chromium.org changed reviewers: + bashi@chromium.org
PTAL https://codereview.chromium.org/2544253002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/bindings/IDLExtendedAttributes.md (left): https://codereview.chromium.org/2544253002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/IDLExtendedAttributes.md:555: Multiple values can be specified e.g. `[CallWith=ScriptState|ScriptArguments]`. The order of the values in the IDL file doesn't matter: the generated parameter list is always in a fixed order (specifically `&state`, `scriptContext`, `scriptArguments.release()`, if present, corresponding to `ScriptState`, `ScriptExecutionContext`, `ScriptArguments`, respectively). No one is using the multiple value version of [CallWith=].
yukishiino@chromium.org changed reviewers: + yukishiino@chromium.org
LGTM with a question. https://codereview.chromium.org/2544253002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/bindings/IDLExtendedAttributes.md (right): https://codereview.chromium.org/2544253002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/IDLExtendedAttributes.md:585: Use `[CallWith=ScriptState]` instead. Just curious. Are we going to deprecate CallWith=ExecutionContext? Or is ExecutionContext discouraged in general? I'm just curious about the background and reasons.
lgtm, Thank you for updating the document! https://codereview.chromium.org/2544253002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/bindings/IDLExtendedAttributes.md (right): https://codereview.chromium.org/2544253002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/IDLExtendedAttributes.md:660: Be careful when you use `[ConstructorCallWith=ScriptState]`. BTW, can this warning be said for [CallWith=ScriptState] as well? A DOM method which takes ScriptState shouldn't store it for later use? Or is it not a problem?
https://codereview.chromium.org/2544253002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/bindings/IDLExtendedAttributes.md (right): https://codereview.chromium.org/2544253002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/IDLExtendedAttributes.md:585: Use `[CallWith=ScriptState]` instead. On 2016/12/02 06:39:18, Yuki wrote: > Just curious. Are we going to deprecate CallWith=ExecutionContext? Or is > ExecutionContext discouraged in general? I'm just curious about the background > and reasons. Yes. I think CallWith=ExecutionContext should just be replaced with CallWith=ScriptState. https://codereview.chromium.org/2544253002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/IDLExtendedAttributes.md:660: Be careful when you use `[ConstructorCallWith=ScriptState]`. On 2016/12/02 06:43:00, bashi1 wrote: > BTW, can this warning be said for [CallWith=ScriptState] as well? A DOM method > which takes ScriptState shouldn't store it for later use? Or is it not a > problem? I added a comment to [CallWith=ScriptState] as well.
The CQ bit was checked by haraken@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bashi@chromium.org, yukishiino@chromium.org Link to the patchset: https://codereview.chromium.org/2544253002/#ps40001 (title: "temp")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1480662159187070,
"parent_rev": "078283330a8b27c18a071bdd8095495084eeb564", "commit_rev":
"e0b6ab97803f2e0549b73615d02e3ddc83043e4d"}
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Update documentation for [CallWith] and [ConstructorCallWith] BUG=669812 ========== to ========== Update documentation for [CallWith] and [ConstructorCallWith] BUG=669812 Committed: https://crrev.com/efbd06c5569a5a7e6e2dda507e3402bf7eec5675 Cr-Commit-Position: refs/heads/master@{#435879} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/efbd06c5569a5a7e6e2dda507e3402bf7eec5675 Cr-Commit-Position: refs/heads/master@{#435879} |
