MD Settings search code will not assume the Advanced section exists.

MD Settings search code will not assume the Advanced section exists.

Guest Mode outside of ChromeOS does not have an Advanced section.

BUG=652666
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/88eb4e208df6ec8bd251d573d8e37b7a65f1dd84
Cr-Commit-Position: refs/heads/master@{#432901}

[hcarmona, 2016-11-16 18:37:48]

Description was changed from

==========
MD Settings search code will not assume the Advanced section exists.

Guest Mode outside of ChromeOS does not have an Advanced section.

BUG=652666
==========

to

==========
MD Settings search code will not assume the Advanced section exists.

Guest Mode outside of ChromeOS does not have an Advanced section.

BUG=652666
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[hcarmona, 2016-11-16 18:37:50]

The CQ bit was checked by hcarmona@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-16 18:38:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hcarmona, 2016-11-16 19:06:53]

hcarmona@chromium.org changed reviewers:
+ dpapad@chromium.org

[hcarmona, 2016-11-16 19:06:53]

PTAL, I found a crash on chromeos when looking at this.
http://crbug.com/665993

[commit-bot: I haz the power, 2016-11-16 19:47:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-16 19:47:00]

Dry run: This issue passed the CQ dry run.

[dpapad, 2016-11-16 20:01:58]

lgtm

[hcarmona, 2016-11-16 20:06:42]

The CQ bit was checked by hcarmona@chromium.org

[commit-bot: I haz the power, 2016-11-16 20:07:17]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[michaelpg, 2016-11-16 20:10:14]

michaelpg@chromium.org changed reviewers:
+ michaelpg@chromium.org

[michaelpg, 2016-11-16 20:10:15]

https://codereview.chromium.org/2507873002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_main/settings_main.js (right):

https://codereview.chromium.org/2507873002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_main/settings_main.js:334: if
(cr.isChromeOS || !loadTimeData.getBoolean('isGuest')) {
Please use:
  if (this.pageVisibility.showAdvancedSettings !== false)
instead, or the this.showAdvancedSettings_ function.. (triple equals cuz it may
be undefined, which means "don't hide")

We shouldn't try to duplicate this logic in multiple places (already have it in
settings_ui.js).

[hcarmona, 2016-11-16 20:11:37]

The CQ bit was unchecked by hcarmona@chromium.org

[michaelpg, 2016-11-16 20:12:32]

no need to uncheck commit if you'd just like to change it later

https://codereview.chromium.org/2507873002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_main/settings_main.js (right):

https://codereview.chromium.org/2507873002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_main/settings_main.js:334: if
(cr.isChromeOS || !loadTimeData.getBoolean('isGuest')) {
On 2016/11/16 20:10:14, michaelpg wrote:
> Please use:
>   if (this.pageVisibility.showAdvancedSettings !== false)
> instead, or the this.showAdvancedSettings_ function.. (triple equals cuz it
may
> be undefined, which means "don't hide")
> 
> We shouldn't try to duplicate this logic in multiple places (already have it
in
> settings_ui.js).

Also because we mess with pageVisibility in tests sometimes.

[hcarmona, 2016-11-16 20:21:04]

The CQ bit was checked by hcarmona@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-16 20:21:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hcarmona, 2016-11-16 20:23:08]

Updated to use pageVisibility.advancedSettings

https://codereview.chromium.org/2507873002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_main/settings_main.js (right):

https://codereview.chromium.org/2507873002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_main/settings_main.js:334: if
(cr.isChromeOS || !loadTimeData.getBoolean('isGuest')) {
On 2016/11/16 20:10:14, michaelpg wrote:
> Please use:
>   if (this.pageVisibility.showAdvancedSettings !== false)
> instead, or the this.showAdvancedSettings_ function.. (triple equals cuz it
may
> be undefined, which means "don't hide")
> 
> We shouldn't try to duplicate this logic in multiple places (already have it
in
> settings_ui.js).

Done.

[hcarmona, 2016-11-16 21:50:16]

The CQ bit was unchecked by hcarmona@chromium.org

[hcarmona, 2016-11-16 21:50:16]

The CQ bit was checked by hcarmona@chromium.org

[hcarmona, 2016-11-16 21:50:17]

The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/2507873002/#ps20001
(title: "Use this.pageVisibility.advancedSettings")

[commit-bot: I haz the power, 2016-11-16 21:50:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[michaelpg, 2016-11-16 22:47:53]

ty, lgtm

[commit-bot: I haz the power, 2016-11-17 00:55:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-17 00:55:19]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[hcarmona, 2016-11-17 17:08:16]

The CQ bit was checked by hcarmona@chromium.org

[commit-bot: I haz the power, 2016-11-17 17:08:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-17 17:44:15]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-17 18:08:43]

Description was changed from

==========
MD Settings search code will not assume the Advanced section exists.

Guest Mode outside of ChromeOS does not have an Advanced section.

BUG=652666
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings search code will not assume the Advanced section exists.

Guest Mode outside of ChromeOS does not have an Advanced section.

BUG=652666
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/88eb4e208df6ec8bd251d573d8e37b7a65f1dd84
Cr-Commit-Position: refs/heads/master@{#432901}
==========

[commit-bot: I haz the power, 2016-11-17 18:08:45]

Patchset 2 (id:??) landed as
https://crrev.com/88eb4e208df6ec8bd251d573d8e37b7a65f1dd84
Cr-Commit-Position: refs/heads/master@{#432901}