Move code for old, deprecated Options UI to only ChromeOS

Move code for old, deprecated Options UI to only ChromeOS

ChromeOS has a few UIs that still use /options/ code.
The goal is to eventually remove the code completely.

R=thestig@chromium.org
BUG=728353
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2927273002
Cr-Commit-Position: refs/heads/master@{#478470}
Committed: https://chromium.googlesource.com/chromium/src/+/fbc499463c31aaa00a4180d11ec348a12650065b

[Dan Beam, 2017-06-09 02:57:54]

Description was changed from

==========
Make Options UI ChromeOS-only

R=
BUG=728353
==========

to

==========
Make Options UI ChromeOS-only

R=
BUG=728353
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2017-06-09 03:00:24]

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

[commit-bot: I haz the power, 2017-06-09 03:00:34]

Dry run: 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, 2017-06-09 03:13:09]

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

[commit-bot: I haz the power, 2017-06-09 03:13:10]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Dan Beam, 2017-06-09 03:53:10]

Description was changed from

==========
Make Options UI ChromeOS-only

R=
BUG=728353
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Move code for old, deprecated Options UI to only ChromeOS

ChromeOS has a few UIs that still use /options/ code.
The goal is to eventually remove the code completely.

R=
BUG=728353
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2017-06-09 03:53:43]

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

[commit-bot: I haz the power, 2017-06-09 03:53:54]

Dry run: CQ is trying da patch.

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

[Dan Beam, 2017-06-09 03:55:45]

Description was changed from

==========
Move code for old, deprecated Options UI to only ChromeOS

ChromeOS has a few UIs that still use /options/ code.
The goal is to eventually remove the code completely.

R=
BUG=728353
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Move code for old, deprecated Options UI to only ChromeOS

ChromeOS has a few UIs that still use /options/ code.
The goal is to eventually remove the code completely.

R=thestig@chromium.org
BUG=728353
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2017-06-09 03:55:45]

dbeam@chromium.org changed reviewers:
+ thestig@chromium.org

[Lei Zhang, 2017-06-09 04:22:54]

https://codereview.chromium.org/2927273002/diff/20001/chrome/BUILD.gn
File chrome/BUILD.gn (left):

https://codereview.chromium.org/2927273002/diff/20001/chrome/BUILD.gn#oldcode...
chrome/BUILD.gn:1531: public_deps += [
"//chrome/browser/resources/chromeos/switch_access" ]
:-\

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/BUILD.gn (right):

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/resource...
chrome/browser/resources/BUILD.gn:109: if (is_chromeos) {
Move out of if (!is_android), i.e. no need for nested if.

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/BUILD.gn
File chrome/browser/ui/BUILD.gn (right):

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/BUILD...
chrome/browser/ui/BUILD.gn:1270: "webui/options/sync_setup_handler.h",
There's most files that start with 'v' below. Sort.

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc (right):

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc:395: #if
defined(OS_CHROMEOS)
Also move out of the !OS_ANDROID block. It can stand on its own.

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings_utils.cc (right):

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings_utils.cc:31: void
ValidateSavedFonts(PrefService* prefs) {
Why not just make this function Mac only then? Fix the one and only caller
instead of pretending it does something on all platforms.

Or don't even ad it to settings_util. Just put it in
chrome/browser/ui/webui/options/font_settings_handler.cc in the anonymous
namespace.

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings_utils_mac.mm (right):

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings_utils_mac.mm:14: #include "base/values.h"
Not needed?

[commit-bot: I haz the power, 2017-06-09 05:08:01]

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

[commit-bot: I haz the power, 2017-06-09 05:08:02]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Dan Beam, 2017-06-09 06:32:55]

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/BUILD.gn (right):

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/resource...
chrome/browser/resources/BUILD.gn:109: if (is_chromeos) {
On 2017/06/09 04:22:53, Lei Zhang wrote:
> Move out of if (!is_android), i.e. no need for nested if.

Done.

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/BUILD.gn
File chrome/browser/ui/BUILD.gn (right):

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/BUILD...
chrome/browser/ui/BUILD.gn:1270: "webui/options/sync_setup_handler.h",
On 2017/06/09 04:22:53, Lei Zhang wrote:
> There's most files that start with 'v' below. Sort.

Done.

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc (right):

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc:395: #if
defined(OS_CHROMEOS)
On 2017/06/09 04:22:53, Lei Zhang wrote:
> Also move out of the !OS_ANDROID block. It can stand on its own.

Done.

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings_utils.cc (right):

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings_utils.cc:31: void
ValidateSavedFonts(PrefService* prefs) {
On 2017/06/09 04:22:53, Lei Zhang wrote:
> Why not just make this function Mac only then? Fix the one and only caller
> instead of pretending it does something on all platforms.
> 
> Or don't even ad it to settings_util. Just put it in
> chrome/browser/ui/webui/options/font_settings_handler.cc in the anonymous
> namespace.

Done.

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings_utils_mac.mm (right):

https://codereview.chromium.org/2927273002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings_utils_mac.mm:14: #include "base/values.h"
On 2017/06/09 04:22:54, Lei Zhang wrote:
> Not needed?

Done.

[Dan Beam, 2017-06-09 06:33:00]

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

[commit-bot: I haz the power, 2017-06-09 06:33:12]

Dry run: CQ is trying da patch.

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

[Lei Zhang, 2017-06-09 06:47:01]

lgtm

https://codereview.chromium.org/2927273002/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings_utils_mac.mm (right):

https://codereview.chromium.org/2927273002/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings_utils_mac.mm:33: }
}  // namespace

[Dan Beam, 2017-06-09 06:49:20]

dbeam@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Dan Beam, 2017-06-09 06:49:21]

+rdevlin.cronin@ for extensions/

https://codereview.chromium.org/2927273002/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings_utils_mac.mm (right):

https://codereview.chromium.org/2927273002/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings_utils_mac.mm:33: }
On 2017/06/09 06:47:01, Lei Zhang wrote:
> }  // namespace

Done.

[Dan Beam, 2017-06-09 06:49:27]

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

[commit-bot: I haz the power, 2017-06-09 06:49:44]

Dry run: 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, 2017-06-09 07:55:01]

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

[commit-bot: I haz the power, 2017-06-09 07:55:02]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Devlin, 2017-06-09 14:56:34]

https://codereview.chromium.org/2927273002/diff/60001/extensions/browser/api/...
File extensions/browser/api/runtime/runtime_apitest.cc (right):

https://codereview.chromium.org/2927273002/diff/60001/extensions/browser/api/...
extensions/browser/api/runtime/runtime_apitest.cc:101: #if defined(OS_CHROMEOS)
Why?  This refers to the extensions options page, and the
runtime.openOptionsPage api is available on all platforms.

[Dan Beam, 2017-06-09 19:23:28]

dbeam@chromium.org changed reviewers:
- rdevlin.cronin@chromium.org

[Dan Beam, 2017-06-09 19:23:29]

Devlin: moving you to CC (not changing extensions/ any more)

https://codereview.chromium.org/2927273002/diff/60001/extensions/browser/api/...
File extensions/browser/api/runtime/runtime_apitest.cc (right):

https://codereview.chromium.org/2927273002/diff/60001/extensions/browser/api/...
extensions/browser/api/runtime/runtime_apitest.cc:101: #if defined(OS_CHROMEOS)
On 2017/06/09 14:56:34, Devlin (sheriff Jun 9 - 12) wrote:
> Why?  This refers to the extensions options page, and the
> runtime.openOptionsPage api is available on all platforms.

yeah, sorry. leftovers from when opening the uber page would cause a test to
fail.  fixed that but forgot to unwrap.

[Dan Beam, 2017-06-09 19:23:36]

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

[commit-bot: I haz the power, 2017-06-09 19:24:00]

Dry run: 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, 2017-06-09 21:24:58]

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

[commit-bot: I haz the power, 2017-06-09 21:24:59]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Dan Beam, 2017-06-09 22:04:38]

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

[commit-bot: I haz the power, 2017-06-09 22:04:57]

Dry run: CQ is trying da patch.

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

[wychen, 2017-06-09 23:07:54]

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

[wychen, 2017-06-09 23:07:55]

https://codereview.chromium.org/2927273002/diff/100001/chrome/browser/ui/BUIL...
File chrome/browser/ui/BUILD.gn (right):

https://codereview.chromium.org/2927273002/diff/100001/chrome/browser/ui/BUIL...
chrome/browser/ui/BUILD.gn:217: # TODO(dbeam): why are all these /chromeos/
files on all platforms?
GN magically filter files based on their names, so that gn files can be simpler.

[Dan Beam, 2017-06-09 23:45:22]

https://codereview.chromium.org/2927273002/diff/100001/chrome/browser/ui/BUIL...
File chrome/browser/ui/BUILD.gn (right):

https://codereview.chromium.org/2927273002/diff/100001/chrome/browser/ui/BUIL...
chrome/browser/ui/BUILD.gn:217: # TODO(dbeam): why are all these /chromeos/
files on all platforms?
On 2017/06/09 23:07:55, wychen wrote:
> GN magically filter files based on their names, so that gn files can be
simpler.

if that's the case, why did I have to move all these files with /chromeos/ in
the path to an if (is_chromeos) { block to appease check_gn_headers?

https://codereview.chromium.org/2927273002/diff2/60001:80001/chrome/browser/u...

[commit-bot: I haz the power, 2017-06-09 23:48:58]

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

[commit-bot: I haz the power, 2017-06-09 23:48:59]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2017-06-09 23:54:19]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2017-06-09 23:54:19]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/2927273002/#ps100001
(title: "check_gn_headers")

[commit-bot: I haz the power, 2017-06-09 23:54:35]

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, 2017-06-10 00:00:53]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1497052459306740,
"parent_rev": "32ba3908a417bdc207a1d9867962500dc5b837ee", "commit_rev":
"83fdf4994dd433ec3eca5f952c1bbb7fb5e8bdda"}

[commit-bot: I haz the power, 2017-06-10 00:01:04]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1497052459306740,
"parent_rev": "2410fc6b7d05f26bca8b649aded31e959060ea3b", "commit_rev":
"fbc499463c31aaa00a4180d11ec348a12650065b"}

[commit-bot: I haz the power, 2017-06-10 00:01:14]

Description was changed from

==========
Move code for old, deprecated Options UI to only ChromeOS

ChromeOS has a few UIs that still use /options/ code.
The goal is to eventually remove the code completely.

R=thestig@chromium.org
BUG=728353
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Move code for old, deprecated Options UI to only ChromeOS

ChromeOS has a few UIs that still use /options/ code.
The goal is to eventually remove the code completely.

R=thestig@chromium.org
BUG=728353
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2927273002
Cr-Commit-Position: refs/heads/master@{#478470}
Committed:
https://chromium.googlesource.com/chromium/src/+/fbc499463c31aaa00a4180d11ec3...
==========

[commit-bot: I haz the power, 2017-06-10 00:01:15]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/fbc499463c31aaa00a4180d11ec3...