Permission prompts shouldn't cause browser to be focused

chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/permission_bubble/permission_prompt_impl.h"
 
 #include <stddef.h>
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/permissions/permission_request.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
+#include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/views/exclusive_access_bubble_views.h"
 #include "chrome/browser/ui/views/harmony/chrome_layout_provider.h"
 #include "chrome/browser/ui/views/page_info/permission_selector_row.h"
 #include "chrome/browser/ui/views/page_info/permission_selector_row_observer.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/strings/grit/components_strings.h"
 #include "components/url_formatter/elide_url.h"
 #include "ui/accessibility/ax_node_data.h"
@@ skipping 244 matching lines @@
 
   // Set |parent_window| because some valid anchors can become hidden.
   bubble_delegate_->set_parent_window(
       platform_util::GetViewForWindow(browser_->window()->GetNativeWindow()));
 
   // Compensate for vertical padding in the anchor view's image. Note this is
   // ignored whenever the anchor view is null.
   bubble_delegate_->set_anchor_view_insets(gfx::Insets(
       GetLayoutConstant(LOCATION_BAR_BUBBLE_ANCHOR_VERTICAL_INSET), 0));
 
-  views::BubbleDialogDelegateView::CreateBubble(bubble_delegate_)->Show();
+  views::Widget* widget =
+      views::BubbleDialogDelegateView::CreateBubble(bubble_delegate_);
+  if (browser_ == BrowserList::GetInstance()->GetLastActive()) {

[raymes, 2017/06/19 04:43:00]

nit: no {} needed

[tapted, 2017/06/20 11:11:32]

This should have a comment - it's pretty subtle why it's being done this way.
E.g.

// If a browser window (or popup) other than the bubble parent has focus, don't
take focus.

also is the choice of BrowserList intentional, rather than just using
browser_->window()->IsActive()? Using BrowserList means that windows from other
apps, or Packaged App windows, won't cause taking focus to be suppressed.

[Timothy Loh, 2017/06/21 07:17:41]

I copied the BrowserList check from 
app_modal_dialog_helper.cc/javascript_dialog_tab_helper.cc, but it sounds like
just checking if it's active is better so I changed it.

+    widget->Show();
+  } else {
+    widget->ShowInactive();
+  }
   bubble_delegate_->SizeToContents();
 
   bubble_delegate_->UpdateAnchor(GetAnchorView(),
                                  GetAnchorPoint(),
                                  GetAnchorArrow());
 }
 
 bool PermissionPromptImpl::CanAcceptRequestUpdate() {
   return !(bubble_delegate_ && bubble_delegate_->IsMouseHovered());
 }
@@ skipping 47 matching lines @@
 
 void PermissionPromptImpl::Accept() {
   if (delegate_)
     delegate_->Accept();
 }
 
 void PermissionPromptImpl::Deny() {
   if (delegate_)
     delegate_->Deny();
 }