Make CA requests that are part of the same user action have the same user_action_id
Categories
(Firefox :: Data Loss Prevention, task)
Tracking
()
People
(Reporter: gstoll, Assigned: gstoll)
References
(Blocks 3 open bugs)
Details
Attachments
(20 files, 5 obsolete files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
We think this will help avoid cases where multiple parts of the same paste, etc. are blocked or warned and the DLP agent pops up their own dialog for each part. (sort of like bug 1927992)
This will require making changes in the clipboard paste but also for file upload. This will also require changes to cancel code since the CancelRequests()
SDK message requires the user_action_id
.
There's another optional field user_action_requests_count
but I'm not sure what the consequences of not filling that out are. (this would be mildly annoying in the clipboard case since we fire off requests one at a time; not sure about file upload)
Assignee | ||
Updated•2 months ago
|
Assignee | ||
Comment 1•1 month ago
|
||
This is helpful for later patches in this stack because when
multiple folders are dragged and dropped onto a page Content Analysis
needs to get all of their contents.
Assignee | ||
Comment 2•1 month ago
|
||
Assignee | ||
Comment 3•1 month ago
|
||
This is helpful so callers don't have to worry about specifying
user_action_id and user_action_requests_count or expanding a request
to analyze a folder into multiple requests for the files contained
inside.
Assignee | ||
Comment 4•1 month ago
|
||
This means the clipboard code is now just responsible for
gathering all the requests together and then it can just
call the new entry point.
Assignee | ||
Comment 5•1 month ago
|
||
Also move the logic for file dialog Content Analysis into
ContentAnalysis.cpp, and make it call the agent in parallel
instead of serialized.
Assignee | ||
Comment 6•1 month ago
|
||
Assignee | ||
Comment 7•1 month ago
|
||
As this will cancel all of the requests with the same userActionId,
this also calls the callbacks for those requests to indicate that.
Comment 8•10 days ago
|
||
Depends on D233537
Comment 9•10 days ago
|
||
Rename "request" variable that is a response to "response".
Depends on D236626
Comment 10•10 days ago
|
||
Remove variant version in favor of two classes.
Use MakeRefPtr as factory and make constructors private so that refcounting
is guaranteed.
Depends on D236627
Comment 11•10 days ago
|
||
Removed flavors from cache since they were not being considered in verdicts.
The cache is all or nothing so it added nothing to the seqno comparison.
This does not include the changes to the non-test callers, which are in another
patch.
Depends on D236628
Comment 12•10 days ago
|
||
The changes include:
- Making various IDL classes builtin.
- Making nsIContentAnalysisResponse subclass nsIContentAnalysisResult.
- Making requestToken and userAction fields internal to CA service.
- Added sourceWindowGlobal (the source of the data to scan) to request and
use it for content-source-is-same-tab determinations. - Made nsIContentAnalysisCallback expect a Result, not a Response.
- Added cancelRequestsByUserAction and renamed cancelContentAnalysisRequest to cancelRequestsByRequestToken (deprecated).
- Added ability for CancelAllRequests to forbid future requests (not needed but for peace of mind).
- Added MakeResponseForTest since response is not builtin.
- Made ContentAnalysis SupportsWeakPtr so that it can be weakly held by the MultipartRequestCallback,
which can't just re-get the service since it doesn't want the mock one. - Move SafeContentAnalysisResultCallback functionality to ContentAnalysisCallback.
- Make various routines use userActionId instead of requestToken.
- Add MultipartRequestCallback to count that we get the right number of responses.
- Remove unneeded thread sync behavior. The only part that was kind-of
neeed was to abort behavior on a background thread if the request were
canceled first -- this is a race that we don't need to win so we now just
ignore it and ignore the result (TODO: I'm not positive I send cancel to
the agent in this case). - Send cancel to the agent (in all other cancel cases).
- Simplify the request map, which is now the user action map mUserActionMap.
- Makes the MultipartRequestCallback the nexus for filtering requests --
this way filters are applied uniformly instead of sometimes-at-some-point-
for-some-use-cases.
Depends on D236629
Comment 13•10 days ago
|
||
AnalyzeContentRequest* calls now require fewer fields. For example, the
URI is, by default, now obtained from the request's WindowGlobalParent's
BrowsingContext.
The commonDialog promise change is cosmetic but avoids async/await, which is nice.
browser-custom-element.js needed to re-get service for tests, since it was
sometimes getting an old mock version from a previous test.
ContentAnalysisCallback does what SafeContentAnalysisResultCallback used to do.
Depends on D236630
Comment 14•10 days ago
|
||
Add and use makeContentAnalysisResponse to make now native object.
Delegate analyzeContentRequest* to non-mock CA service. Folder
expansion, etc is done and tested with the real service now.
Also improves logging.
Depends on D236631
Comment 15•10 days ago
|
||
Allows us to delay at a granularity finer than one second.
Depends on D236632
Comment 16•10 days ago
|
||
Depends on D236633
Comment 17•10 days ago
|
||
Make browser_clipboard_paste_prompt_content_analysis.js wait for
selectionchange instead of paste, since paste happens immediately but
selectionchange happens after CA allows the contents to change.
Depends on D236634
Comment 18•10 days ago
|
||
Some tests change the expected order of the generated requests, which is not
important. Some also now expect null URI because getting the URI from the
windowGlobalParent is now the fallback/default. Note that DND tests are in a
separate patch because they get broader changes and additional tests.
Depends on D236635
Comment 19•10 days ago
|
||
Adds userActionId/userActionCount DND tests. Has AnalyzeContentRequest*
delegation (like head.js, which it does not use). Switches to using
MakeResponseForTest. Adds bypassForSameTab test. Makes dropPromise actually
wait for the drop.
Depends on D236636
Comment 20•10 days ago
|
||
The URL filter now takes uri as parameter because request may not have it,
since the windowGlobalParent is sometimes used to get it (which we could do
but this is a more thorough test).
Depends on D236637
Comment 21•10 days ago
|
||
nsIContentAnalysisCallback now receives an nsIContentAnalysisResult, not an
nsIContentAnalysisResponse. Also fixes a test typo.
Depends on D236638
Comment 22•10 days ago
|
||
Adds CheckUserActionRequestsCanCancelAndHaveSameUserActionId.
Depends on D236639
Comment 23•8 days ago
|
||
Depends on D236640
Updated•7 days ago
|
Updated•7 days ago
|
Updated•7 days ago
|
Updated•7 days ago
|
Updated•7 days ago
|
Comment 24•7 days ago
|
||
Depends on D236982
Comment 25•3 days ago
|
||
Allows clients to set the user action ID (consistently) but the more useful
effect is that, on return from AnalyzeContentRequests*, this will guarantee
a user action ID is set that can be canceled, or an nserror exception will
be thrown. It does this by adding an entry for the user action into the
user action map before all requests are finalized (notably before request
tokens have been generated), and if it has been removed from the map by
the time requests are final then it was canceled in the interim.
Depends on D237136
Description
•