components/dom_distiller/content/common/distiller_javascript_service.mojom - Issue 1265843005: Refactor feedback to use native JS object

Side by Side Diff

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Side by Side Diff: components/dom_distiller/content/common/distiller_javascript_service.mojom

Issue 1265843005: Refactor feedback to use native JS object (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@js-mojo-combine

Patch Set: Param order Created 5 years, 3 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« components/dom_distiller/content/browser/distiller_javascript_service_impl.cc ('K') | « components/dom_distiller/content/browser/dom_distiller_viewer_source.cc ('k') | components/dom_distiller/content/renderer/distiller_native_javascript.h » ('j') | components/dom_distiller/content/renderer/distiller_native_javascript.h » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 // Copyright 2015 The Chromium Authors. All rights reserved.	1 // Copyright 2015 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be	2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 module dom_distiller;	5 module dom_distiller;

6	6

7 // This service is implemented by the browser process and is used by the	7 // This service is implemented by the browser process and is used by the

8 // renderer when a distiller JavaScript function is called.	8 // renderer when a distiller JavaScript function is called.

9 interface DistillerJavaScriptService {	9 interface DistillerJavaScriptService {

10 HandleDistillerEchoCall(string message);	10 HandleDistillerEchoCall(string message);

	11 HandleDistillerFeedbackCall(bool good);
	nyquist 2015/09/10 19:04:13 Should we have comments for these two methods? Or Should we have comments for these two methods? Or do you not think it's helpful? mdjones 2015/09/11 16:50:13 I tried to make the function names self-documentin Show quoted text On 2015/09/10 19:04:13, nyquist wrote: > Should we have comments for these two methods? Or do you not think it's helpful? I tried to make the function names self-documenting, but I added comments for clarity. nyquist 2015/09/11 17:14:22 Oh. Right. Yeah, I think the method names are expl Show quoted text On 2015/09/11 16:50:13, mdjones wrote: > On 2015/09/10 19:04:13, nyquist wrote: > > Should we have comments for these two methods? Or do you not think it's > helpful? > > I tried to make the function names self-documenting, but I added comments for > clarity. Oh. Right. Yeah, I think the method names are explanatory by themselves. The new comments aren't really more helpful than the method name. I was thinking more: "Triggers sending user feedback using the system feedback mechanism." mdjones 2015/09/11 18:25:54 I would expect that kind of comment to be written Show quoted text On 2015/09/11 17:14:22, nyquist wrote: > On 2015/09/11 16:50:13, mdjones wrote: > > On 2015/09/10 19:04:13, nyquist wrote: > > > Should we have comments for these two methods? Or do you not think it's > > helpful? > > > > I tried to make the function names self-documenting, but I added comments for > > clarity. > > Oh. Right. Yeah, I think the method names are explanatory by themselves. The new > comments aren't really more helpful than the method name. > > I was thinking more: > "Triggers sending user feedback using the system feedback mechanism." I would expect that kind of comment to be written for the implementation rather than the interface; I added comments in the .h file for this implementation.
11 };	12 };

OLD	NEW