Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2822433003:
Cleanup Tool WebUI: Add functionality to the Scan button (Closed)
|
Created:
3 years, 8 months ago by proberge Modified:
3 years, 8 months ago CC:
chromium-reviews, alito+watch_chromium.org, ftirelo+watch_chromium.org, arv+watch_chromium.org, csharp+watch_chromium.org, joenotcharles+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionCleanup Tool WebUI: Add functionality to the Scan button
This will allow users to manually launch the Chrome Cleanup Tool in
scanning mode to detect Unwanted Software on their system.
At the moment, the scan does not actually happen and dummy data is
returned.
BUG=690020
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2822433003
Cr-Commit-Position: refs/heads/master@{#467065}
Committed: https://chromium.googlesource.com/chromium/src/+/81200e069bcced52999b92c3a060ef4c12b8d8b8
Patch Set 1 #Patch Set 2 : Remove component updating logic from cleanup_action_handler #
Total comments: 20
Patch Set 3 : Address review comments on #2 #
Total comments: 2
Patch Set 4 : Add comment to eventually localize strings #
Total comments: 8
Patch Set 5 : Add WeakPtrFactory #
Messages
Total messages: 26 (10 generated)
Description was changed from ========== Cleanup Tool WebUI: Add functionality to the Scan button This will allow users to manually launch the Chrome Cleanup Tool in scanning mode to detect Unwanted Software on their system. At the moment, the scan does not actually happen and dummy data is returned. BUG=690020 ========== to ========== Cleanup Tool WebUI: Add functionality to the Scan button This will allow users to manually launch the Chrome Cleanup Tool in scanning mode to detect Unwanted Software on their system. At the moment, the scan does not actually happen and dummy data is returned. BUG=690020 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Cleanup Tool WebUI: Add functionality to the Scan button This will allow users to manually launch the Chrome Cleanup Tool in scanning mode to detect Unwanted Software on their system. At the moment, the scan does not actually happen and dummy data is returned. BUG=690020 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Cleanup Tool WebUI: Add functionality to the Scan button This will allow users to manually launch the Chrome Cleanup Tool in scanning mode to detect Unwanted Software on their system. At the moment, the scan does not actually happen and dummy data is returned. BUG=690020 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
proberge@chromium.org changed reviewers: + ftirelo@chromium.org
https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/componen... File chrome/browser/component_updater/sw_reporter_installer_win.cc (right): https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/componen... chrome/browser/component_updater/sw_reporter_installer_win.cc:505: // TODO(proberge): Implement me. Nit: Please describe this as a stub and what is intended to be implemented here. https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/componen... File chrome/browser/component_updater/sw_reporter_installer_win.h (right): https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/componen... chrome/browser/component_updater/sw_reporter_installer_win.h:95: // TODO(proberge): Replace the Closure with a typed callback. What is the intended type for this callback? Would it related to the Mojo interfaces at https://cs.chromium.org/chromium/src/components/chrome_cleaner/public/interfa... ? https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... File chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc (right): https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:15: constexpr char kComponentId[] = "gkmgaooipdjhmangpemjhigmamcehddo"; Please declare this in chrome/browser/component_updater/sw_reporter_installer_win.h and use it here. Notice that we have a reference to this string at sw_reporter_installer_win.cc, so I think they should stay close to each other. https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:21: component_ids = cus->GetComponentIDs(); Please merge this into the previous line. https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:68: if (!IsCleanupComponentRegistered()) { What happens if the reporter is not available because it's being downloaded for the first time? https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:70: // TODO(proberge): Localize strings once they are finalized. Please move these strings to constants at the beginning of this file on the anonymous namespace, so it will be easier to find them all afterwards. Same for the next method. https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:74: "The Cleanup Tool is not supported."); Nit: The Cleanup Tool -> Chrome Cleanup https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:95: "2 potentially harmful programs detected"); Please add a comment that these are being used as placeholders while we don't have the reporter fully connected to Chrome. https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... File chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.h (right): https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.h:25: void HandleStartScan(const base::ListValue* args); Please add a description to these two methods. https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.h:26: void ReportScanResults(std::string callback_id); const std::string&
https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/componen... File chrome/browser/component_updater/sw_reporter_installer_win.cc (right): https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/componen... chrome/browser/component_updater/sw_reporter_installer_win.cc:505: // TODO(proberge): Implement me. On 2017/04/13 17:10:43, ftirelo wrote: > Nit: Please describe this as a stub and what is intended to be implemented here. Done. https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/componen... File chrome/browser/component_updater/sw_reporter_installer_win.h (right): https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/componen... chrome/browser/component_updater/sw_reporter_installer_win.h:95: // TODO(proberge): Replace the Closure with a typed callback. On 2017/04/13 17:10:43, ftirelo wrote: > What is the intended type for this callback? Would it related to the Mojo > interfaces at > https://cs.chromium.org/chromium/src/components/chrome_cleaner/public/interfa... > ? With the current design we won't be needing all that information. At the moment I'm thinking of bool scanSuccessful vector<string> uws_names https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... File chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc (right): https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:15: constexpr char kComponentId[] = "gkmgaooipdjhmangpemjhigmamcehddo"; On 2017/04/13 17:10:43, ftirelo wrote: > Please declare this in > chrome/browser/component_updater/sw_reporter_installer_win.h and use it here. > Notice that we have a reference to this string at sw_reporter_installer_win.cc, > so I think they should stay close to each other. Done. https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:21: component_ids = cus->GetComponentIDs(); On 2017/04/13 17:10:43, ftirelo wrote: > Please merge this into the previous line. Done. https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:68: if (!IsCleanupComponentRegistered()) { On 2017/04/13 17:10:43, ftirelo wrote: > What happens if the reporter is not available because it's being downloaded for > the first time? RegisterUserInitiatedSwReporterScan is responsible for responding to the callback. It should try to update the component, then run it. If it fails, it should return scanSuccess=false; in that case ReportScanResults would behave differently. https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:70: // TODO(proberge): Localize strings once they are finalized. On 2017/04/13 17:10:43, ftirelo wrote: > Please move these strings to constants at the beginning of this file on the > anonymous namespace, so it will be easier to find them all afterwards. Same for > the next method. Done. https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:74: "The Cleanup Tool is not supported."); On 2017/04/13 17:10:43, ftirelo wrote: > Nit: The Cleanup Tool -> Chrome Cleanup Done. https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:95: "2 potentially harmful programs detected"); On 2017/04/13 17:10:43, ftirelo wrote: > Please add a comment that these are being used as placeholders while we don't > have the reporter fully connected to Chrome. On top of the TODO at the top of this method? https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... File chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.h (right): https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.h:25: void HandleStartScan(const base::ListValue* args); On 2017/04/13 17:10:43, ftirelo wrote: > Please add a description to these two methods. The first one doesn't (implicit description: handles the "startScan" call from Javascript"). Added a description for the second. https://codereview.chromium.org/2822433003/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.h:26: void ReportScanResults(std::string callback_id); On 2017/04/13 17:10:43, ftirelo wrote: > const std::string& Done.
LGTM with a small nit. https://codereview.chromium.org/2822433003/diff/40001/chrome/browser/ui/webui... File chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc (right): https://codereview.chromium.org/2822433003/diff/40001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:25: constexpr char kDetectionOkText[] = "No problems detected"; Nit: please add a comment saying that these are placeholders that will be replaced once we connect to the reporter runner.
proberge@chromium.org changed reviewers: + tommycli@chromium.org
https://codereview.chromium.org/2822433003/diff/40001/chrome/browser/ui/webui... File chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc (right): https://codereview.chromium.org/2822433003/diff/40001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:25: constexpr char kDetectionOkText[] = "No problems detected"; On 2017/04/18 13:50:59, ftirelo wrote: > Nit: please add a comment saying that these are placeholders that will be > replaced once we connect to the reporter runner. I don't think the strings will come from the reporter so omitting that part. Otherwise done.
https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... File chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc (right): https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:30: constexpr char kUnsupportedHelpText[] = "Please try reinstalling Chrome"; Passing localized text back via the C++ handler seems... odd. The usual pattern we use is to pass back an integer code (an enum), and then generate the appropriate localized string on the JavaScript side. Are the strings really not "finalized"? If they appear in any mocks, I would just add the strings to the grd files and update later if the mocks change... https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:97: AllowJavascript(); I assume this scan can take a long time, where JavaScript may become disallowed. In that case, our suggested pattern is to use a weak pointer factory and cancel the callback when javascript is disallowed. See example usage of callback_weak_ptr_factory_. https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/settings/profile...
https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... File chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc (right): https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:30: constexpr char kUnsupportedHelpText[] = "Please try reinstalling Chrome"; On 2017/04/21 18:24:21, tommycli wrote: > Passing localized text back via the C++ handler seems... odd. > > The usual pattern we use is to pass back an integer code (an enum), and then > generate the appropriate localized string on the JavaScript side. > > Are the strings really not "finalized"? If they appear in any mocks, I would > just add the strings to the grd files and update later if the mocks change... To elaborate... we usually like to design our internal APIs with similar quality standards as external APIs. If we pass status as localized text instead of fixed codes (in an enum), it becomes impossible to programatically query what happened -- all we can do is display the text.
https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... File chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc (right): https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:30: constexpr char kUnsupportedHelpText[] = "Please try reinstalling Chrome"; On 2017/04/21 18:24:21, tommycli wrote: > Passing localized text back via the C++ handler seems... odd. > > The usual pattern we use is to pass back an integer code (an enum), and then > generate the appropriate localized string on the JavaScript side. > > Are the strings really not "finalized"? If they appear in any mocks, I would > just add the strings to the grd files and update later if the mocks change... From the mocks I was implementing, two of the strings (kDetectionUwSText and kDetectionTimeText) required plurality support that did not seem available from the WebUI's JS. It seems like I have to localize them in C++. For example: <message name="IDS_CLEANUP_TOOL_DAYS_LAST_RUN" desc="A message on the Chrome Cleanup Tool page, shown to inform when the most recent scan for Unwanted Software was done. [ICU Syntax]"> {DAYS_COUNT, plural, =0 {Last scanned today} =1 {Last scanned yesterday} other {Last scanned # days ago}} </message> Is there an elegant way to resolve this? I might be able to push back on the string to get some that are more easily localizable. > Are the strings really not "finalized"? If they appear in any mocks, I would just add the strings to the grd files and update later if the mocks change... Is there a way to add strings to the grd files without staging them for translation? I'd like to avoid wasting translation resources. https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:97: AllowJavascript(); On 2017/04/21 18:24:21, tommycli wrote: > I assume this scan can take a long time, where JavaScript may become disallowed. > > In that case, our suggested pattern is to use a weak pointer factory and cancel > the callback when javascript is disallowed. See example usage of > callback_weak_ptr_factory_. > > https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/settings/profile... Makes a lot of sense. Thanks for the example usage!
https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... File chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc (right): https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:30: constexpr char kUnsupportedHelpText[] = "Please try reinstalling Chrome"; On 2017/04/21 21:04:32, proberge wrote: > On 2017/04/21 18:24:21, tommycli wrote: > > Passing localized text back via the C++ handler seems... odd. > > > > The usual pattern we use is to pass back an integer code (an enum), and then > > generate the appropriate localized string on the JavaScript side. > > > > Are the strings really not "finalized"? If they appear in any mocks, I would > > just add the strings to the grd files and update later if the mocks change... > > From the mocks I was implementing, two of the strings (kDetectionUwSText and > kDetectionTimeText) required plurality support that did not seem available from > the WebUI's JS. It seems like I have to localize them in C++. > > For example: > <message name="IDS_CLEANUP_TOOL_DAYS_LAST_RUN" desc="A message on the Chrome > Cleanup Tool page, shown to inform when the most recent scan for Unwanted > Software was done. [ICU Syntax]"> > {DAYS_COUNT, plural, > =0 {Last scanned today} > =1 {Last scanned yesterday} > other {Last scanned # days ago}} > </message> > > Is there an elegant way to resolve this? I might be able to push back on the > string to get some that are more easily localizable. > > > Are the strings really not "finalized"? If they appear in any mocks, I would > just add the strings to the grd files and update later if the mocks change... > > Is there a way to add strings to the grd files without staging them for > translation? I'd like to avoid wasting translation resources. It appears there's translation_expectations.pyl as documented here: https://www.chromium.org/developers/design-documents/ui-localization However, I would probably just strive to get these strings finalized in time for branch cut. I don't believe they are translated before that time anyways. As for plurality, you might be stuck passing the strings from C++ in that case. I see the clear browsing data dialog also passes the strings from C++ in that case: https://cs.chromium.org/chromium/src/chrome/browser/browsing_data/browsing_da... Hope that helps... I think our general pattern is -- avoid sending the localized strings from C++ if possible, but in this case maybe it's unavoidable.
alito@chromium.org changed reviewers: + alito@chromium.org
https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... File chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc (right): https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:30: constexpr char kUnsupportedHelpText[] = "Please try reinstalling Chrome"; On 2017/04/21 21:04:32, proberge wrote: > On 2017/04/21 18:24:21, tommycli wrote: > > Passing localized text back via the C++ handler seems... odd. > > > > The usual pattern we use is to pass back an integer code (an enum), and then > > generate the appropriate localized string on the JavaScript side. > > > > Are the strings really not "finalized"? If they appear in any mocks, I would > > just add the strings to the grd files and update later if the mocks change... > > From the mocks I was implementing, two of the strings (kDetectionUwSText and > kDetectionTimeText) required plurality support that did not seem available from > the WebUI's JS. It seems like I have to localize them in C++. > > For example: > <message name="IDS_CLEANUP_TOOL_DAYS_LAST_RUN" desc="A message on the Chrome > Cleanup Tool page, shown to inform when the most recent scan for Unwanted > Software was done. [ICU Syntax]"> > {DAYS_COUNT, plural, > =0 {Last scanned today} > =1 {Last scanned yesterday} > other {Last scanned # days ago}} > </message> > > Is there an elegant way to resolve this? I might be able to push back on the > string to get some that are more easily localizable. > > > Are the strings really not "finalized"? If they appear in any mocks, I would > just add the strings to the grd files and update later if the mocks change... > > Is there a way to add strings to the grd files without staging them for > translation? I'd like to avoid wasting translation resources. Re not translating strings: I've noticed that some messages in grd files have the attribute "translateable" set to false. I can't find any official documentation about it though...
https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... File chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc (right): https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:30: constexpr char kUnsupportedHelpText[] = "Please try reinstalling Chrome"; On 2017/04/21 22:12:37, alito wrote: > On 2017/04/21 21:04:32, proberge wrote: > > On 2017/04/21 18:24:21, tommycli wrote: > > > Passing localized text back via the C++ handler seems... odd. > > > > > > The usual pattern we use is to pass back an integer code (an enum), and then > > > generate the appropriate localized string on the JavaScript side. > > > > > > Are the strings really not "finalized"? If they appear in any mocks, I would > > > just add the strings to the grd files and update later if the mocks > change... > > > > From the mocks I was implementing, two of the strings (kDetectionUwSText and > > kDetectionTimeText) required plurality support that did not seem available > from > > the WebUI's JS. It seems like I have to localize them in C++. > > > > For example: > > <message name="IDS_CLEANUP_TOOL_DAYS_LAST_RUN" desc="A message on the Chrome > > Cleanup Tool page, shown to inform when the most recent scan for Unwanted > > Software was done. [ICU Syntax]"> > > {DAYS_COUNT, plural, > > =0 {Last scanned today} > > =1 {Last scanned yesterday} > > other {Last scanned # days ago}} > > </message> > > > > Is there an elegant way to resolve this? I might be able to push back on the > > string to get some that are more easily localizable. > > > > > Are the strings really not "finalized"? If they appear in any mocks, I would > > just add the strings to the grd files and update later if the mocks change... > > > > Is there a way to add strings to the grd files without staging them for > > translation? I'd like to avoid wasting translation resources. > > Re not translating strings: I've noticed that some messages in grd files have > the attribute "translateable" set to false. I can't find any official > documentation about it though... Looks like we only have until May 11th to get strings in for M60. I'll start a fire under our UX person to provide better strings before then. It's unlikely any of the current ones will be used, so I'll skip on translating them.
On 2017/04/25 17:39:32, proberge wrote: > https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... > File chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc (right): > > https://codereview.chromium.org/2822433003/diff/60001/chrome/browser/ui/webui... > chrome/browser/ui/webui/cleanup_tool/cleanup_action_handler.cc:30: constexpr > char kUnsupportedHelpText[] = "Please try reinstalling Chrome"; > On 2017/04/21 22:12:37, alito wrote: > > On 2017/04/21 21:04:32, proberge wrote: > > > On 2017/04/21 18:24:21, tommycli wrote: > > > > Passing localized text back via the C++ handler seems... odd. > > > > > > > > The usual pattern we use is to pass back an integer code (an enum), and > then > > > > generate the appropriate localized string on the JavaScript side. > > > > > > > > Are the strings really not "finalized"? If they appear in any mocks, I > would > > > > just add the strings to the grd files and update later if the mocks > > change... > > > > > > From the mocks I was implementing, two of the strings (kDetectionUwSText and > > > kDetectionTimeText) required plurality support that did not seem available > > from > > > the WebUI's JS. It seems like I have to localize them in C++. > > > > > > For example: > > > <message name="IDS_CLEANUP_TOOL_DAYS_LAST_RUN" desc="A message on the Chrome > > > Cleanup Tool page, shown to inform when the most recent scan for Unwanted > > > Software was done. [ICU Syntax]"> > > > {DAYS_COUNT, plural, > > > =0 {Last scanned today} > > > =1 {Last scanned yesterday} > > > other {Last scanned # days ago}} > > > </message> > > > > > > Is there an elegant way to resolve this? I might be able to push back on the > > > string to get some that are more easily localizable. > > > > > > > Are the strings really not "finalized"? If they appear in any mocks, I > would > > > just add the strings to the grd files and update later if the mocks > change... > > > > > > Is there a way to add strings to the grd files without staging them for > > > translation? I'd like to avoid wasting translation resources. > > > > Re not translating strings: I've noticed that some messages in grd files have > > the attribute "translateable" set to false. I can't find any official > > documentation about it though... > > Looks like we only have until May 11th to get strings in for M60. I'll start a > fire under our UX person to provide better strings before then. > It's unlikely any of the current ones will be used, so I'll skip on translating > them. Alright, it sounds like you have a handle on the string issue. I'm going to mark this lgtm because I don't want to hold up this CL over the string issue, since you seem to be on the path to taking care of it.
proberge@chromium.org changed reviewers: + sorin@chromium.org
Thanks Tommy! ++sorin for browser/component_updater changes.
lgtm Thank you! component updater lgtm
The CQ bit was checked by proberge@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ftirelo@chromium.org Link to the patchset: https://codereview.chromium.org/2822433003/#ps80001 (title: "Add WeakPtrFactory")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1493142768634900,
"parent_rev": "a71da5150bbafbf33e18c97c837dd4f10dc39d49", "commit_rev":
"81200e069bcced52999b92c3a060ef4c12b8d8b8"}
Message was sent while issue was closed.
Description was changed from ========== Cleanup Tool WebUI: Add functionality to the Scan button This will allow users to manually launch the Chrome Cleanup Tool in scanning mode to detect Unwanted Software on their system. At the moment, the scan does not actually happen and dummy data is returned. BUG=690020 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Cleanup Tool WebUI: Add functionality to the Scan button This will allow users to manually launch the Chrome Cleanup Tool in scanning mode to detect Unwanted Software on their system. At the moment, the scan does not actually happen and dummy data is returned. BUG=690020 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2822433003 Cr-Commit-Position: refs/heads/master@{#467065} Committed: https://chromium.googlesource.com/chromium/src/+/81200e069bcced52999b92c3a060... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/81200e069bcced52999b92c3a060... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
