chromeos: Remove CryptohomeLibrary::TpmGetPassword and TpmIsReady

chrome/browser/ui/webui/about_ui.cc

 // Copyright (c) 2012 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/ui/webui/about_ui.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 503 matching lines @@
   return WrapWithTR(row);
 }
 
 std::string AddStringRow(const std::string& name, const std::string& value) {
   std::string row;
   row.append(WrapWithTD(name));
   row.append(WrapWithTD(value));
   return WrapWithTR(row);
 }
 
-void FinishCryptohomeDataRequestInternal(
-    scoped_refptr<AboutUIHTMLSource> source,
-    int refresh,
-    int request_id,
-    chromeos::DBusMethodCallStatus call_status,
-    bool is_tpm_token_ready) {
-  if (call_status != chromeos::DBUS_METHOD_CALL_SUCCESS)
-    is_tpm_token_ready = false;
+class CryptohomeDataRequest : public base::RefCounted<CryptohomeDataRequest> {

[stevenjb, 2012/07/12 15:32:02]

This seems like it belongs in its own file, not about_ui.cc, even if it is
currently only used here.

+ public:
+  CryptohomeDataRequest(scoped_refptr<AboutUIHTMLSource> source,
+                        const std::string& query,
+                        int request_id)
+      : source_(source),
+        query_(query),
+        request_id_(request_id),
+        num_pending_values_(0),
+        is_mounted_(false),
+        tpm_is_ready_(false),
+        tpm_is_enabled_(false),
+        tpm_is_owned_(false),
+        tpm_is_being_owned_(false),
+        is_tpm_token_ready_(false) {
+  }
 
-  chromeos::CryptohomeLibrary* cryptohome =
-      chromeos::CrosLibrary::Get()->GetCryptohomeLibrary();
-  std::string output;
-  AppendHeader(&output, refresh, "About Cryptohome");
-  AppendBody(&output);
-  AppendRefresh(&output, refresh, "cryptohome");
+  // Starts asynchronous value fetching to finish data request.
+  void Start() {
+    // Request bool values asynchronously.
+    RequestBoolProperty(&chromeos::CryptohomeClient::TpmIsReady,
+                        &tpm_is_ready_);
+    RequestBoolProperty(&chromeos::CryptohomeClient::Pkcs11IsTpmTokenReady,
+                        &is_tpm_token_ready_);
 
-  output.append("<h3>CryptohomeLibrary:</h3>");
-  output.append("<table>");
-  output.append(AddBoolRow("IsMounted", cryptohome->IsMounted()));
-  output.append(AddBoolRow("TpmIsReady", cryptohome->TpmIsReady()));
-  output.append(AddBoolRow("TpmIsEnabled", cryptohome->TpmIsEnabled()));
-  output.append(AddBoolRow("TpmIsOwned", cryptohome->TpmIsOwned()));
-  output.append(AddBoolRow("TpmIsBeingOwned", cryptohome->TpmIsBeingOwned()));
-  output.append(AddBoolRow("Pkcs11IsTpmTokenReady", is_tpm_token_ready));
-  output.append("</table>");
+    // TODO(hashimoto): Get these values asynchronously. crbug.com/126674

[stevenjb, 2012/07/12 15:32:02]

We could, however, wait to move this until addressing crbug.com/126674.

[hashimoto, 2012/07/13 04:27:21]

Yes, let's fix crbug.com/126674 first.

+    chromeos::CryptohomeLibrary* cryptohome_library =
+        chromeos::CrosLibrary::Get()->GetCryptohomeLibrary();
+    is_mounted_ = cryptohome_library->IsMounted();
+    tpm_is_enabled_ = cryptohome_library->TpmIsEnabled();
+    tpm_is_owned_ = cryptohome_library->TpmIsOwned();
+    tpm_is_being_owned_ = cryptohome_library->TpmIsBeingOwned();
+  }
 
-  output.append("<h3>crypto:</h3>");
-  output.append("<table>");
-  output.append(AddBoolRow("IsTPMTokenReady", crypto::IsTPMTokenReady()));
-  std::string token_name, user_pin;
-  if (crypto::IsTPMTokenReady())
-    crypto::GetTPMTokenInfo(&token_name, &user_pin);
-  output.append(AddStringRow("token_name", token_name));
-  output.append(AddStringRow("user_pin", std::string(user_pin.length(), '*')));
-  output.append("</table>");
-  AppendFooter(&output);
+ private:
+  // Member function pointer to CryptohomeClient's bool value getter.
+  typedef void (chromeos::CryptohomeClient::*CryptohomeBoolGetterMethod)(
+      const chromeos::CryptohomeClient::BoolMethodCallback&);

[stevenjb, 2012/07/12 15:32:02]

This is extremely difficult to read/parse. I don't suppose we could simplify it?
Or create a helper class instead? Again, that could wait until the rest of the
class is made asynchronous and moved.

[hashimoto, 2012/07/13 04:27:21]

Agree, member-function-pointer and operator '->*' are ones of the trickiest
parts of C++.

 
-  source->FinishDataRequest(output, request_id);
-}
+  ~CryptohomeDataRequest() {}
 
-void FinishCryptohomeDataRequest(scoped_refptr<AboutUIHTMLSource> source,
-                                 const std::string& query,
-                                 int request_id) {
-  int refresh;
-  base::StringToInt(query, &refresh);
+  // Requests Cryptohome's bool property. OnBoolValueReceived will be called.
+  void RequestBoolProperty(CryptohomeBoolGetterMethod getter,
+                           bool* destination) {
+    ++num_pending_values_;
+    (chromeos::DBusThreadManager::Get()->GetCryptohomeClient()->*getter)(
+        base::Bind(&CryptohomeDataRequest::OnBoolValueReceived,
+                   this,
+                   destination));

[stevenjb, 2012/07/12 15:32:02]

Also very confusing.

+  }
 
-  chromeos::DBusThreadManager::Get()->GetCryptohomeClient()->
-      Pkcs11IsTpmTokenReady(base::Bind(&FinishCryptohomeDataRequestInternal,
-                                       source,
-                                       refresh,
-                                       request_id));
-}
+  // Called when a bool property is received.  This method finishes data request
+  // when there is no pending values to be received.
+  void OnBoolValueReceived(bool* destination,
+                           chromeos::DBusMethodCallStatus call_status,
+                           bool value) {
+    if (call_status == chromeos::DBUS_METHOD_CALL_SUCCESS)
+      *destination = value;
+    if (--num_pending_values_ == 0)
+      Finish();
+  }
+
+  // Finishes data request.
+  void Finish() {
+    int refresh = 0;
+    base::StringToInt(query_, &refresh);
+
+    std::string output;
+    AppendHeader(&output, refresh, "About Cryptohome");
+    AppendBody(&output);
+    AppendRefresh(&output, refresh, "cryptohome");
+
+    output.append("<h3>CryptohomeLibrary:</h3>");
+    output.append("<table>");
+    output.append(AddBoolRow("IsMounted", is_mounted_));
+    output.append(AddBoolRow("TpmIsReady", tpm_is_ready_));
+    output.append(AddBoolRow("TpmIsEnabled", tpm_is_enabled_));
+    output.append(AddBoolRow("TpmIsOwned", tpm_is_owned_));
+    output.append(AddBoolRow("TpmIsBeingOwned", tpm_is_being_owned_));
+    output.append(AddBoolRow("Pkcs11IsTpmTokenReady", is_tpm_token_ready_));
+    output.append("</table>");
+
+    output.append("<h3>crypto:</h3>");
+    output.append("<table>");
+    output.append(AddBoolRow("IsTPMTokenReady", crypto::IsTPMTokenReady()));
+    std::string token_name, user_pin;
+    if (crypto::IsTPMTokenReady())
+      crypto::GetTPMTokenInfo(&token_name, &user_pin);
+    output.append(AddStringRow("token_name", token_name));
+    output.append(
+        AddStringRow("user_pin", std::string(user_pin.length(), '*')));
+    output.append("</table>");
+    AppendFooter(&output);
+
+    source_->FinishDataRequest(output, request_id_);
+  }
+
+  // Data request parameters.
+  scoped_refptr<AboutUIHTMLSource> source_;
+  std::string query_;
+  int request_id_;
+
+  // Number of pending values to be received.
+  int num_pending_values_;
+
+  // Bool values to be appended to the output.
+  bool is_mounted_;
+  bool tpm_is_ready_;
+  bool tpm_is_enabled_;
+  bool tpm_is_owned_;
+  bool tpm_is_being_owned_;
+  bool is_tpm_token_ready_;
+
+  friend class base::RefCounted<CryptohomeDataRequest>;
+};
 
 std::string AboutDiscardsRun() {
   std::string output;
   AppendHeader(&output, 0, "About discards");
   output.append(StringPrintf("<meta http-equiv=\"refresh\" content=\"2;%s\">",
                              chrome::kChromeUIDiscardsURL));
   output.append(WrapWithTag("p", "Discarding a tab..."));
   g_browser_process->oom_priority_manager()->LogMemoryAndDiscardTab();
   AppendFooter(&output);
   return output;
@@ skipping 801 matching lines @@
   std::string host = source_name();
   // Add your data source here, in alphabetical order.
   if (host == chrome::kChromeUIChromeURLsHost) {
     response = ChromeURLs();
   } else if (host == chrome::kChromeUICreditsHost) {
     int idr = (path == kCreditsJsPath) ? IDR_CREDITS_JS : IDR_CREDITS_HTML;
     response = ResourceBundle::GetSharedInstance().GetRawDataResource(
         idr, ui::SCALE_FACTOR_NONE).as_string();
 #if defined(OS_CHROMEOS)
   } else if (host == chrome::kChromeUICryptohomeHost) {
-    FinishCryptohomeDataRequest(this, path, request_id);
+    scoped_refptr<CryptohomeDataRequest> request(
+        new CryptohomeDataRequest(this, path, request_id));
+    request->Start();
     return;
   } else if (host == chrome::kChromeUIDiscardsHost) {
     response = AboutDiscards(path);
 #endif
 #if defined(USE_ASH)
   } else if (host == chrome::kChromeUITransparencyHost) {
     response = AboutTransparency(path);
 #endif
   } else if (host == chrome::kChromeUIDNSHost) {
     AboutDnsHandler::Start(this, request_id);
@@ skipping 64 matching lines @@
 
 AboutUI::AboutUI(content::WebUI* web_ui, const std::string& name)
     : WebUIController(web_ui) {
   Profile* profile = Profile::FromWebUI(web_ui);
   ChromeURLDataManager::DataSource* source =
       new AboutUIHTMLSource(name, profile);
   if (source) {
     ChromeURLDataManager::AddDataSource(profile, source);
   }
 }