Don't allow elevation for CHROME_OS.

Don't allow elevation for CHROME_OS.

BUG=228867
TEST=Manual

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

[Adrian Kuegel, 2013-04-11 13:11:49]

Bernhard, could you please review this changelist? Feel free to reject if it is
too much stuff from me to review already.

[Bernhard Bauer, 2013-04-11 13:31:18]

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/managed_mod...
File chrome/browser/managed_mode/managed_mode_navigation_observer.cc (right):

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/managed_mod...
chrome/browser/managed_mode/managed_mode_navigation_observer.cc:345:
is_elevated_ = is_elevated;
Hmm, could we DCHECK in that case?

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/managed_mod...
File chrome/browser/managed_mode/managed_user_service.cc (right):

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/managed_mod...
chrome/browser/managed_mode/managed_user_service.cc:143: callback.Run(false);
Again, should we maybe DCHECK?

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/managed_mod...
chrome/browser/managed_mode/managed_user_service.cc:425:
elevated_for_extensions_.insert(extension_id);
And here

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/managed_mod...
chrome/browser/managed_mode/managed_user_service.cc:432:
elevated_for_extensions_.erase(extension_id);
And here

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/resources/h...
File chrome/browser/resources/history/history.js (right):

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/resources/h...
chrome/browser/resources/history/history.js:1841: function
managedUserElevated(isElevated) {
Do we even expect this whole method to be called on Chrome OS?

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/resources/o...
File chrome/browser/resources/options/managed_user_settings.js (right):

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/resources/o...
chrome/browser/resources/options/managed_user_settings.js:76:
$('set-passphrase').onclick = function() {
This should also be hidden on Chrome OS.

[Adrian Kuegel, 2013-04-11 15:56:10]

I added NOTREACHED statements in the elevation related functions, and modified
the other code accordingly to make it work. Also I removed the set passphrase
button and the checkbox on ChromeOS. Can you please take another look?

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/managed_mod...
File chrome/browser/managed_mode/managed_mode_navigation_observer.cc (right):

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/managed_mod...
chrome/browser/managed_mode/managed_mode_navigation_observer.cc:345:
is_elevated_ = is_elevated;
Yes, that should work. But it implies I need to change the code which would set
this to true at startup. I started implementing this trying to do as few changes
as possible (which might not be the right way to do it, I know :-) ).

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/managed_mod...
File chrome/browser/managed_mode/managed_user_service.cc (right):

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/managed_mod...
chrome/browser/managed_mode/managed_user_service.cc:133: return false;
I could add a DCHECK here, too, if I modify the code in
extension_uninstall_dialog.cc, anyway.

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/managed_mod...
chrome/browser/managed_mode/managed_user_service.cc:143: callback.Run(false);
If I add a DCHECK here, I would have to also change the code in
extension_uninstall_dialog.cc and crx_installer.cc. But maybe I should do that,
anyway.

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/resources/h...
File chrome/browser/resources/history/history.js (right):

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/resources/h...
chrome/browser/resources/history/history.js:1841: function
managedUserElevated(isElevated) {
I guess I should move the call to chrome.send('getManagedUserElevated') inside
the !cr.isChromeOS block. Then this method should not be called anymore.

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/resources/o...
File chrome/browser/resources/options/managed_user_settings.js (right):

https://codereview.chromium.org/14066003/diff/3001/chrome/browser/resources/o...
chrome/browser/resources/options/managed_user_settings.js:76:
$('set-passphrase').onclick = function() {
True. And I should exclude the browsertest for set passphrase.

[Bernhard Bauer, 2013-04-11 16:26:38]

lgtm

[Adrian Kuegel, 2013-04-12 13:51:12]

James, could I please get your approval for this changelist? Bernhard is OWNER
for chrome/browser/managed_mode, but I still need approval for all other files.
Please let me know if I should also ask someone else.

[James Hawkins, 2013-04-12 17:14:07]

On 2013/04/12 13:51:12, Adrian Kuegel wrote:
> James, could I please get your approval for this changelist? Bernhard is OWNER
> for chrome/browser/managed_mode, but I still need approval for all other
files.
> Please let me know if I should also ask someone else.

I'm not owner of extensions/, but I'll do resources/ and webui/

[James Hawkins, 2013-04-12 17:15:36]

LGTM but I'd like to note that this is a poor long-term fix (adding a ton of
platform checks).  I suggest filing a bug to refactor this such that the checks
can be removed.

https://codereview.chromium.org/14066003/diff/5011/chrome/browser/resources/m...
File chrome/browser/resources/managed_mode_block_interstitial.html (right):

https://codereview.chromium.org/14066003/diff/5011/chrome/browser/resources/m...
chrome/browser/resources/managed_mode_block_interstitial.html:27: <button
id="bypass-block-button" i18n-content="bypassBlockButton"></button>
80 cols

[Adrian Kuegel, 2013-04-16 14:44:36]

Benjamin, can you please review the files in chrome/browser/extensions?

[Adrian Kuegel, 2013-04-16 14:52:08]

On 2013/04/12 17:15:36, James Hawkins wrote:
> LGTM but I'd like to note that this is a poor long-term fix (adding a ton of
> platform checks).  I suggest filing a bug to refactor this such that the
checks
> can be removed.

Done (crbug 231923).

[not at google - send to devlin, 2013-04-17 01:34:11]

Yeah this is death by a thousand ifdefs. lgtm I suppose.

[commit-bot: I haz the power, 2013-04-18 07:49:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/akuegel@chromium.org/14066003/32001

[commit-bot: I haz the power, 2013-04-18 09:01:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/akuegel@chromium.org/14066003/48001

[commit-bot: I haz the power, 2013-04-18 11:27:56]

Change committed as 194882