ChromePrompt Mojo IPC interface

pipa/mojom/chrome_prompt.mojom

+// Copyright 2017 The Chromium Authors. All Rights Reserved.

[grt (UTC plus 2), 2017/03/15 10:58:13]

why not the full license header?

[ftirelo, 2017/03/15 14:27:58]

No reason. Copied the header from the internal reporter repo, which only
contains the first line. Thanks for noticing that.

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

Just to confirm: this new repo you're creating should contain open-source code
under the same license as Chromium?

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

Yes. Same license as Chromium. Our original goal was for this code to reside in
the Chromium repo, but rolling out new versions in the reporter repo would not
be feasible, which led us to the new repo.

+
+module pipa.mojom;
+
+enum ObservedBehavior {
+  UNSPECIFIED = 0,
+  ADS_INJECTOR = 1,
+  SETTINGS_HIJACKER= 2,

[grt (UTC plus 2), 2017/03/15 10:58:13]

nit: missing whitespace before '='

[ftirelo, 2017/03/15 14:27:58]

I think I'm relying too much on git cl format :)

+  BROWSER_HIJACKER = 3,
+};
+
+// Information about removable Unwanted Software matched by the Software
+// Reporter to be shown in the Chrome prompt.
+struct UwS {
+  string name;

[grt (UTC plus 2), 2017/03/15 10:58:13]

nit: document this field

[ftirelo, 2017/03/15 14:27:58]

Done.

+  // List of behaviors to be presented to the user.

[grt (UTC plus 2), 2017/03/15 10:58:13]

nit: in c++ style, we have a blank line separating the previous member from the
next member's comment. does mojo style deviate from this?

[ftirelo, 2017/03/15 14:27:58]

A quick search didn't show any style-guide for Mojo. So, I'll try to be as close
to C++ style as possible.

+  array<ObservedBehavior> observed_behaviours;
+  // List of files that will be deleted by the Chrome Cleanup Tool for this

[grt (UTC plus 2), 2017/03/15 10:58:13]

can you be more specific? is it a list of fully-qualified path names of files?

[ftirelo, 2017/03/15 14:27:58]

Done.

+  // 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 {
+  array<UwS> removable_uws_found;
+  // If true, the Chrome Cleanup Tool will need to run elevated.
+  bool elevation_required;
+};
+
+// The request sent by Chrome to the Software Reporter Tool with the user

[grt (UTC plus 2), 2017/03/15 10:58:13]

request -> response?

[ftirelo, 2017/03/15 14:27:58]

Done.

+// 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.
+  bool logs_uploading_enabled;
+};
+
+// Service invoked by the Software Reporter Tool to request Chrome to prompt

[grt (UTC plus 2), 2017/03/15 10:58:13]

nit: document the service from the other side, since it's implemented by Chrome
rather than by the SRT. maybe:

// A service provided by Chrome to process request to prompt the user to remove
unwanted software. This service is used by the Software Reporter Tool so that
the removal user experience is provided by Chrome.

or something like that

[ftirelo, 2017/03/15 14:27:58]

Done.

+// user.
+interface ChromePrompt {
+  PromptUser(PromptUserRequest request) => (PromptUserResponse response);

[Ken Rockot(use gerrit already), 2017/03/14 18:13:06]

nit: If these Request and Response types are only used for PromptUser, and you
don't expect the number of fields to grow too large, it's probably cleaner to
just inline the fields as message arguments?

  PromptUser(array<UwS> removable_uws, bool elevation_required)
      => (bool accepted, bool upload_logs);

[ftirelo, 2017/03/14 20:26:58]

The main reason is that we expect this to grow with time, specially in v2. But
we are also concerned about possible divergences between the Chrome and the
Reporter binaries, when we update the list, since they don't follow the same
release cycle. What would happen if, for example, the Reporter sees:
  PromptUser(removable_uws, elevation_required)
but Chrome sees:
  PromptUser(removable_uws, elevation_required, another_field) ?

+};