Add chrome://slow for enabling performance tracing for feedback reports.

Add chrome://slow for enabling performance tracing for feedback reports.

BUG=269809
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216845

[Zachary Kuznia, 2013-08-08 01:21:29]

Please take a look.

[Dmitry Polukhin, 2013-08-08 21:13:16]

https://codereview.chromium.org/22336007/diff/1/chrome/app/chromeos_strings.grdp
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/22336007/diff/1/chrome/app/chromeos_strings.g...
chrome/app/chromeos_strings.grdp:1619: <message name="IDS_SLOW_TITLE"
desc="Title of the page at chrome:slow.">
As we discussed please remove title. Because this title is better fit for
chrome:tracing page.

https://codereview.chromium.org/22336007/diff/1/chrome/browser/resources/chro...
File chrome/browser/resources/chromeos/slow.html (right):

https://codereview.chromium.org/22336007/diff/1/chrome/browser/resources/chro...
chrome/browser/resources/chromeos/slow.html:22: <button id="slow-enable"
i18n-content="slowEnable"></button>
I would make both buttons initially hidden because you control visibility in JS
code anyway.

https://codereview.chromium.org/22336007/diff/1/chrome/browser/ui/webui/chrom...
File chrome/browser/ui/webui/chromeos/slow_ui.cc (right):

https://codereview.chromium.org/22336007/diff/1/chrome/browser/ui/webui/chrom...
chrome/browser/ui/webui/chromeos/slow_ui.cc:67: // The handler for Javascript
messages related to the "sim-unlock" view.
It looks like it is not sim-unlock anymore :)

[Zachary Kuznia, 2013-08-08 21:28:57]

https://codereview.chromium.org/22336007/diff/1/chrome/app/chromeos_strings.grdp
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/22336007/diff/1/chrome/app/chromeos_strings.g...
chrome/app/chromeos_strings.grdp:1619: <message name="IDS_SLOW_TITLE"
desc="Title of the page at chrome:slow.">
On 2013/08/08 21:13:16, Dmitry Polukhin wrote:
> As we discussed please remove title. Because this title is better fit for
> chrome:tracing page.

Done.

https://codereview.chromium.org/22336007/diff/1/chrome/browser/resources/chro...
File chrome/browser/resources/chromeos/slow.html (right):

https://codereview.chromium.org/22336007/diff/1/chrome/browser/resources/chro...
chrome/browser/resources/chromeos/slow.html:22: <button id="slow-enable"
i18n-content="slowEnable"></button>
On 2013/08/08 21:13:16, Dmitry Polukhin wrote:
> I would make both buttons initially hidden because you control visibility in
JS
> code anyway.

Done.

https://codereview.chromium.org/22336007/diff/1/chrome/browser/ui/webui/chrom...
File chrome/browser/ui/webui/chromeos/slow_ui.cc (right):

https://codereview.chromium.org/22336007/diff/1/chrome/browser/ui/webui/chrom...
chrome/browser/ui/webui/chromeos/slow_ui.cc:67: // The handler for Javascript
messages related to the "sim-unlock" view.
On 2013/08/08 21:13:16, Dmitry Polukhin wrote:
> It looks like it is not sim-unlock anymore :)

Done.

[Dmitry Polukhin, 2013-08-08 21:31:05]

lgtm

[Zachary Kuznia, 2013-08-08 21:39:06]

Hi jhawkins,

Could you do an OWNERS review of this?

Thanks,
-Zach

[Zachary Kuznia, 2013-08-09 20:54:43]

Hello thestig,

Could you do an OWNERS review of this?

Thanks,
-Zach

[Lei Zhang, 2013-08-09 21:00:01]

lgtm, but please fix the string descriptions.

https://codereview.chromium.org/22336007/diff/7001/chrome/app/chromeos_string...
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/22336007/diff/7001/chrome/app/chromeos_string...
chrome/app/chromeos_strings.grdp:1630: <message name="IDS_SLOW_DISABLE" desc="">
your message should have descriptions for the translators.

[Zachary Kuznia, 2013-08-09 21:52:20]

https://codereview.chromium.org/22336007/diff/7001/chrome/app/chromeos_string...
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/22336007/diff/7001/chrome/app/chromeos_string...
chrome/app/chromeos_strings.grdp:1630: <message name="IDS_SLOW_DISABLE" desc="">
On 2013/08/09 21:00:01, Lei Zhang wrote:
> your message should have descriptions for the translators.

Done.

[Lei Zhang, 2013-08-09 21:55:50]

lgtm++

[commit-bot: I haz the power, 2013-08-09 22:05:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zork@chromium.org/22336007/43001

[commit-bot: I haz the power, 2013-08-09 22:41:43]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac&number...

[commit-bot: I haz the power, 2013-08-09 22:52:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zork@chromium.org/22336007/43001

[James Cook, 2013-08-09 22:54:48]

I don't think you should commit this with this override intact:

virtual bool ShouldAddContentSecurityPolicy() const OVERRIDE {
  return false;
}

tsepez can advise, but I think this is a potential security issue

[Lei Zhang, 2013-08-09 23:00:08]

+tsepez, uncheck CQ box.

[Zachary Kuznia, 2013-08-09 23:44:44]

Changed this as per Tom's suggestions.  The CSP is now safe.

[James Cook, 2013-08-10 00:03:21]

LGTM with nit, thanks for getting CSP worked out so quickly

https://codereview.chromium.org/22336007/diff/14002/chrome/browser/ui/webui/c...
File chrome/browser/ui/webui/chromeos/slow_ui.cc (right):

https://codereview.chromium.org/22336007/diff/14002/chrome/browser/ui/webui/c...
chrome/browser/ui/webui/chromeos/slow_ui.cc:52: 
nit: one blank line

[Zachary Kuznia, 2013-08-10 00:05:46]

https://codereview.chromium.org/22336007/diff/14002/chrome/browser/ui/webui/c...
File chrome/browser/ui/webui/chromeos/slow_ui.cc (right):

https://codereview.chromium.org/22336007/diff/14002/chrome/browser/ui/webui/c...
chrome/browser/ui/webui/chromeos/slow_ui.cc:52: 
On 2013/08/10 00:03:21, James Cook wrote:
> nit: one blank line

Done.

[commit-bot: I haz the power, 2013-08-10 00:11:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zork@chromium.org/22336007/69001

[commit-bot: I haz the power, 2013-08-10 13:59:48]

Change committed as 216845