Implementation for chrome.certificateProvider.requestPin/stopPinRequest

chrome/browser/extensions/api/certificate_provider/certificate_provider_apitest.cc

Index: chrome/browser/extensions/api/certificate_provider/certificate_provider_apitest.cc
diff --git a/chrome/browser/extensions/api/certificate_provider/certificate_provider_apitest.cc b/chrome/browser/extensions/api/certificate_provider/certificate_provider_apitest.cc
index f4b3aa3c022d3becf0bbc94c1207be67020f43b2..67998400755295ae5280d11ef1db95fc44efa790 100644
--- a/chrome/browser/extensions/api/certificate_provider/certificate_provider_apitest.cc
+++ b/chrome/browser/extensions/api/certificate_provider/certificate_provider_apitest.cc
@@ -20,6 +20,9 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
+#include "chrome/browser/chromeos/certificate_provider/certificate_provider_service.h"
+#include "chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.h"
+#include "chrome/browser/extensions/api/certificate_provider/certificate_provider_api.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -38,6 +41,9 @@
 #include "extensions/test/result_catcher.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "ui/views/controls/label.h"
+#include "ui/views/controls/textfield/textfield.h"
+#include "ui/views/widget/widget.h"
 
 using testing::Return;
 using testing::_;
@@ -122,6 +128,43 @@ std::string JsUint8Array(const std::vector<uint8_t>& bytes) {
   return res;
 }
 
+// Enters the code in the ShowPinDialog window and pushes the OK event.
+void EnterCode(chromeos::CertificateProviderService* service,
+               base::string16 code) {

[emaxx, 2016/09/19 14:01:44]

nit: Pass the strings by const-reference (here and in other places in this
file).

[igorcov, 2016/09/19 15:42:37]

Done.

+  chromeos::RequestPinView* view =
+      service->pin_dialog_manager()->active_view_for_testing();
+  view->textfield_for_testing()->SetText(code);
+  view->Accept();
+  base::RunLoop().RunUntilIdle();
+}
+
+// Enters the valid code for extensions from local example folders, in the
+// ShowPinDialog window and waits for the window to close.
+void EnterCorrectPin(chromeos::CertificateProviderService* service) {
+  views::Widget* active_window =
+      service->pin_dialog_manager()->active_window_for_testing();
+  EnterCode(service, base::ASCIIToUTF16("1234"));
+
+  // Wait for extension to send request to close the window.
+  while (!active_window->IsClosed()) {
+    base::RunLoop().RunUntilIdle();
+  }
+}
+
+// Enters an invalid code for extensions from local example folders, in the
+// ShowPinDialog window and waits for the window to update with the error.
+void EnterWrongPin(chromeos::CertificateProviderService* service) {
+  EnterCode(service, base::ASCIIToUTF16("123"));
+
+  chromeos::RequestPinView* view =
+      service->pin_dialog_manager()->active_view_for_testing();
+  // Waiting for extension to open the dialog again with an error in red
+  // displayed.
+  while (view->error_label_for_testing()->enabled_color() != SK_ColorRED) {
+    base::RunLoop().RunUntilIdle();
+  }
+}
+
 class CertificateProviderApiTest : public ExtensionApiTest {
  public:
   CertificateProviderApiTest() {}
@@ -153,6 +196,27 @@ class CertificateProviderApiTest : public ExtensionApiTest {
     content::RunAllPendingInMessageLoop();
   }
 
+  // Loads certificate_provider extension from folder (the page basic.html).
+  // Returns the CertificateProviderService object from browser context.
+  chromeos::CertificateProviderService* LoadExtensionFromFolder(

[emaxx, 2016/09/19 14:01:44]

Please change the existing tests in this file to use this helper function too.
Currently, it seems that there's some duplication of logic between the old code
and the new code.

+      std::string folder,
+      std::string file_name) {
+    content::WebContents* extension_contents =
+        browser()->tab_strip_model()->GetActiveWebContents();
+    chromeos::CertificateProviderService* service =
+        chromeos::CertificateProviderServiceFactory::GetForBrowserContext(
+            extension_contents->GetBrowserContext());
+    const base::FilePath extension_path =
+        test_data_dir_.AppendASCII("certificate_provider/" + folder);
+    const extensions::Extension* const extension =
+        LoadExtension(extension_path);
+    service->pin_dialog_manager()->AddSignRequestId(extension->id(), 123);

[emaxx, 2016/09/19 14:01:44]

nit: The test uses the same constant "123" for different purposes: first, as the
sign request id, and, second, as the entered PIN.
Please change this to make these two cases using different constants.

[igorcov, 2016/09/19 15:42:36]

Done.

+    ui_test_utils::NavigateToURL(browser(),
+                                 extension->GetResourceURL(file_name));
+    base::RunLoop().RunUntilIdle();
+    return service;
+  }
+
  protected:
   policy::MockConfigurationPolicyProvider provider_;
 };
@@ -268,3 +332,99 @@ IN_PROC_BROWSER_TEST_F(CertificateProviderApiTest, Basic) {
     EXPECT_TRUE(result);
   }
 }
+
+// User closes the dialog three times, and the extension should be blocked from

[emaxx, 2016/09/19 14:01:44]

nit: Replace the "three times" text with a more generic text, because the exact
number is actually determined by the externally defined
kMaxClosedDialogsPer10Mins constant.

+// showing it again.
+IN_PROC_BROWSER_TEST_F(CertificateProviderApiTest, ShowPinDialogClose) {
+  chromeos::CertificateProviderService* service =
+      LoadExtensionFromFolder("request_pin", "basic.html");
+
+  views::Widget* window =
+      service->pin_dialog_manager()->active_window_for_testing();
+  for (int i = 0;
+       i < extensions::api::certificate_provider::kMaxClosedDialogsPer10Mins;
+       i++) {
+    window->Close();
+    // Waiting for the new window to pop up.
+    while (window == service->pin_dialog_manager()->active_window_for_testing())

[emaxx, 2016/09/19 14:01:44]

Not sure whether I understand the code correctly. Are you assuming that after
destroying the old Widget* window, the newly constructed window will have a
different address in the memory? This may be true only by occasion, but you
shouldn't rely on this in the test code.

[igorcov, 2016/09/19 15:42:36]

You are right. I've changed the code to wait for the view to update.

+      base::RunLoop().RunUntilIdle();
+
+    window = service->pin_dialog_manager()->active_window_for_testing();
+  }
+
+  // This time when closed, the window should not be able to pop up again.
+  window->Close();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(service->pin_dialog_manager()->active_view_for_testing(), nullptr);
+}
+
+// User enters the correct PIN.
+IN_PROC_BROWSER_TEST_F(CertificateProviderApiTest, ShowPinDialogAccept) {

[emaxx, 2016/09/19 14:01:44]

nit: Please move this test to be the first one among the PIN dialog tests -
because this is actually the most basic scenario.

[igorcov, 2016/09/19 15:42:37]

Done.

+  chromeos::CertificateProviderService* service =
+      LoadExtensionFromFolder("request_pin", "basic.html");
+
+  EnterCorrectPin(service);
+  chromeos::RequestPinView* view =
+      service->pin_dialog_manager()->active_view_for_testing();
+
+  // The view should be set to nullptr when the window is closed.
+  EXPECT_EQ(view, nullptr);
+}
+
+// User enters a wrong PIN first and a correct PIN on the second try.
+IN_PROC_BROWSER_TEST_F(CertificateProviderApiTest, ShowPinDialogWrongPin) {
+  chromeos::CertificateProviderService* service =
+      LoadExtensionFromFolder("request_pin", "basic.html");
+
+  EnterWrongPin(service);
+  chromeos::RequestPinView* view =
+      service->pin_dialog_manager()->active_view_for_testing();
+
+  // The window should be active.
+  EXPECT_EQ(
+      service->pin_dialog_manager()->active_window_for_testing()->IsVisible(),
+      true);
+  view = service->pin_dialog_manager()->active_view_for_testing();
+  EXPECT_NE(view, nullptr);
+
+  // Enter the valid PIN.
+  EnterCorrectPin(service);
+  view = service->pin_dialog_manager()->active_view_for_testing();
+
+  // The view should be set to nullptr when the window is closed.
+  EXPECT_EQ(view, nullptr);
+}
+
+// User enters wrong PIN three times.
+IN_PROC_BROWSER_TEST_F(CertificateProviderApiTest,
+                       ShowPinDialogWrongPinThreeTimes) {
+  chromeos::CertificateProviderService* service =
+      LoadExtensionFromFolder("request_pin", "basic.html");
+  for (int i = 0; i < 3; i++) {
+    EnterWrongPin(service);
+  }
+
+  chromeos::RequestPinView* view =
+      service->pin_dialog_manager()->active_view_for_testing();
+
+  // The textfield has to be disabled, as extension does not allow input now.
+  EXPECT_EQ(view->textfield_for_testing()->enabled(), false);
+
+  // Close the dialog.
+  service->pin_dialog_manager()->active_window_for_testing()->Close();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(service->pin_dialog_manager()->active_view_for_testing(), nullptr);
+}
+
+// User closes the dialog while the extension is processing the request.
+IN_PROC_BROWSER_TEST_F(CertificateProviderApiTest,
+                       ShowPinDialogCloseWhileProcessing) {
+  chromeos::CertificateProviderService* service =
+      LoadExtensionFromFolder("request_pin", "basic_lock.html");
+
+  EnterCode(service, base::ASCIIToUTF16("123"));
+  service->pin_dialog_manager()->active_window_for_testing()->Close();
+  base::RunLoop().RunUntilIdle();
+  // The view should be set to nullptr when the window is closed.
+  EXPECT_EQ(service->pin_dialog_manager()->active_view_for_testing(), nullptr);
+}