adds restart message and link to crash reporting setting

adds restart message and link to crash reporting setting

When the user disables or enables crash reporting in the settings menu
this CL adds a restart message very similar to the hardware acceleration
setting also in the setting menu.

It also clears the Breakpad pipe environment variable when the user is disabling crash reporting, so that the restarted Chrome, which inherits the current environment variable, won't be able to connect to the crash handler service and report crashes.

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

[Evan Stade, 2014-07-29 21:35:05]

https://codereview.chromium.org/424833008/diff/20001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/424833008/diff/20001/chrome/app/generated_res...
chrome/app/generated_resources.grd:11827: +        (requires Chrome <ph
name="BEGIN_BUTTON">&lt;button id="metrics-reporting-reset-restart-button"
type="button" class="link-button standalone-link-button"&gt;</ph>restart<ph
name="END_BUTTON">&lt;/button&gt;</ph>)
please do not put html in grd files

[luken, 2014-07-29 21:37:27]

https://codereview.chromium.org/424833008/diff/20001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/424833008/diff/20001/chrome/app/generated_res...
chrome/app/generated_resources.grd:11827: +        (requires Chrome <ph
name="BEGIN_BUTTON">&lt;button id="metrics-reporting-reset-restart-button"
type="button" class="link-button standalone-link-button"&gt;</ph>restart<ph
name="END_BUTTON">&lt;/button&gt;</ph>)
On 2014/07/29 21:35:04, Evan Stade wrote:
> please do not put html in grd files

I copy/modified this code from here:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/app/google_...

How do you suggest I do this instead?

[Evan Stade, 2014-07-29 21:46:39]

On 2014/07/29 21:37:27, luken wrote:
>
https://codereview.chromium.org/424833008/diff/20001/chrome/app/generated_res...
> File chrome/app/generated_resources.grd (right):
> 
>
https://codereview.chromium.org/424833008/diff/20001/chrome/app/generated_res...
> chrome/app/generated_resources.grd:11827: +        (requires Chrome <ph
> name="BEGIN_BUTTON">&lt;button id="metrics-reporting-reset-restart-button"
> type="button" class="link-button standalone-link-button"&gt;</ph>restart<ph
> name="END_BUTTON">&lt;/button&gt;</ph>)
> On 2014/07/29 21:35:04, Evan Stade wrote:
> > please do not put html in grd files
> 
> I copy/modified this code from here:
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/app/google_...
> 
> How do you suggest I do this instead?

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/app/generat...

[luken, 2014-07-29 21:53:30]

On 2014/07/29 21:46:39, Evan Stade wrote:
> On 2014/07/29 21:37:27, luken wrote:
> >
>
https://codereview.chromium.org/424833008/diff/20001/chrome/app/generated_res...
> > File chrome/app/generated_resources.grd (right):
> > 
> >
>
https://codereview.chromium.org/424833008/diff/20001/chrome/app/generated_res...
> > chrome/app/generated_resources.grd:11827: +        (requires Chrome <ph
> > name="BEGIN_BUTTON">&lt;button id="metrics-reporting-reset-restart-button"
> > type="button" class="link-button standalone-link-button"&gt;</ph>restart<ph
> > name="END_BUTTON">&lt;/button&gt;</ph>)
> > On 2014/07/29 21:35:04, Evan Stade wrote:
> > > please do not put html in grd files
> > 
> > I copy/modified this code from here:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/app/google_...
> > 
> > How do you suggest I do this instead?
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/app/generat...

Thanks for the pointer, I'll do it that way!

[luken, 2014-07-29 23:37:53]

https://codereview.chromium.org/424833008/diff/20001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/424833008/diff/20001/chrome/app/generated_res...
chrome/app/generated_resources.grd:11827: +        (requires Chrome <ph
name="BEGIN_BUTTON">&lt;button id="metrics-reporting-reset-restart-button"
type="button" class="link-button standalone-link-button"&gt;</ph>restart<ph
name="END_BUTTON">&lt;/button&gt;</ph>)
On 2014/07/29 21:37:27, luken wrote:
> On 2014/07/29 21:35:04, Evan Stade wrote:
> > please do not put html in grd files
> 
> I copy/modified this code from here:
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/app/google_...
> 
> How do you suggest I do this instead?

Done.

[Sigurður Ásgeirsson, 2014-07-30 09:46:22]

https://codereview.chromium.org/424833008/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/424833008/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/browser_options_handler.cc:1527: (clear)();
nit: wonky formatting and indent.

[luken, 2014-07-30 17:57:09]

https://codereview.chromium.org/424833008/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/424833008/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/browser_options_handler.cc:1527: (clear)();
On 2014/07/30 09:46:22, Sigurður Ásgeirsson wrote:
> nit: wonky formatting and indent.

Done.

[cpu_(ooo_6.6-7.5), 2014-07-30 22:41:37]

c++ bits lgtm

[Evan Stade, 2014-07-31 15:04:37]

https://codereview.chromium.org/424833008/diff/60001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options.js (right):

https://codereview.chromium.org/424833008/diff/60001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.js:366:
restartButton.setAttribute('class',
restartButton.id = ...
restartButton.className = ...

but then again, you should probably put <span><button><span> with the
appropriate attributes in the html file

[luken, 2014-07-31 18:58:37]

https://codereview.chromium.org/424833008/diff/60001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options.js (right):

https://codereview.chromium.org/424833008/diff/60001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.js:366:
restartButton.setAttribute('class',
On 2014/07/31 15:04:37, Evan Stade wrote:
> restartButton.id = ...
> restartButton.className = ...
> 
> but then again, you should probably put <span><button><span> with the
> appropriate attributes in the html file

I moved it to HTML, but in order to battle rogue whitespace within the string I
had to format the HTML a bit awkwardly.

[Evan Stade, 2014-07-31 19:32:31]

lgtm, thanks

https://codereview.chromium.org/424833008/diff/80001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options.js (right):

https://codereview.chromium.org/424833008/diff/80001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.js:362:
$('metrics-reporting-reset-restart-starting-text').textContent =
nit: you don't need IDs on all these elements (given how long the IDs all are,
it makes it a bit harder to read here and in the html). Something like this
would work:

var restartElements =
    $('metrics-reporting-reset-restart').querySelectorall('*');
for (int i = 0; i < restartTextFragments.length; i++) {
  restartElements[i].textContent = restartTextFragments[i];
}

restartElements[1].onClick = ...

[luken, 2014-08-01 00:53:58]

Siggi, can you PTAL? Thanks

https://codereview.chromium.org/424833008/diff/80001/chrome/browser/resources...
File chrome/browser/resources/options/browser_options.js (right):

https://codereview.chromium.org/424833008/diff/80001/chrome/browser/resources...
chrome/browser/resources/options/browser_options.js:362:
$('metrics-reporting-reset-restart-starting-text').textContent =
On 2014/07/31 19:32:31, Evan Stade wrote:
> nit: you don't need IDs on all these elements (given how long the IDs all are,
> it makes it a bit harder to read here and in the html). Something like this
> would work:
> 
> var restartElements =
>     $('metrics-reporting-reset-restart').querySelectorall('*');
> for (int i = 0; i < restartTextFragments.length; i++) {
>   restartElements[i].textContent = restartTextFragments[i];
> }
> 
> restartElements[1].onClick = ...

Done.

[Sigurður Ásgeirsson, 2014-08-01 09:04:58]

lgtm as far as crash reporting is concerned. Alas the code is brittle to changes
in e.g. the name or existence of the "clear env var" export, and we haven't had
good luck with the long-term stability of these.
Is it viable to add some sort of post-build step for this, or else I'd arrange
to set up a manual QA step to prevent regressions...

https://codereview.chromium.org/424833008/diff/100001/chrome/browser/resource...
File chrome/browser/resources/options/browser_options.js (right):

https://codereview.chromium.org/424833008/diff/100001/chrome/browser/resource...
chrome/browser/resources/options/browser_options.js:370: /*
is this comment block intentional?

[luken, 2014-08-01 15:45:47]

It's a good point about the brittleness. I will talk to cpu@ about testing
strategies, and I won't close the bug until we have something in place.

https://codereview.chromium.org/424833008/diff/100001/chrome/browser/resource...
File chrome/browser/resources/options/browser_options.js (right):

https://codereview.chromium.org/424833008/diff/100001/chrome/browser/resource...
chrome/browser/resources/options/browser_options.js:370: /*
On 2014/08/01 09:04:58, Sigurður Ásgeirsson wrote:
> is this comment block intentional?
Whoops, nope, accidentally left that in. Removed.

[luken, 2014-08-01 16:08:46]

The CQ bit was checked by luken@chromium.org

[commit-bot: I haz the power, 2014-08-01 16:11:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/luken@chromium.org/424833008/120001

[commit-bot: I haz the power, 2014-08-01 22:41:38]

Change committed as 287110