ChromePrompt Mojo IPC interface

pipa/mojom/chrome_prompt.mojom

+// Copyright 2017 The Chromium Authors. All Rights Reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+module pipa.mojom;

[joenotcharles, 2017/03/15 16:10:00]

I wonder if we should use the name "chrome_cleaner.mojom" for the module. It's
more obvious what it means from in Chromium code, and from Foil code it means
the mojom file is in the same namespace as our protobufs, which seems
reasonable.

[grt (UTC plus 2), 2017/03/16 09:02:11]

$0.02: "pipa" means nothing to me, and is counter to the Google C++ style for
naming: https://google.github.io/styleguide/cppguide.html#General_Naming_Rules

[ftirelo, 2017/03/21 16:54:28]

Done.

[ftirelo, 2017/03/21 16:54:28]

Moved the code to the chrome_cleaner namespace and defined a top-level dir for
chrome_cleaner. Kept pipa/ for repo config code.

+
+enum ObservedBehavior {
+  UNSPECIFIED = 0,
+  ADS_INJECTOR = 1,
+  SETTINGS_HIJACKER = 2,
+  BROWSER_HIJACKER = 3,
+};
+
+// Information about removable Unwanted Software matched by the Software
+// Reporter to be shown in the Chrome prompt.
+struct UwS {
+  // The name of this unwanted software.
+  string name;

[joenotcharles, 2017/03/15 16:10:00]

Should we include the ID here too?

[ftirelo, 2017/03/21 16:54:28]

Done.

+
+  // List of behaviors to be presented to the user.
+  array<ObservedBehavior> observed_behaviours;
+
+  // List of fully-qualified paths of the files that will be deleted by the
+  // Chrome Cleanup Tool for this unwanted software.
+  array<string> files_to_delete;
+};
+
+// A request sent by the Software Reporter Tool to Chrome requiring the user
+// to be prompted.
+struct PromptUserRequest {
+  // The list of removable UwS detected by the Software Reporter Tool.
+  array<UwS> removable_uws_found;
+
+  // If true, the Chrome Cleanup Tool will need to run elevated.
+  bool elevation_required;
+};
+
+// The response sent by Chrome to the Software Reporter Tool with the user
+// acceptance result.
+struct PromptUserResponse {
+  // If true, the user accepted the prompt.
+  bool prompt_accepted;
+
+  // If true, the user agrees with sending detailed logs information to Google.

[csharp, 2017/03/15 17:22:34]

nit: Worth also mentioning crashes? Maybe the variable name would also need to
change

[ftirelo, 2017/03/21 16:54:28]

We will not send crash reports for users in SBER2, only for users with metrics
reporting enabled. For crash reporting, we will still rely on the command line
switch sent by Chrome.

+  bool logs_uploading_enabled;
+};
+
+// Service provided by Chrome to prompt the user to run the Chrome Cleanup Tool
+// if unwanted software is detected in the system. This service is used by the
+// Software Reporter Tool so that all user interaction is provided by Chrome.
+interface ChromePrompt {
+  PromptUser(PromptUserRequest request) => (PromptUserResponse response);

[joenotcharles, 2017/03/15 16:10:00]

I think the Request / Response structs obscure what's happening here because
they're an extra level of indirection. Under the hood PromptUser is serialized
into a message containing the name and a bundle of info that goes with that
message, and "=>" tells us we can expect a response with another bundle of info.
Having that info tagged "PromptUserRequest" because it's part of a struct
doesn't add anything because we already know this is a PromptUser message, and
that the left side is the request and the right side is the response.

And having the interface "PromptUser(removable_uws_found, elevation_required) =>
(prompt_accepted, logs_uploading_enabled)" makes it easier to tell what's
changed if we look at different versions of the .mojom file: there's no chance
we'll look at just the "PromptUser" message and think the parameters haven't
changed because we didn't look at the structs, and if we see a diff between two
structs we don't need to search for which message uses them.

[ftirelo, 2017/03/21 16:54:28]

Done.

+};