Settings People Rewrite: Make Sync/Sign-in naming consistent to People.

Settings People Rewrite: Make Sync/Sign-in naming consistent to People.

Patch does a few things:
 1) Renames a bunch of inconsistently named Sync / Sign-in classes and
    Polymer pages to the consistent People.

 2) Enables People section on ChromeOS, as it should be.

BUG=563721, 559481
Committed: https://crrev.com/8224d69c949ca117ec7cc706b4e1bf81e4eaee49
Cr-Commit-Position: refs/heads/master@{#364510}

[tommycli, 2015-12-08 23:00:00]

tommycli@chromium.org changed reviewers:
+ dschuyler@chromium.org

[tommycli, 2015-12-08 23:00:01]

dschuyler: ptal. this patch just renames a bunch of sync_ to people_.

Thanks!

[tommycli, 2015-12-09 23:56:21]

dschuyler: ping

[dschuyler, 2015-12-10 00:47:18]

On 2015/12/09 23:56:21, tommycli wrote:
> dschuyler: ping

Cool, so this looks like mostly a rename/move.  Though 
it also looks like there as a cl format (or similar)
pass over the code.  Some of the formatting leaves an
oddly placed second line with "->" starting the line.
Please look those over and let me know if you also think
it looks like the auto-format is making an error.

[dschuyler, 2015-12-10 00:51:48]

https://codereview.chromium.org/1503333003/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/people_handler.cc (right):

https://codereview.chromium.org/1503333003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/people_handler.cc:270: ->IsAuthenticated()) {
I think cl format is making a mistake here.  Please change this back.  :)

https://codereview.chromium.org/1503333003/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/people_handler_unittest.cc (right):

https://codereview.chromium.org/1503333003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/people_handler_unittest.cc:266:
->current_login_ui());
This is another case where it looks more readable in the older format.

[tommycli, 2015-12-10 01:57:02]

dschuyler: thanks! see below

https://codereview.chromium.org/1503333003/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/people_handler.cc (right):

https://codereview.chromium.org/1503333003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/people_handler.cc:270: ->IsAuthenticated()) {
On 2015/12/10 00:51:48, dschuyler wrote:
> I think cl format is making a mistake here.  Please change this back.  :)

Done. I provided a third alternative, since I think the old one is pretty
egregiously bad actually.

https://codereview.chromium.org/1503333003/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/people_handler_unittest.cc (right):

https://codereview.chromium.org/1503333003/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/people_handler_unittest.cc:266:
->current_login_ui());
On 2015/12/10 00:51:48, dschuyler wrote:
> This is another case where it looks more readable in the older format.

Done.

[dschuyler, 2015-12-10 17:55:16]

Other than the formatting nits, lgtm.

https://codereview.chromium.org/1503333003/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/people_handler.cc (right):

https://codereview.chromium.org/1503333003/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/people_handler.cc:274:
browser->profile())->IsAuthenticated()) {
nit: the wrapped line is indented too far.

https://codereview.chromium.org/1503333003/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/people_handler_unittest.cc (right):

https://codereview.chromium.org/1503333003/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/people_handler_unittest.cc:309:
->current_login_ui());
nit: (well, this is a collection of nits): Please do a grep/find in these
changed .cc files for "  ->".  You'll find several of these oddly wrapped lines.

[tommycli, 2015-12-10 19:34:36]

dschuyler: thanks!

If the below two nits weren't resolved to your satisfaction, feel free to
uncheck the CQ box and we can discuss in-person.

https://codereview.chromium.org/1503333003/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/people_handler.cc (right):

https://codereview.chromium.org/1503333003/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/people_handler.cc:274:
browser->profile())->IsAuthenticated()) {
On 2015/12/10 17:55:16, dschuyler wrote:
> nit: the wrapped line is indented too far.

Err... I think the old way doesn't follow the style guide. Multi-line function
calls are always indented.

The only time conditionals have the second line aligned with the first are when
the first line ends with a boolean operator and the second line is an
independent clause.

https://google.github.io/styleguide/cppguide.html#Boolean_Expressions
https://google.github.io/styleguide/cppguide.html#Function_Calls

https://codereview.chromium.org/1503333003/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/people_handler_unittest.cc (right):

https://codereview.chromium.org/1503333003/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/people_handler_unittest.cc:309:
->current_login_ui());
On 2015/12/10 17:55:16, dschuyler wrote:
> nit: (well, this is a collection of nits): Please do a grep/find in these
> changed .cc files for "  ->".  You'll find several of these oddly wrapped
lines.

Wrapping with "  ->" is pretty common in the Chrome codebase: 
https://code.google.com/p/chromium/codesearch#search/&q=%22%20%20-%3E%22%20fi...

That being said, I just reversed git cl format on this whole file, since the old
styling also meets the style guide criteria.

[tommycli, 2015-12-10 19:34:41]

The CQ bit was checked by tommycli@chromium.org

[tommycli, 2015-12-10 19:34:41]

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

[commit-bot: I haz the power, 2015-12-10 19:39:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1503333003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1503333003/80001

[commit-bot: I haz the power, 2015-12-10 21:12:35]

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

[commit-bot: I haz the power, 2015-12-10 21:12:40]

Failed to apply patch for
chrome/browser/resources/settings/basic_page/basic_page.html:
While running git apply --index -3 -p1;
  error: patch failed:
chrome/browser/resources/settings/basic_page/basic_page.html:3
  Falling back to three-way merge...
  Applied patch to
'chrome/browser/resources/settings/basic_page/basic_page.html' with conflicts.
  U chrome/browser/resources/settings/basic_page/basic_page.html

Patch:       chrome/browser/resources/settings/basic_page/basic_page.html
Index: chrome/browser/resources/settings/basic_page/basic_page.html
diff --git a/chrome/browser/resources/settings/basic_page/basic_page.html
b/chrome/browser/resources/settings/basic_page/basic_page.html
index
ca8e3d90e85f5978c44e099916e9fca6bcf1b510..9103cf706fa912bd6006bcc3d844096d8e96f12b
100644
--- a/chrome/browser/resources/settings/basic_page/basic_page.html
+++ b/chrome/browser/resources/settings/basic_page/basic_page.html
@@ -3,6 +3,7 @@
 <link rel="import" href="chrome://md-settings/search_page/search_page.html">
 <link rel="import"
href="chrome://md-settings/settings_page/settings_section.html">
 <link rel="import"
href="chrome://md-settings/on_startup_page/on_startup_page.html">
+<link rel="import" href="chrome://md-settings/people_page/people_page.html">
 
 <if expr="chromeos">
 <link rel="import"
href="chrome://md-settings/internet_page/internet_page.html">
@@ -11,19 +12,16 @@
 
 <if expr="not chromeos">
 <link rel="import"
href="chrome://md-settings/default_browser_page/default_browser_page.html">
-<link rel="import" href="chrome://md-settings/signin_page/signin_page.html">
 </if>
 
 <dom-module id="settings-basic-page">
   <link rel="import" type="css" href="basic_page.css">
   <template>
-<if expr="not chromeos">
     <settings-section i18n-values="page-title:peoplePageTitle"
         current-route="[[currentRoute]]" section="people">
-      <settings-signin-page current-route="{{currentRoute}}">
-      </settings-signin-page>
+      <settings-people-page current-route="{{currentRoute}}">
+      </settings-people-page>
     </settings-section>
-</if>
 <if expr="chromeos">
     <settings-section i18n-values="page-title:internetPageTitle"
         current-route="[[currentRoute]]" section="internet">

[commit-bot: I haz the power, 2015-12-10 21:15:39]

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

[tommycli, 2015-12-10 21:38:20]

The CQ bit was checked by tommycli@chromium.org

[tommycli, 2015-12-10 21:38:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
dschuyler@chromium.org
Link to the patchset: https://codereview.chromium.org/1503333003/#ps100001
(title: "merge upstream changes")

[commit-bot: I haz the power, 2015-12-10 21:40:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1503333003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1503333003/100001

[commit-bot: I haz the power, 2015-12-10 22:40:11]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-12-10 22:41:42]

Description was changed from

==========
Settings People Rewrite: Make Sync/Sign-in naming consistent to People.

Patch does a few things:
 1) Renames a bunch of inconsistently named Sync / Sign-in classes and
    Polymer pages to the consistent People.

 2) Enables People section on ChromeOS, as it should be.

BUG=563721,559481
==========

to

==========
Settings People Rewrite: Make Sync/Sign-in naming consistent to People.

Patch does a few things:
 1) Renames a bunch of inconsistently named Sync / Sign-in classes and
    Polymer pages to the consistent People.

 2) Enables People section on ChromeOS, as it should be.

BUG=563721,559481

Committed: https://crrev.com/8224d69c949ca117ec7cc706b4e1bf81e4eaee49
Cr-Commit-Position: refs/heads/master@{#364510}
==========

[commit-bot: I haz the power, 2015-12-10 22:41:43]

Patchset 6 (id:??) landed as
https://crrev.com/8224d69c949ca117ec7cc706b4e1bf81e4eaee49
Cr-Commit-Position: refs/heads/master@{#364510}