Fix a forgotten return statement in BrowserContentSettingBubbleModelDelegate.

chrome/browser/ui/content_settings/content_setting_bubble_model_browsertest.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 <vector>
 
 #include "base/files/file_path.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_content_setting_bubble_model_delegate.h"
+#include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/content_settings/content_setting_bubble_model.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/content_settings/core/common/content_settings_types.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 const base::FilePath::CharType kDocRoot[] =
@@ skipping 69 matching lines @@
           CONTENT_SETTINGS_TYPE_MIXEDSCRIPT));
   model->OnCustomLinkClicked();
 
   content::TestNavigationObserver observer(
       browser()->tab_strip_model()->GetActiveWebContents());
   observer.Wait();
 
   EXPECT_FALSE(GetActiveTabSpecificContentSettings()->IsContentBlocked(
       CONTENT_SETTINGS_TYPE_MIXEDSCRIPT));
 }
+
+class ContentSettingBubbleModelMediaStreamTest : public InProcessBrowserTest {
+ public:
+  void ManageMediaStreamSettings(
+      TabSpecificContentSettings::MicrophoneCameraState state) {
+    // Create a bubble with the given camera and microphone access state.
+    TabSpecificContentSettings::FromWebContents(GetFirstTab())->
+        OnMediaStreamPermissionSet(
+            GetFirstTab()->GetLastCommittedURL(),
+            state, std::string(), std::string(), std::string(), std::string());
+    scoped_ptr<ContentSettingBubbleModel> bubble(
+        ContentSettingBubbleModel::CreateContentSettingBubbleModel(
+            browser()->content_setting_bubble_model_delegate(),
+            GetFirstTab(),
+            browser()->profile(),
+            CONTENT_SETTINGS_TYPE_MEDIASTREAM));
+
+    // Click the management link, which opens in a new tab or window.
+    bubble->OnManageLinkClicked();

[msw, 2015/08/06 18:41:30]

One more thought (sorry). You might actually want to still wait for this tab to
load (or at least open), if you synchronously click a link and then try to get
the active window/tab, the window manager might not have even processed the
focus change and this test might be flaky as-is. It probably wouldn't hurt to
leave the nav observer wait here.

[msramek, 2015/08/06 19:11:25]

Well, but the nav observer waits for a specific tab to finish navigation. If we
can't rely on the newly opened tab being focused, then we don't know on which
tab to wait for the navigation. So that wouldn't solve the problem. (see e.g.
line 96 above - we're waiting for GetActiveWebContents()).

Waiting for a navigation would only help us if GetVisibleUrl() didn't return the
currently loaded URL immediately, but only a while after the new tab was opened
and focused.

I readded the observer in case there is something else I missed (I mean, a lot
of other tests do it that way), but it would probably have been ok anyway.

[msw, 2015/08/06 19:24:11]

Hmm, good point. I hope GetActiveTab() doesn't return the old tab used to launch
the manage link, otherwise this might wait indefinitely for a navigation that's
happening in another window/tab that wasn't yet opened/active, and then the test
would time out. Maybe you could add a fatal ASSERT_NE(original_tab,
GetActiveTab()) before trying to wait (doing original_tab = GetFirstTab()
earlier, potentially in-lining GetFirstTab(), since then it'd just be called
once). Otherwise, I don't know how make this bullet-proof; you might just have
to watch this test to see if it's flaky, or ping someone else for better advice.

[msramek, 2015/08/11 13:07:41]

Done. I inlined the original tab and added an assert. I made sure that the
original tab points to a non-internal URL, otherwise the assert would not even
be true - if there already is a chrome://something tab open, it gets reused by
|chrome::ShowContentSettings|.

My trybots are still green after several tries, so I feel comfortable landing
this and watching the status. I also left a comment so that sheriffs feel free
to disable the test if necessary.

+  }
+
+  content::WebContents* GetActiveTab() {
+    // First, we need to find the active browser window. It should be at
+    // the same desktop as the browser in which we invoked the bubble.
+    Browser* active_browser = chrome::FindLastActiveWithHostDesktopType(
+        browser()->host_desktop_type());
+    return active_browser->tab_strip_model()->GetActiveWebContents();
+  }
+
+ private:
+  content::WebContents* GetFirstTab() {
+    return browser()->tab_strip_model()->GetWebContentsAt(0);
+  }
+};
+
+// Tests that clicking on the management link in the media bubble opens
+// the correct section of the settings UI.
+IN_PROC_BROWSER_TEST_F(ContentSettingBubbleModelMediaStreamTest, ManageLink) {
+  // For each of the three options, we click the management link and check if
+  // the active tab starts loading the correct internal url. We don't need to
+  // wait until the navigation finishes.
+
+  // The microphone bubble links to microphone exceptions.
+  ManageMediaStreamSettings(TabSpecificContentSettings::MICROPHONE_ACCESSED);
+  EXPECT_EQ(GURL("chrome://settings/contentExceptions#media-stream-mic"),
+            GetActiveTab()->GetVisibleURL());
+
+  // The camera bubble links to camera exceptions.
+  ManageMediaStreamSettings(TabSpecificContentSettings::CAMERA_ACCESSED);
+  EXPECT_EQ(GURL("chrome://settings/contentExceptions#media-stream-camera"),
+            GetActiveTab()->GetVisibleURL());
+
+  // The bubble for both media devices links to the the first section of the
+  // default media content settings, which is the microphone section.
+  ManageMediaStreamSettings(TabSpecificContentSettings::MICROPHONE_ACCESSED |
+                            TabSpecificContentSettings::CAMERA_ACCESSED);
+  EXPECT_EQ(GURL("chrome://settings/content#media-stream-mic"),
+            GetActiveTab()->GetVisibleURL());
+}