Create Settings Signals Collector Boilerplate This will be used to collect settings signals (currently only registry presence/value on windows and plist presence/value on mac). It's part of an additional endpoint verification signal in go/ev-additional-signals-prd [chromium/src : main]
Zonghan Xu (Gerrit)
Attention is currently required from: Zonghan Xu.
Zonghan Xu uploaded patch set #82 to this change.
Create Settings Signals Collector Boilerplate
This will be used to collect settings signals (currently only registry presence/value on windows and plist presence/value on mac). It's part of an additional endpoint verification signal in go/ev-additional-signals-prd
This collector runs on browser process.
Fixed: b:237526915
Change-Id: I6b1a2490e573a0854a24458a046b94d8aba850e0
---
M chrome/browser/enterprise/signals/signals_aggregator_factory.cc
M chrome/browser/enterprise/signals/signals_aggregator_factory.h
M components/device_signals/core/browser/BUILD.gn
A components/device_signals/core/browser/mock_settings_client.cc
A components/device_signals/core/browser/mock_settings_client.h
A components/device_signals/core/browser/settings_client.h
A components/device_signals/core/browser/settings_signals_collector.cc
A components/device_signals/core/browser/settings_signals_collector.h
A components/device_signals/core/browser/settings_signals_collector_unittest.cc
M components/device_signals/core/browser/signals_types.cc
M components/device_signals/core/browser/signals_types.h
11 files changed, 406 insertions(+), 12 deletions(-)
To view, visit change 3884016. To unsubscribe, or for help writing mail filters, visit settings.
Zonghan Xu (Gerrit)
Attention is currently required from: Sebastien Lalancette.
8 comments:
Commit Message:
Create Settings Signals Collector Boilerplate
Keep this on one line then move the rest to a different paragraph. […]
Done
File chrome/browser/enterprise/signals/signals_aggregator_factory.cc:
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
std::unique_ptr<device_signals::SettingsClient> settings_client =
device_signals::BuildSettingsClient();
collectors.push_back(
std::make_unique<device_signals::SettingsSignalsCollector>(
std::move(settings_client)));
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)If we don't yet have valid clients, maybe we can just not put the code here yet.
I commented out the lines (75-77) where we create the new collectors
This should be easier for Hamda to find where to create her Plist client
File chrome/browser/enterprise/signals/signals_aggregator_factory.cc:
namespace device_signals {
std::unique_ptr<SettingsClient> BuildSettingsClient() {
// return nullptr for unsupported platforms
return std::make_unique<SettingsClient>();
}
} // namespace device_signalsNot sure what is the purpose of this function, and why it is exposed in the header? Also, you cannot […]
Done
File components/device_signals/core/browser/settings_client.h:
`#include <vector>`
Done
Patch Set #81, Line 8:
"base/callback.h"Switch to `base/callback_forward.h` in the header file, and use `base/callback.h` in the .cc file.
There is no .cc file and switching to base/callback_forward.h causes error
^
In file included from ../../components/device_signals/core/browser/mock_settings_client.cc:5:
In file included from ../../components/device_signals/core/browser/mock_settings_client.h:9:
In file included from ../../testing/gmock/include/gmock/gmock.h:13:
In file included from ../../third_party/googletest/src/googlemock/include/gmock/gmock.h:58:
In file included from ../../third_party/googletest/src/googlemock/include/gmock/gmock-function-mocker.h:43:
In file included from ../../third_party/googletest/src/googlemock/include/gmock/gmock-spec-builders.h:76:
../../third_party/googletest/src/googlemock/include/gmock/gmock-matchers.h:637:26: error: no matching function for call to 'get'
const Value& value = std::get<N - 1>(values);
File components/device_signals/core/browser/settings_signals_collector.h:
Patch Set #81, Line 50:
// Instance used to retrieve a pointer to a SettingsClient instance.I know this was copied from `file_sytem_signals_collector`, but this comment doesn't really apply he […]
Done
File components/device_signals/core/browser/settings_signals_collector.cc:
#include "components/device_signals/core/common/common_types.h"
#include "components/device_signals/core/common/mojom/system_signals.mojom.h"Not needed?
Done
File components/device_signals/core/browser/settings_signals_collector_unittest.cc:
Patch Set #81, Line 138:
// EXPECT_CALL(*settings_client_, GetSettings()).Times(1);Remove?
Done
To view, visit change 3884016. To unsubscribe, or for help writing mail filters, visit settings.
Sebastien Lalancette (Gerrit)
Attention is currently required from: Zonghan Xu.
3 comments:
Commit Message:
Create Settings Signals Collector Boilerplate
Done
I guess we also need a blank space between the title and the next paragraph.. notice how big the title of this CL is.
File chrome/browser/enterprise/signals/signals_aggregator_factory.cc:
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
std::unique_ptr<device_signals::SettingsClient> settings_client =
device_signals::BuildSettingsClient();
collectors.push_back(
std::make_unique<device_signals::SettingsSignalsCollector>(
std::move(settings_client)));
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
I commented out the lines (75-77) where we create the new collectors […]
Please remove the factory code will actually make the DCHECK occur in SettingsSignalsCollector. Also, we tend not to commit commented-out code.
File components/device_signals/core/browser/settings_client.h:
Patch Set #81, Line 8:
"base/callback.h"There is no .cc file and switching to base/callback_forward.h causes error […]
That's because you need to add `#include "base/callback.h"` to `mock_settings.client.cc`.
To view, visit change 3884016. To unsubscribe, or for help writing mail filters, visit settings.