Support foregroundtask registration for packaged applications#1451
Support foregroundtask registration for packaged applications#1451
Conversation
ghost
added
the
loneursid
added
the
area-Notifications
sharath2727
force-pushed
the
user/vemancha/foregroundtaskforpackagedapplications
branch
from
4a6e6bc to
d47e323
Compare
sharath2727
force-pushed
the
user/vemancha/foregroundtaskforpackagedapplications
branch
from
5e4110d to
c951092
Compare
sharath2727
changed the title
jonwis
left a comment
jonwis
left a comment
There was a problem hiding this comment.
Two things to change, the rest are optional - use of that "count" thing to track number of registered handlers, and the missing function-level try/catch on the noexcept method. Otherwise looks pretty good, glad to see this coming together.
|
|
||
| // Escape special characters to follow command line standards for any app activation type in AppLifecycle | ||
| // (See AppInstance.cpp and Serialize() from other activation types) | ||
| std::wstring payloadAsWideString = ConvertByteArrayToWideString(payloadLength, payload); |
There was a problem hiding this comment.
@aeloros can you check this? You were working on similar "command line sanitization" issues as well.
There was a problem hiding this comment.
I can't remember if we talked about this before or not, but I believe we would be better served not creating some new payload model. Instead we should create a contractId for the push server, and use the already existing infrastructure to pass properties through the query string. This might be an example of a fragile design since it requires continued upkeep to make it special.
There was a problem hiding this comment.
@jonwis /@aeloros - I see howard introducing a lot of string conversion APIs in dev/common: #1513. Once the PR merges some of these conversion APIs in my .h/.cpp will be removed.
@aeloros - "Instead we should create a contractId for the push server, and use the already existing infrastructure to pass properties through the query string. This might be an example of a fragile design since it requires continued upkeep to make it special" - can you please help me understand this further? Maybe I could work on this going ahead to simplify things here.
There was a problem hiding this comment.
Sure, the command-line on the receiving end (when GetActivatedEventArgs() parses it) already has a format that supports passing payload information. Specifically I'm talking about the ability to pass a Uri that contains a key/value pair (the query string).
There are problems with the current implementation of GetActivatedEventArgs() in that it doesn't tolerate malformed command-lines well. I have a change out to fix that, but it changes the expectations of the format as well.
I'm mostly suggesting here that we should try to find a better way to utilize the command-line. One that doesn't have the calling developer designing custom formats to support payloads. We should instead find a way to utilize the Uri format and serialize + encode data into that format.
This also lends itself to creating shared functions in the code base that generate that command-line. I've started to do that in a super simple way with GenerateCommandLine() inside of ActivationRegistrationManager.cpp. It could be moved and enhanced or accompanied with other helper functions to make a useful reusable toolset for callers to create command-lines in a consistent way.
I don't think any of this is the end of the world if not done. At this point in the conversation though, it probably would be intentional technical debt. Obviously we should avoid that when possible, but dealing with technical debt isn't the only factor in our lives. :)
loneursid
deleted the
user/vemancha/foregroundtaskforpackagedapplications
branch
loneursid
restored the
user/vemancha/foregroundtaskforpackagedapplications
branch
|
So, I've messed up. I've acidentaly merged this branch into WNP_LRP. As a result GIT closed this PR and the branch was deleted. I've reverted the merge from WNP_LRP and restored the branch. Still trying to figure out how to re-open this PR. |
8 participants
No description provided.