Split NTPLoginHandler across chrome://apps and chrome://history.

Split NTPLoginHandler across chrome://apps and chrome://history.

NTPLoginHandler was originally used to handle login from the NTP. It now
handles login on chrome://apps and chrome://history and employs a dirty
little hack to be re-used.

Undo that hack (exposing of ntp.updateLogin on chrome://history) and
split the handler logically, slimming the history handler
(which runs on mobile).

R=mad@chromium.org
BUG=329637
Committed: https://crrev.com/55191fa24c75b7f3b63bc4bd7a183cdcb27af8fb
Cr-Commit-Position: refs/heads/master@{#330874}

[Dan Beam, 2015-05-20 02:39:23]

Patchset #1 (id:1) has been deleted

[Dan Beam, 2015-05-20 02:43:29]

Patchset #2 (id:40001) has been deleted

[Dan Beam, 2015-05-20 02:43:33]

Patchset #1 (id:20001) has been deleted

[Dan Beam, 2015-05-20 02:44:04]

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
File chrome/browser/ui/webui/ntp/new_tab_ui.cc (left):

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/ntp/new_tab_ui.cc:111: web_ui->AddMessageHandler(new
NTPLoginHandler());
not necessary on chrome://newtab any more :)

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
File chrome/browser/ui/webui/ntp/ntp_resource_cache.cc (right):

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/ntp/ntp_resource_cache.cc:463:
AppLauncherLoginHandler::ShouldShow(profile_));
will probably need to futz with this later...

[MAD, 2015-05-20 19:33:02]

LGTM with a bunch of nits.

BYE
MAD

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
File chrome/browser/ui/webui/app_launcher_login_handler.cc (right):

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/app_launcher_login_handler.cc:13: #include
"base/prefs/pref_service.h"
I don't think we need the pref_* stuff anymore, right?

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/app_launcher_login_handler.cc:19: #include
"chrome/browser/profiles/profile_manager.h"
Do we still need this?

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/app_launcher_login_handler.cc:167: std::string username
= profile_info_watcher_->GetAuthenticatedUsername();
DCHECK(profile_info_watcher_.get());  ?

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
File chrome/browser/ui/webui/app_launcher_login_handler.h (right):

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/app_launcher_login_handler.h:67:
scoped_ptr<ProfileInfoWatcher> profile_info_watcher_;
Maybe add the same comment you added to the history handler for this?

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
File chrome/browser/ui/webui/history_login_handler.cc (right):

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/history_login_handler.cc:9: #include
"base/prefs/pref_service.h"
I don't think we need this, and neither pref_names.h below.

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/history_login_handler.cc:14: #include
"chrome/browser/sync/profile_sync_service_factory.h"
No need for signin_* and profile_sync_* either, right?

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
File chrome/browser/ui/webui/profile_info_watcher.cc (right):

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/profile_info_watcher.cc:10: #include
"base/strings/string16.h"
Not needed.

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/profile_info_watcher.cc:18: #include
"content/public/browser/web_ui.h"
Not needed?

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
File chrome/browser/ui/webui/profile_info_watcher.h (right):

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/profile_info_watcher.h:15: class PrefService;
Not needed?

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/profile_info_watcher.h:26: // Gets the authenticated
username (e.g. username@gmail.com) for |profile|.
profile -> profile_

[Dan Beam, 2015-05-20 21:53:24]

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
File chrome/browser/ui/webui/app_launcher_login_handler.cc (right):

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/app_launcher_login_handler.cc:13: #include
"base/prefs/pref_service.h"
On 2015/05/20 19:33:02, MAD wrote:
> I don't think we need the pref_* stuff anymore, right?

this class uses PrefService still

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/app_launcher_login_handler.cc:19: #include
"chrome/browser/profiles/profile_manager.h"
On 2015/05/20 19:33:02, MAD wrote:
> Do we still need this?

Yep, see L172

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/app_launcher_login_handler.cc:167: std::string username
= profile_info_watcher_->GetAuthenticatedUsername();
On 2015/05/20 19:33:01, MAD wrote:
> DCHECK(profile_info_watcher_.get());  ?

nah, that's redundant
https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/scoped...

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
File chrome/browser/ui/webui/app_launcher_login_handler.h (right):

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/app_launcher_login_handler.h:67:
scoped_ptr<ProfileInfoWatcher> profile_info_watcher_;
On 2015/05/20 19:33:02, MAD wrote:
> Maybe add the same comment you added to the history handler for this?

Done.

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
File chrome/browser/ui/webui/history_login_handler.cc (right):

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/history_login_handler.cc:9: #include
"base/prefs/pref_service.h"
On 2015/05/20 19:33:02, MAD wrote:
> I don't think we need this, and neither pref_names.h below.

Done.

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/history_login_handler.cc:14: #include
"chrome/browser/sync/profile_sync_service_factory.h"
On 2015/05/20 19:33:02, MAD wrote:
> No need for signin_* and profile_sync_* either, right?

Done.

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
File chrome/browser/ui/webui/profile_info_watcher.cc (right):

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/profile_info_watcher.cc:10: #include
"base/strings/string16.h"
On 2015/05/20 19:33:02, MAD wrote:
> Not needed.

Done.

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/profile_info_watcher.cc:18: #include
"content/public/browser/web_ui.h"
On 2015/05/20 19:33:02, MAD wrote:
> Not needed?

Done.

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
File chrome/browser/ui/webui/profile_info_watcher.h (right):

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/profile_info_watcher.h:15: class PrefService;
On 2015/05/20 19:33:02, MAD wrote:
> Not needed?

Done.

https://codereview.chromium.org/1143963002/diff/20002/chrome/browser/ui/webui...
chrome/browser/ui/webui/profile_info_watcher.h:26: // Gets the authenticated
username (e.g. username@gmail.com) for |profile|.
On 2015/05/20 19:33:02, MAD wrote:
> profile -> profile_

Done.

[Dan Beam, 2015-05-20 23:59:54]

Patchset #3 (id:90001) has been deleted

[Dan Beam, 2015-05-21 01:18:22]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2015-05-21 01:18:23]

The patchset sent to the CQ was uploaded after l-g-t-m from mad@chromium.org
Link to the patchset: https://codereview.chromium.org/1143963002/#ps110001
(title: "gyp fixes")

[commit-bot: I haz the power, 2015-05-21 01:19:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1143963002/110001

[commit-bot: I haz the power, 2015-05-21 01:52:00]

Committed patchset #3 (id:110001)

[commit-bot: I haz the power, 2015-05-21 01:53:45]

Patchset 3 (id:??) landed as
https://crrev.com/55191fa24c75b7f3b63bc4bd7a183cdcb27af8fb
Cr-Commit-Position: refs/heads/master@{#330874}