Move screenlock_bridge to components/proximity_auth

chrome/browser/signin/easy_unlock_util.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/signin/easy_unlock_util.h"
+
+#include "base/lazy_instance.h"
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/strings/string16.h"

[Ilya Sherman, 2015/04/23 20:52:53]

nit: Not needed?

[msarda, 2015/04/24 15:47:05]

Done.

+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_window.h"
+#include "chrome/browser/signin/signin_manager_factory.h"
+#include "components/proximity_auth/proximity_auth_client.h"
+#include "components/proximity_auth/screenlock_bridge.h"
+#include "components/signin/core/browser/signin_manager_base.h"
+
+namespace {
+
+// A Chrome specific implementation of the ProximityAuthClient.

[Ilya Sherman, 2015/04/23 20:52:53]

nit: "Chrome specific" -> "Chrome-specific"

[msarda, 2015/04/24 15:47:05]

Done.

+class ChromeProximityAuthClient : public ProximityAuthClient {
+ public:
+  ChromeProximityAuthClient();
+  virtual ~ChromeProximityAuthClient();

[Ilya Sherman, 2015/04/23 20:52:53]

nit: Please leave a blank line after this one.

[sdefresne, 2015/04/24 14:22:02]

~ChromeProximityAuthClient() override;

+  std::string GetAuthenticatedUsername(

[Ilya Sherman, 2015/04/23 20:52:53]

nit: Above this line, please add "// ProximityAuthClient:"

[msarda, 2015/04/24 15:47:05]

Done.

+      content::BrowserContext* browser_context) override;
+  void Lock(content::BrowserContext* browser_context) override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ChromeProximityAuthClient);
+};
+
+ChromeProximityAuthClient::ChromeProximityAuthClient() {
+}
+
+ChromeProximityAuthClient::~ChromeProximityAuthClient() {
+}

[Ilya Sherman, 2015/04/23 20:52:53]

Optional: Eventually, we'll probably a more involved client interface, and it
will make sense to have a separate header and .cc file for this class.  My
personal preference would be to split those off now, but I don't feel strongly
about it.

[msarda, 2015/04/24 15:47:06]

I prefer to do that later as I would argue we could leave it here and just make
it public via a call like GetProximityAuthClientInstance().

+
+std::string ChromeProximityAuthClient::GetAuthenticatedUsername(
+    content::BrowserContext* browser_context) {
+  Profile* profile = Profile::FromBrowserContext(browser_context);
+  const SigninManagerBase* signin_manager =
+      SigninManagerFactory::GetForProfileIfExists(profile);
+  // |profile| has to be a signed-in profile with SigninManager already
+  // created. Otherwise, just crash to collect stack.
+  CHECK(signin_manager);

[Ilya Sherman, 2015/04/23 20:52:53]

nit: No need for the explicit CHECK -- a null pointer dereference will crash
regardless.

[sdefresne, 2015/04/24 14:22:02]

It will only crash reliably if the called method is a virtual method, otherwise
the method address is resolved at compile time and depending on the method
implementation it may not crash.

[msarda, 2015/04/24 15:47:06]

+1 for what Sylvain said (this was also the reason why I added it). We've seen 
crashes in Bling related to Profile that has a stack that was pretty hard to
read.

Note that GetAuthenticatedUsername is not virtual.

+  return signin_manager->GetAuthenticatedUsername();
+}
+
+void ChromeProximityAuthClient::Lock(content::BrowserContext* browser_context) {
+  profiles::LockProfile(Profile::FromBrowserContext(browser_context));
+}
+
+// A utilitary class that serves as the glue code required to initialize and

[Ilya Sherman, 2015/04/23 20:52:53]

nit: s/utilitary/utility

[msarda, 2015/04/24 15:47:05]

Done.

[msarda, 2015/04/24 15:47:05]

Done.

+// manage the lifecycle of various objects of the Proximity Auth components.
+class EasyUnlockUtil {
+ public:
+  ScreenlockBridge* GetScreenlockBridge();
+
+ private:
+  EasyUnlockUtil();
+  ~EasyUnlockUtil();
+
+  scoped_ptr<ChromeProximityAuthClient> proximity_auth_client_;

[sdefresne, 2015/04/24 14:22:03]

Why not directly use instance value instead of scoped_ptr?

ChromeProximityAuthClient proximity_auth_client_;
ScreenlockBridge screenlock_bridge_;

EasyUnlockUtil::EasyUnlockUtil()
: proximity_auth_client_(), screenlock_bride_(&proximity_auth_client_) {
}

[msarda, 2015/04/24 15:47:05]

Done.

+  scoped_ptr<ScreenlockBridge> screenlock_bridge_;
+  friend struct base::DefaultLazyInstanceTraits<EasyUnlockUtil>;

[sdefresne, 2015/04/24 14:22:02]

style: friend comes before everything else

https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...

[msarda, 2015/04/24 15:47:05]

Done.

+  friend struct base::DefaultDeleter<EasyUnlockUtil>;
+
+  DISALLOW_COPY_AND_ASSIGN(EasyUnlockUtil);
+};
+
+base::LazyInstance<EasyUnlockUtil> g_easy_unlock_util_instance =
+    LAZY_INSTANCE_INITIALIZER;
+
+EasyUnlockUtil::EasyUnlockUtil() {
+  proximity_auth_client_.reset(new ChromeProximityAuthClient());
+  screenlock_bridge_.reset(new ScreenlockBridge(proximity_auth_client_.get()));
+}
+
+EasyUnlockUtil::~EasyUnlockUtil() {
+}
+
+ScreenlockBridge* EasyUnlockUtil::GetScreenlockBridge() {
+  return screenlock_bridge_.get();
+}
+
+}  // namespace
+
+ScreenlockBridge* GetScreenlockBridgeInstance() {
+  return g_easy_unlock_util_instance.Pointer()->GetScreenlockBridge();
+}