Run the certificate management dialog with BaseShellDialogImpl.

chrome/browser/ui/webui/options/advanced_options_utils_win.cc

Index: chrome/browser/ui/webui/options/advanced_options_utils_win.cc
diff --git a/chrome/browser/ui/webui/options/advanced_options_utils_win.cc b/chrome/browser/ui/webui/options/advanced_options_utils_win.cc
index de9080e41af4f25e8b6f3c8bfcbf0786521984b4..38e1c9899c132963f01714f244197f481cfe334b 100644
--- a/chrome/browser/ui/webui/options/advanced_options_utils_win.cc
+++ b/chrome/browser/ui/webui/options/advanced_options_utils_win.cc
@@ -10,11 +10,16 @@
 #include <shellapi.h>
 
 #include "base/bind.h"
+#include "base/location.h"
+#include "base/memory/ref_counted.h"
+#include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
+#include "base/single_thread_task_runner.h"
 #include "base/threading/thread.h"
 #include "chrome/browser/browser_process.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
+#include "ui/shell_dialogs/base_shell_dialog_win.h"
 #include "ui/views/win/hwnd_util.h"
 
 using content::BrowserThread;
@@ -22,6 +27,50 @@ using content::WebContents;
 
 namespace options {
 
+namespace {
+
+// Shows a Windows certificate management dialog on the dialog thread. Deletes
+// itself when the dialog is closed.

[davidben, 2015/07/23 19:49:33]

I hate self-deleting classes, but the dialog is fire-and-forget, so I don't see
a clear alternative.

+class ManageCertificatesDialog : public ui::BaseShellDialogImpl {
+ public:
+  explicit ManageCertificatesDialog(WebContents* web_contents)
+      : ui_task_runner_(base::MessageLoopForUI::current()->task_runner()) {
+    HWND parent =
+        views::HWNDForNativeWindow(web_contents->GetTopLevelNativeWindow());
+    run_state_ = BeginRun(parent);
+    run_state_.dialog_thread->message_loop()->PostTask(

[mmenke, 2015/07/23 20:03:34]

PostTaskAndReply?  (You'll need to use task_runner() instead of message_loop(),
I believe)

[davidben, 2015/07/23 20:21:55]

Done.

+        FROM_HERE,
+        base::Bind(&ManageCertificatesDialog::Show, base::Unretained(this)));
+  }
+
+ private:
+  ~ManageCertificatesDialog() override {}
+
+  void Show() {
+    CRYPTUI_CERT_MGR_STRUCT cert_mgr = {0};
+    cert_mgr.dwSize = sizeof(CRYPTUI_CERT_MGR_STRUCT);
+    cert_mgr.hwndParent = run_state_.owner;
+    ::CryptUIDlgCertMgr(&cert_mgr);
+    DisableOwner(run_state_.owner);

[davidben, 2015/07/23 19:49:33]

This seems kinda unnecessary since we're not doing anything with the result, but
I didn't want to call EndRun with it enabled in case that confused anything.

[mmenke, 2015/07/23 20:03:34]

Doesn't BeginRun already do this?

[davidben, 2015/07/23 20:21:55]

BeginRun disables it, but CryptUIDlgCertMgr will re-enable it, so DisableOwner
exists to let you keep it disabled while the response is in transit.

We don't actually care about this. I just wasn't sure if EndRun (which
internally re-enables the window) would break, so I matched other callers. Do
you or ananta know if this is okay?

[ananta, 2015/07/23 20:32:57]

Does CryptUIDlgCertMgr display a modal dialog?. From the description it seems
that it does, and if yes then the parent window would not get any input or
activation messages anyways? 

[mmenke, 2015/07/23 20:34:29]

Enabling an already enabled window should be fine.

[davidben, 2015/07/24 22:43:35]

I was just going by the documentation of DisableOwner and how SelectFileDialog
uses it:
https://code.google.com/p/chromium/codesearch#chromium/src/ui/shell_dialogs/b...

Removed.

+
+    ui_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&ManageCertificatesDialog::DialogClosed,
+                              base::Unretained(this)));
+  }
+
+  void DialogClosed() {
+    EndRun(run_state_);

[mmenke, 2015/07/23 20:03:34]

Could you just do this in the destructor, and use a scoped_ptr in your putative
PostTaskAndReply?  Having something take ownership of itself in its constructor
seems a little weird (Though that's what you're implicitly doing), so could make
a separate method to do that after construction or something.

[davidben, 2015/07/23 20:21:55]

Done. Made it no longer self-owning.

+    delete this;
+  }
+
+  scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
+  RunState run_state_;
+
+  DISALLOW_COPY_AND_ASSIGN(ManageCertificatesDialog);
+};
+
+}  // namespace
+
 // Callback that opens the Internet Options control panel dialog with the
 // Connections tab selected.
 void OpenConnectionDialogCallback() {
@@ -58,11 +107,7 @@ void AdvancedOptionsUtilities::ShowNetworkProxySettings(
 
 void AdvancedOptionsUtilities::ShowManageSSLCertificates(
       WebContents* web_contents) {
-  CRYPTUI_CERT_MGR_STRUCT cert_mgr = { 0 };
-  cert_mgr.dwSize = sizeof(CRYPTUI_CERT_MGR_STRUCT);
-  cert_mgr.hwndParent = views::HWNDForNativeWindow(
-      web_contents->GetTopLevelNativeWindow());
-  ::CryptUIDlgCertMgr(&cert_mgr);
+  new ManageCertificatesDialog(web_contents);
 }
 
 }  // namespace options