Add support for window.startModalDialog

[GoogleCodeExporter]

Attached is a patch that implements modal dialog support for Windows.

This patch is just to let others try and see if there are any problems with the 
default handler.

The next steps are to catch any potential issues, add support to let clients 
override the modal loop, and maybe talk about Mac support.

I've tested this patch with this test page:

http://www.gusverdun.com/cef/modal/main.htm

The approach here was taken from the way the developer tools handled a debug 
pause condition. See WebKitClientMessageLoopImpl in browser_dev_tools_agent.cc. 

The default handler also disables the opener window while the modal loop is 
running and makes sure it enables it before the modal dialog closes. The modal 
loop is exited when the browser object for the modal loop is destroyed.

Original issue reported on code.google.com by gusver...@gmail.com on 27 May 2011 at 2:50

Attachments:

• modal_dialog_win.patch

[GoogleCodeExporter]

Thanks for working on this. A few questions:

1. Clicking the 'X' on the popup dialog causes the parent window UI to freeze. 
This is probably because the "Console Message" dialog shown by cefclient needs 
to be parented to the modal dialog window instead of the parent window. We'll 
need some way for the client application to know when a window is being 
displayed with modality. Can we make this information available in 
OnBeforePopup()?

2. When a modal dialog is displayed animation in the parent window (like your 
clock example) stops running. Is this expected/desirable behavior?

Original comment by magreenb...@gmail.com on 1 Jun 2011 at 2:21

• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

1. Ouch, good catch! I think I see the error in my ways, the parent window is 
not being re-enabled in this scenario and I was expecting all code paths go 
through UIT_CloseBrowser(). I will investigate an alternative.

2. Yes, same thing happens in Firefox. Chrome::runModal instantiates a class 
called PageGroupLoadDeferrer which defers any callback in the opener's page 
group while the object is alive. I believe this is being done to make sure the 
opener does not attempt to navigate away from the current URL while the modal 
dialog is up, timers included.

Original comment by gusver...@gmail.com on 3 Jun 2011 at 3:07

[GoogleCodeExporter]

Attached is a patch that fixes the problem in comment 1 and extends 
CefLifeSpanHandler to allow clients to override the modal dialog support.

I changed when CefLifeSpanHandler::OnBeforeClose is being called. The previous 
code called this method after the window has been closed, in 
UIT_DestroyBrowser(). I need to be able to re-enable the owner window before 
the window is destroyed, so I added a WM_CLOSE handler in the 
CefBrowserImpl::WndProc and moved the OnBeforeClose call  there.

This means that, from now on, it will be up to clients to forward WM_CLOSE 
messages to all contained CefBrowserWindow instances when the frame window is 
about to close. (See cefclient_win.cpp) if they want to get an OnBeforeClose 
callback.

New methods in CefLifespanHandler:

 ///
  // Called just before a modal loop is about to be entered. This gives you
  // a chance to override the default implementation which disables the opener
  // window and makes this browser window owned by the opener. Return true to 
  // skip the default. If you handle this call, you will also need to handle
  // the OnBeforeClose() call to restore the opener window's state.
  ///
  /*--cef()--*/
  virtual bool OnBeforeModal(CefRefPtr<CefBrowser> browser) { return false; }

  ///
  // Called to enter the modal loop. Provide your own modal loop here. Return 
  // true if you ran your own modal loop and false to use the default.
  ///
  /*--cef()--*/
  virtual bool RunModal(CefRefPtr<CefBrowser> browser) { return false; }

  ///
  // Called when a modal browser window has been destroyed. You must implement 
  // this if you are handling RunModal().
  ///
  /*--cef()--*/
  virtual void QuitModal() { }

Original comment by gusver...@gmail.com on 4 Jun 2011 at 7:46

Attachments:

• modal_dialog_win_2.patch

[GoogleCodeExporter]

1:

> I need to be able to re-enable the owner window before the window is 
destroyed, so
> I added a WM_CLOSE handler in the CefBrowserImpl::WndProc and moved the 
> OnBeforeClose call  there.
> This means that, from now on, it will be up to clients to forward WM_CLOSE 
> messages to all contained CefBrowserWindow instances when the frame window is 
> about to close. (See cefclient_win.cpp) if they want to get an OnBeforeClose 
> callback.

What specifically couldn't you do from WM_DESTROY that works from WM_CLOSE? For 
example, were the calls to GetAncestor() or EnableWindow() failing?

2:

+CefBrowserImpl::~CefBrowserImpl()
+{
+#if defined(OS_WIN)
+  if (is_modal_) {
+    if( client_.get()) {
+      CefRefPtr<CefLifeSpanHandler> handler = client_->GetLifeSpanHandler();
+      if (handler.get()) {
+        // Notify the handler that it can exit its modal loop if it was in one.
+        handler->QuitModal();
+      }
+    }
+
+    // exit our own internal modal message loop now...
+    if (internal_modal_message_loop_is_active_) {
+      MessageLoop* message_loop = MessageLoop::current();
+      message_loop->QuitNow();
+    }
+  }
+#endif
+}

We can't put this kind of logic in the CefBrowserImpl destructor because there 
is no guarantee that the CefBrowserImpl object will be destroyed on the UI 
thread. This logic should probably be in UIT_DestroyBrowser() instead.

3: Under what circumstances would a client return true from either 
OnBeforeModal or RunModal but not both? It might be better to have only a 
RunModal method which would be called before any of the modal-related logic. 
Returning true would cancel both the modal setup and execution, and returning 
false would execute both.

Original comment by magreenb...@gmail.com on 6 Jun 2011 at 2:49

[GoogleCodeExporter]

Issue 56 has been merged into this issue.

Original comment by magreenb...@gmail.com on 6 Jun 2011 at 4:33

[GoogleCodeExporter]

Thanks for taking the time to review my latest patch.

1. The sequence of events is: WM_CLOSE -> DestroyWindow -> focus change -> 
WM_DESTROY. Enabling the window in WM_DESTROY is too late with regards to 
restoring focus to the operner window.

2. My bad. I was not that happy with this either. I'll move it to 
UIT_DestroyBrowser().

3. I don't have a specific use-case. However, my thinking was that 
OnBeforeModal would let you override things that only require Win32 APIs, while 
RunModal would let you override, or make use of, the MessageLoop implementation 
that is not exposed in libcef. I am OK with simplifying this to just the 
RunModal callback.

I'll work on a new patch and add a test case in cefclient.

Original comment by gusver...@gmail.com on 7 Jun 2011 at 4:43

[GoogleCodeExporter]

Attached is a patch that addresses the previous issues and adds a "Modal 
Dialog" option to cefclient's tests menu.

Original comment by gusver...@gmail.com on 10 Jun 2011 at 2:27

Attachments:

• modal_dialog_win_3.patch

[GoogleCodeExporter]

Thanks, committed as revision 255 with minor changes:

1. Use |opener_| in place of |is_popup_| and make the implementation 
cross-platform.
2. Style-related changes.

Original comment by magreenb...@gmail.com on 14 Jun 2011 at 3:11

• Changed state: Fixed