Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(18)

Issue 2750463006: ChromePrompt Mojo IPC interface (Closed)

Created:
3 years, 7 months ago by ftirelo
Modified:
3 years, 7 months ago
CC:
Joe Mason
Target Ref:
refs/heads/master
Visibility:
Public.

Description

ChromePrompt Mojo IPC interface. The interface will be pulled via DEPS in both Chromium (server side) and Software Reporter (client side) repos. Interface implementation will be landed in the Chromium repo. BUG=690020 R=csharp@chromium.org, grt@chromium.org, joenotcharles@google.com, rockot@chromium.org Committed: https://chromium.googlesource.com/chromium/pipa/+/ef74310fb4d63c737b8a888c2ea72b4e458218ef

Patch Set 1 #

Patch Set 2 : Additional comment #

Total comments: 21

Patch Set 3 : Code review #

Total comments: 24

Patch Set 4 : Code review #

Patch Set 5 : Final comments #

Patch Set 6 : Add not_shown in prompt enum #

Patch Set 7 : Fix typo #

Total comments: 12

Patch Set 8 : Next round of reviews #

Total comments: 2

Patch Set 9 : Observed behaviours as a struct of bools #

Total comments: 1

Patch Set 10 : Standard directory layout #

Unified diffs Side-by-side diffs Delta from patch set Stats (+115 lines, -3 lines) Patch
M .gitignore View 1 chunk +11 lines, -0 lines 0 comments Download
M BUILD.gn View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -0 lines 0 comments Download
M DEPS View 1 2 3 4 5 6 7 3 chunks +7 lines, -1 line 0 comments Download
M GITDEPS View 1 2 3 4 5 6 7 3 chunks +15 lines, -2 lines 0 comments Download
A OWNERS View 1 2 3 4 1 chunk +5 lines, -0 lines 0 comments Download
A chrome_cleaner/public/interfaces/BUILD.gn View 1 2 3 4 5 6 7 8 9 1 chunk +11 lines, -0 lines 0 comments Download
A chrome_cleaner/public/interfaces/chrome_prompt.mojom View 1 2 3 4 5 6 7 8 9 1 chunk +65 lines, -0 lines 0 comments Download

Messages

Total messages: 29 (6 generated)
Fabio Tirelo
3 years, 7 months ago (2017-03-14 15:25:35 UTC) #3
Ken Rockot(use gerrit already)
https://codereview.chromium.org/2750463006/diff/20001/GITDEPS File GITDEPS (right): https://codereview.chromium.org/2750463006/diff/20001/GITDEPS#newcode53 GITDEPS:53: "third_party/Webkit", I would probably try to avoid bringing this ...
3 years, 7 months ago (2017-03-14 18:13:06 UTC) #6
ftirelo
Thanks for the suggestions. Below my responses. https://codereview.chromium.org/2750463006/diff/20001/GITDEPS File GITDEPS (right): https://codereview.chromium.org/2750463006/diff/20001/GITDEPS#newcode53 GITDEPS:53: "third_party/Webkit", On ...
3 years, 7 months ago (2017-03-14 20:26:58 UTC) #7
Ken Rockot(use gerrit already)
On 2017/03/14 at 20:26:58, ftirelo wrote: > Thanks for the suggestions. Below my responses. > ...
3 years, 7 months ago (2017-03-14 20:28:42 UTC) #8
grt (UTC plus 2)
what repo is this in? is there already a top-level OWNERS file? if no, should ...
3 years, 7 months ago (2017-03-15 10:58:13 UTC) #9
ftirelo
We will use this repo to define the IPC interface and utility libs shared by ...
3 years, 7 months ago (2017-03-15 14:27:58 UTC) #10
joenotcharles
https://codereview.chromium.org/2750463006/diff/40001/GITDEPS File GITDEPS (right): https://codereview.chromium.org/2750463006/diff/40001/GITDEPS#newcode53 GITDEPS:53: "third_party/Webkit", Can you also add a comment explaining why ...
3 years, 7 months ago (2017-03-15 16:10:00 UTC) #12
csharp
https://codereview.chromium.org/2750463006/diff/40001/DEPS File DEPS (right): https://codereview.chromium.org/2750463006/diff/40001/DEPS#newcode21 DEPS:21: "chromium_git": "https://chromium.googlesource.com", nit: Please add a blank line after ...
3 years, 7 months ago (2017-03-15 17:22:34 UTC) #13
grt (UTC plus 2)
https://codereview.chromium.org/2750463006/diff/20001/pipa/mojom/chrome_prompt.mojom File pipa/mojom/chrome_prompt.mojom (right): https://codereview.chromium.org/2750463006/diff/20001/pipa/mojom/chrome_prompt.mojom#newcode1 pipa/mojom/chrome_prompt.mojom:1: // Copyright 2017 The Chromium Authors. All Rights Reserved. ...
3 years, 7 months ago (2017-03-16 09:02:11 UTC) #14
ftirelo
PTAL https://codereview.chromium.org/2750463006/diff/20001/GITDEPS File GITDEPS (right): https://codereview.chromium.org/2750463006/diff/20001/GITDEPS#newcode53 GITDEPS:53: "third_party/Webkit", On 2017/03/14 20:26:58, ftirelo wrote: > On ...
3 years, 7 months ago (2017-03-21 16:54:28 UTC) #15
grt (UTC plus 2)
i'm getting down to the nitty-nits here. looks pretty good. https://codereview.chromium.org/2750463006/diff/120001/DEPS File DEPS (right): https://codereview.chromium.org/2750463006/diff/120001/DEPS#newcode1 ...
3 years, 7 months ago (2017-03-22 13:48:14 UTC) #16
ftirelo
PTAnL The only point I'd like to discuss a bit more is related to bitfields ...
3 years, 7 months ago (2017-03-22 14:25:37 UTC) #17
Ken Rockot(use gerrit already)
https://codereview.chromium.org/2750463006/diff/70008/chrome_cleaner/mojom/chrome_prompt.mojom File chrome_cleaner/mojom/chrome_prompt.mojom (right): https://codereview.chromium.org/2750463006/diff/70008/chrome_cleaner/mojom/chrome_prompt.mojom#newcode8 chrome_cleaner/mojom/chrome_prompt.mojom:8: enum ObservedBehaviour { Yeah, please don't use enums for ...
3 years, 7 months ago (2017-03-22 14:41:45 UTC) #18
joenotcharles
https://codereview.chromium.org/2750463006/diff/120001/chrome_cleaner/mojom/chrome_prompt.mojom File chrome_cleaner/mojom/chrome_prompt.mojom (right): https://codereview.chromium.org/2750463006/diff/120001/chrome_cleaner/mojom/chrome_prompt.mojom#newcode25 chrome_cleaner/mojom/chrome_prompt.mojom:25: array<ObservedBehaviour> observed_behaviours; On 2017/03/22 14:25:36, ftirelo wrote: > On ...
3 years, 7 months ago (2017-03-22 14:45:04 UTC) #19
Ken Rockot(use gerrit already)
On 2017/03/22 at 14:45:04, joenotcharles wrote: > https://codereview.chromium.org/2750463006/diff/120001/chrome_cleaner/mojom/chrome_prompt.mojom > File chrome_cleaner/mojom/chrome_prompt.mojom (right): > > https://codereview.chromium.org/2750463006/diff/120001/chrome_cleaner/mojom/chrome_prompt.mojom#newcode25 ...
3 years, 7 months ago (2017-03-22 15:02:18 UTC) #20
grt (UTC plus 2)
lgtm from me. i'll bow out of the array vs bitfield discussion since i don't ...
3 years, 7 months ago (2017-03-22 15:23:50 UTC) #21
ftirelo
I ended up going with Ken's suggestion of a struct of bools, since it's easier ...
3 years, 7 months ago (2017-03-22 17:37:37 UTC) #22
joenotcharles
lgtm
3 years, 7 months ago (2017-03-22 17:43:38 UTC) #23
csharp
lgtm
3 years, 7 months ago (2017-03-22 20:08:16 UTC) #24
grt (UTC plus 2)
still lgtm
3 years, 7 months ago (2017-03-23 09:29:00 UTC) #25
Ken Rockot(use gerrit already)
lgtm https://codereview.chromium.org/2750463006/diff/150001/chrome_cleaner/mojom/BUILD.gn File chrome_cleaner/mojom/BUILD.gn (right): https://codereview.chromium.org/2750463006/diff/150001/chrome_cleaner/mojom/BUILD.gn#newcode1 chrome_cleaner/mojom/BUILD.gn:1: # Copyright 2017 The Chromium Authors. All Rights ...
3 years, 7 months ago (2017-03-23 17:41:26 UTC) #26
Fabio Tirelo
Thanks for the reviews! On 2017/03/23 17:41:26, Ken Rockot wrote: > lgtm > > https://codereview.chromium.org/2750463006/diff/150001/chrome_cleaner/mojom/BUILD.gn ...
3 years, 7 months ago (2017-03-23 17:56:23 UTC) #27
ftirelo
3 years, 7 months ago (2017-03-23 17:56:50 UTC) #29
Message was sent while issue was closed.
Committed patchset #10 (id:170001) manually as
ef74310fb4d63c737b8a888c2ea72b4e458218ef (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698