In Chromium for Mac and Android, we can end up in cases where we end up with 2 select popups showing (if the user presses a select and then quickly presses another before the 1st one has shown). We need a way for the browser to cancel a select popup early to prevent that case.
Created attachment 136230 [details] Patch
Comment on attachment 136230 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136230&action=review > Source/WebKit/chromium/src/ExternalPopupMenu.cpp:75 > + // The client might refuse to create a popup (when there is already one pending to be shown for example). Hmm, can you point me to the code that might refuse to create a popup? The implementation that I found for createExternalPopupMenu() [1] looks like it will always create a new one, but sometimes destroy the old one (if the DCHECK fails). Does this patch assume a Chromium change as well? [1] https://code.google.com/searchframe#OAMlx_jo-ck/src/content/renderer/render_view_impl.cc&l=1577-1583
Yes there is an CL on the Chromium side to make this work: https://chromiumcodereview.appspot.com/10392093/
Ok, looks good when coupled with the Chromium CL. tkent, can you double check for an official review, since I'm not a reviewer?
Comment on attachment 136230 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136230&action=review > Source/WebKit/chromium/src/ExternalPopupMenu.cpp:76 > + else > + // The client might refuse to create a popup (when there is already one pending to be shown for example). > + didCancel(); nit: This else clause contains two physical lines. So we need to enclose it with {}.
Created attachment 142003 [details] Patch
tkent, I fixed the nit, could you R+ CQ+ this? Thanks!
Comment on attachment 142003 [details] Patch Clearing flags on attachment: 142003 Committed r118258: <http://trac.webkit.org/changeset/118258>
All reviewed patches have been landed. Closing bug.