Tuesday, November 29, 2016

Make file-picker modal in Chromium for Linux

When I stared working on Chromium, I found a bug in Chromium for Linux: the file-picker was not modal. So, while opening a file-picker, the users could control the main window. Sometimes, it caused mis-behaviors such as we were able to send an email while attaching a file.
There was already a bug and it seemed easy to fix it, but, it took 2 years to fully fix the problem.

The root cause is as follows:

Chromium for Linux uses GtkFileChooserDialog to open a file-picker, but it is not modal to the X11 host window because GtkFileChooserDialog can be modal only to the parent GtkWindow. So I tried to allow the X11 host window to disable input event handling to make a file-picker modal:  Here is the details:

Opening a file-picker



DisableEventListening() disables event listening for the host window using aura::ScopedWindowTargeter, which allows to temporarily replace the event-targeter with ui::NullEventTargeter. It returns a scope handle that is used to call |destroy_callback| when closing the file-picker.

class ScopedHandle {
 public:
   explicit ScopedHandle(const base::Closure& destroy_callback);
   ~ScopedHandle();
   void CancelCallback();

 private:
    base::Closure destroy_callback_;
    DISALLOW_COPY_AND_ASSIGN(ScopedHandle);
};


In addition, we also set another destroy callback(OnFilePickerDestroy) to the GtkFileChooserDialog that can be called when the file-picker is closed.

Close the file-picker


As you can see, OnFilePickerDestroy deletes scoped_handle.
void OnFilePickerDestroy(views::DesktopWindowTreeHostX11::ScopedHandle*
   scoped_handle) {
 delete scoped_handle;
}


Then, |destroy_callback| of ScopedHandle below is automatically called.
void DesktopWindowTreeHostX11::EnableEventListening() {
 DCHECK(modal_dialog_xid_);
 modal_dialog_xid_ = 0;
 targeter_for_modal_.reset();
}

You can find more details and discussion at here.

The first change list was reverted due to the UI freezing problem that happens when the users open a file-picker from a child window of the X11 host window. The second change list finally fixed this issue(BUG 408481, 579408). I also added a test case for the fix: BrowserSelectFileDialogTest.ModalTest.

No comments: