This is a large number of largely mechanical changes. When possible, the code
was called to directly call `.get()` instead, performing the assertions
manually. In many places PromiseFlatCString() calls were added to dynamically
copy the strings into null terminated buffers if the string was not null
terminated.
Differential Revision: https://phabricator.services.mozilla.com/D286158
Most of these are impossible to be null, but for reasons external to the
component. We shouldn't rely on that.
This is only relevant at shutdown (after observer service shutdown), so it
isn't tested.
Differential Revision: https://phabricator.services.mozilla.com/D272358
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
This fixes some intermittency in the gtests, because now we know the
Acknowledge() call has happened when the test receives the
"dlp-acknowledgement-sent-raw" message and no more calls will be made
to the client.
Differential Revision: https://phabricator.services.mozilla.com/D252575
Records the action returned in a response from the DLP agent, as well
as how long the agent took to return a response (roughly). Note that if
the agent pops up some kind of its own WARN dialog, the timing recorded
will include how long it took for the user to pick an option, so this
may not end up being too valuable.
Also includes a bit more shuffling around of test code so the telemetry
tests can use it.
Differential Revision: https://phabricator.services.mozilla.com/D252574
Records requests made (specifically the analysisType and reason), and
also requests allowed/blocked by the allow/deny URL lists.
Differential Revision: https://phabricator.services.mozilla.com/D252572
Records connection attempts, retries, and failures (with nsresult).
This also adds a GetCreatingClientForTest() method so that tests can
call to create a client, then SpinEventLoopUntil() until
GetCreatingClientForTest() returns false, and after this point the
telemetry should be recorded.
Also moves some test code from TestContentAnalysis.cpp to
TestContentAnalysisUtils.h so it can be used in the new telemetry gtests.
Note that these new gtests start the agent every time it's needed - this
will be cleaned up in part 4.
Differential Revision: https://phabricator.services.mozilla.com/D252571
Records telemetry for the settings used with Content Analysis. Note
that there is no way in C++ to get the default value of a pref that was
set via an Enterprise Policy (with setAndLockPref()) because that method
actually sets the default value, so I had to modify some of my plans for
telemetry to gather. For example, I was going to record interception points
whose Enabled value differed from the default, but instead we record
any interception points whose Enabled value is false. (right now these
are equivalent, but Downloads will be default-off so this will change)
For the allow/deny URL lists I didn't want to record the actual URLs
on those lists, so we really do want to just record whether the values
are different from the default. I hard-coded the default into
ContentAnalysis.cpp, and we can rely on the JS test
test_ca_enterprise_config_with_default_prefs_telemetry() to start failing
if that changes.
Also adds a forceRecreateClientForTest() method to ContentAnalysis that
tests can use to force trying to connect to a client. (since this is when
we record the settings telemetry)
Differential Revision: https://phabricator.services.mozilla.com/D252570
This fixes some intermittency in the gtests, because now we know the
Acknowledge() call has happened when the test receives the
"dlp-acknowledgement-sent-raw" message and no more calls will be made
to the client.
Differential Revision: https://phabricator.services.mozilla.com/D252575
Records the action returned in a response from the DLP agent, as well
as how long the agent took to return a response (roughly). Note that if
the agent pops up some kind of its own WARN dialog, the timing recorded
will include how long it took for the user to pick an option, so this
may not end up being too valuable.
Also includes a bit more shuffling around of test code so the telemetry
tests can use it.
Differential Revision: https://phabricator.services.mozilla.com/D252574
Records requests made (specifically the analysisType and reason), and
also requests allowed/blocked by the allow/deny URL lists.
Differential Revision: https://phabricator.services.mozilla.com/D252572
Records connection attempts, retries, and failures (with nsresult).
This also adds a GetCreatingClientForTest() method so that tests can
call to create a client, then SpinEventLoopUntil() until
GetCreatingClientForTest() returns false, and after this point the
telemetry should be recorded.
Also moves some test code from TestContentAnalysis.cpp to
TestContentAnalysisUtils.h so it can be used in the new telemetry gtests.
Note that these new gtests start the agent every time it's needed - this
will be cleaned up in part 4.
Differential Revision: https://phabricator.services.mozilla.com/D252571
Records telemetry for the settings used with Content Analysis. Note
that there is no way in C++ to get the default value of a pref that was
set via an Enterprise Policy (with setAndLockPref()) because that method
actually sets the default value, so I had to modify some of my plans for
telemetry to gather. For example, I was going to record interception points
whose Enabled value differed from the default, but instead we record
any interception points whose Enabled value is false. (right now these
are equivalent, but Downloads will be default-off so this will change)
For the allow/deny URL lists I didn't want to record the actual URLs
on those lists, so we really do want to just record whether the values
are different from the default. I hard-coded the default into
ContentAnalysis.cpp, and we can rely on the JS test
test_ca_enterprise_config_with_default_prefs_telemetry() to start failing
if that changes.
Also adds a forceRecreateClientForTest() method to ContentAnalysis that
tests can use to force trying to connect to a client. (since this is when
we record the settings telemetry)
Differential Revision: https://phabricator.services.mozilla.com/D252570
This doesn't fix all the problems, but accessing mUserActionMap while
we're in the middle of shutdown is one way to hit an assert, so check
IsShutDown() first.
Differential Revision: https://phabricator.services.mozilla.com/D251879
This adds a nsIContentAnalysisBatchResult interface which includes the
allowedFiles that the new method returns. On the C++ side I made the
existing ContentAnalysisActionResult class implement this interface
as well, but I'm not married to this; we could make a separate class
instead.
In practice I kind of doubt that a user on Windows could actually drag
some data that has some files and some non-files, but the code should
handle this case correctly. The non-files control whether the overall
result of the nsIContentAnalysisBatchResult is eAllow or eBlock, and
the files that are allowed will get put in allowedFiles.
Differential Revision: https://phabricator.services.mozilla.com/D248519
Causes the thread pool to abandon the thread at shutdown (instead
of joining it) so that it won't hold up shutdown if the connection to the
agent is still taking a long time (greater than 2 seconds) processing a
request. See nsThreadPool::ShutdownWithTimeout.
Differential Revision: https://phabricator.services.mozilla.com/D246357
Since the cancel dialog is only associated with one user action ID, and a
folder check now uses different user action IDs for each file, user-cancel
needs to cancel all of the associated user action IDs. (Other cancel
reasons will not want that behavior.)
This patch adds a set of these compound actions to the service. Each entry
is the complete set of user action IDs that constitute the compound action.
The set is held until the compound action issues a response.
Differential Revision: https://phabricator.services.mozilla.com/D246160
Causes the thread pool to abandon the thread at shutdown (instead
of joining it) so that it won't hold up shutdown if the connection to the
agent is still taking a long time (greater than 2 seconds) processing a
request. See nsThreadPool::ShutdownWithTimeout.
Differential Revision: https://phabricator.services.mozilla.com/D246357
Since the cancel dialog is only associated with one user action ID, and a
folder check now uses different user action IDs for each file, user-cancel
needs to cancel all of the associated user action IDs. (Other cancel
reasons will not want that behavior.)
This patch adds a set of these compound actions to the service. Each entry
is the complete set of user action IDs that constitute the compound action.
The set is held until the compound action issues a response.
Differential Revision: https://phabricator.services.mozilla.com/D246160
Since this feature affects old tests, this also adds a test-only field that
tests (especially old tests) can set to indicate that Gecko should skip the
feature.
Since the feature is contained to the agent threads, and those don't exist
for mocked CA, this is only relevant in gtests, where CA is not mocked.
Differential Revision: https://phabricator.services.mozilla.com/D243721
The scans would take time to check with the DLP despite the request having
already been canceled/blocked and the relevant response to that having
already been sent.
Differential Revision: https://phabricator.services.mozilla.com/D243228
Unfortunately this is a little hard to test because we'd have to wait
to see if two ACKs were sent. I did manually verify that this only sends
one ACK in the problem case, and all gtests pass.
Differential Revision: https://phabricator.services.mozilla.com/D244146
Sylvestre Ledru