Preserve uninitialized state for effectAllowed on drop events [chromium/src : main]
Hyowon Kim (Gerrit)
Hyowon Kim added 1 comment![]( "Open in Gerrit")
effect_allowed_ = AtomicString("uninitialized");Hyowon KimPlease add unitialized into the keywords.json5 and use it as keywords::kUninitialized here (and other places in this file).
Done
Related details
- Chromium IPC Reviews
- Dave Tapuska
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: hide...@chromium.org
📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).
IPC reviewer(s): hide...@chromium.org
Reviewer source(s):
hide...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
- Dave Tapuska
- Hidehiko Abe
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dave Tapuska (Gerrit)
Dave Tapuska voted Code-Review+1![]( "Open in Gerrit")
- Hidehiko Abe
- Hyowon Kim
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hidehiko Abe (Gerrit)
Hidehiko Abe added 1 comment![]( "Open in Gerrit")
This happened because both values were converted to the same operation
mask `kDragOperationEvery` and we lost uninitialized state.If the degeneration is the problem, why we cannot assign the value into different state? If they need to be handled in the same way, clients of the value can handle these (newly) distinguished values as a same one?
Related details
- Hyowon Kim
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hyowon Kim (Gerrit)
Hyowon Kim added 1 comment![]( "Open in Gerrit")
This happened because both values were converted to the same operation
mask `kDragOperationEvery` and we lost uninitialized state.If the degeneration is the problem, why we cannot assign the value into different state? If they need to be handled in the same way, clients of the value can handle these (newly) distinguished values as a same one?
Yes, I agree the core problem is that `uninitialized` and `all` both become `kDragOperationEvery`.
So this CL adds a separate state, `source_effect_allowed_is_uninitialized`, and keeps it through the drag data path.
If your question is why I did not add it to `DragOperationsMask` itself, my understanding is that `DragOperationsMask` is an operation bitmask, while `uninitialized` is a state, not an operation.
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/common/page/drag_operation.h;l=38-40
If I misunderstood your question, or if there is a better way to add a separate state, please let me know. Thank you for the review.
Related details
- Hidehiko Abe
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |