Commit 6eca4fd0 authored by estark's avatar estark Committed by Commit bot

Before this change, fullscreen and mouse lock were allowed on file:// URLs...

Before this change, fullscreen and mouse lock were allowed on file:// URLs without giving the user a chance to Allow or Deny them and without showing any persistent UI. This change makes it so that users are always prompted with persistent UI on file:// URLs. There is no way to remember the preference to show or hide the persistent permission bubble because file:// URLs lack a clear origin concept.

BUG=455953,455882
TEST=Download http://adrifelt.github.io/demos/all-permissions (right click, Save As). Open the downloaded file in Chrome. Click "Fullscreen": a permissions bubble should have "Dismiss" and "Exit full screen" buttons, where "Dismiss" just dismisses the bubble. If you click Dismiss, the bubble should again show up when you enter fullscreen a second time. For Pointer Lock, the bubble should show Allow and Deny buttons, and the mouse should not be locked until you click Allow. Clicking Allow or Deny should not affect the behavior of the bubble if you press Pointer Lock again.

Review URL: https://codereview.chromium.org/903683005

Cr-Commit-Position: refs/heads/master@{#316322}
parent 85a2fa2a
......@@ -13654,6 +13654,9 @@ Some features may be unavailable. Please check that the profile exists and you
<message name="IDS_FULLSCREEN_ALLOW" desc="Text in the bubble button that grants permission for a site to enter fullscreen and/or lock the mouse.">
Allow
</message>
<message name="IDS_FULLSCREEN_DISMISS" desc="Text in the bubble button that allows a page to stay in fullscreen without persistent UI warning the user that they are in fullscreen mode.">
Dismiss
</message>
<if expr="use_titlecase">
<message name="IDS_FULLSCREEN_EXIT_FULLSCREEN" desc="In Title Case: Text in the bubble button that forces a webpage out of fullscreen mode.">
Exit Full Screen
......
......@@ -262,13 +262,16 @@ const float kHideDuration = 0.7;
labelFrame.size = textFrame.size;
[exitLabel_ setFrame:labelFrame];
// Update the title of denyButton_ according to the current bubbleType_,
// or show no button at all.
// Update the title of |allowButton_| and |denyButton_| according to the
// current |bubbleType_|, or show no button at all.
if (exclusive_access_bubble::ShowButtonsForType(bubbleType_)) {
NSString* denyButtonText =
SysUTF16ToNSString(
exclusive_access_bubble::GetDenyButtonTextForType(bubbleType_));
[denyButton_ setTitle:denyButtonText];
NSString* allowButtonText = SysUTF16ToNSString(
exclusive_access_bubble::GetAllowButtonTextForType(bubbleType_, url_));
[allowButton_ setTitle:allowButtonText];
} else {
[self showButtons:NO];
}
......
......@@ -138,8 +138,8 @@ base::string16 ExclusiveAccessBubble::GetCurrentDenyButtonText() const {
return exclusive_access_bubble::GetDenyButtonTextForType(bubble_type_);
}
base::string16 ExclusiveAccessBubble::GetAllowButtonText() const {
return l10n_util::GetStringUTF16(IDS_FULLSCREEN_ALLOW);
base::string16 ExclusiveAccessBubble::GetCurrentAllowButtonText() const {
return exclusive_access_bubble::GetAllowButtonTextForType(bubble_type_, url_);
}
base::string16 ExclusiveAccessBubble::GetInstructionText() const {
......
......@@ -80,9 +80,9 @@ class ExclusiveAccessBubble : public gfx::AnimationDelegate {
// The following strings may change according to the content type and URL.
base::string16 GetCurrentMessageText() const;
base::string16 GetCurrentDenyButtonText() const;
base::string16 GetCurrentAllowButtonText() const;
// The following strings never change.
base::string16 GetAllowButtonText() const;
base::string16 GetInstructionText() const;
// The browser this bubble is in.
......
......@@ -51,10 +51,12 @@ base::string16 GetLabelTextForType(ExclusiveAccessBubbleType type,
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_EXTENSION_FULLSCREEN_EXIT_INSTRUCTION:
return l10n_util::GetStringUTF16(
IDS_FULLSCREEN_UNKNOWN_EXTENSION_TRIGGERED_FULLSCREEN);
default:
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_NONE:
NOTREACHED();
return base::string16();
}
NOTREACHED();
return base::string16();
}
switch (type) {
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_BUTTONS:
......@@ -80,10 +82,12 @@ base::string16 GetLabelTextForType(ExclusiveAccessBubbleType type,
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_EXTENSION_FULLSCREEN_EXIT_INSTRUCTION:
return l10n_util::GetStringFUTF16(
IDS_FULLSCREEN_EXTENSION_TRIGGERED_FULLSCREEN, host);
default:
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_NONE:
NOTREACHED();
return base::string16();
}
NOTREACHED();
return base::string16();
}
base::string16 GetDenyButtonTextForType(ExclusiveAccessBubbleType type) {
......@@ -99,12 +103,39 @@ base::string16 GetDenyButtonTextForType(ExclusiveAccessBubbleType type) {
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_MOUSELOCK_EXIT_INSTRUCTION:
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_BROWSER_FULLSCREEN_EXIT_INSTRUCTION:
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_EXTENSION_FULLSCREEN_EXIT_INSTRUCTION:
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_NONE:
NOTREACHED(); // No button in this case.
return base::string16();
default:
NOTREACHED();
}
NOTREACHED();
return base::string16();
}
base::string16 GetAllowButtonTextForType(ExclusiveAccessBubbleType type,
const GURL& url) {
switch (type) {
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_BUTTONS:
// Show a Dismiss button instead of Allow for file:// URLs; on
// file:// URLs, the preference is not saved for the origin, so
// the user is opting to just Dismiss the dialog rather than Allow
// future fullscreen attempts.
if (url.SchemeIsFile())
return l10n_util::GetStringUTF16(IDS_FULLSCREEN_DISMISS);
return l10n_util::GetStringUTF16(IDS_FULLSCREEN_ALLOW);
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_MOUSELOCK_BUTTONS:
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_MOUSELOCK_BUTTONS:
return l10n_util::GetStringUTF16(IDS_FULLSCREEN_ALLOW);
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_EXIT_INSTRUCTION:
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_MOUSELOCK_EXIT_INSTRUCTION:
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_MOUSELOCK_EXIT_INSTRUCTION:
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_BROWSER_FULLSCREEN_EXIT_INSTRUCTION:
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_EXTENSION_FULLSCREEN_EXIT_INSTRUCTION:
case EXCLUSIVE_ACCESS_BUBBLE_TYPE_NONE:
NOTREACHED(); // No button in this case.
return base::string16();
}
NOTREACHED();
return base::string16();
}
bool ShowButtonsForType(ExclusiveAccessBubbleType type) {
......
......@@ -40,6 +40,8 @@ base::string16 GetLabelTextForType(ExclusiveAccessBubbleType type,
const GURL& url,
extensions::ExtensionRegistry* registry);
base::string16 GetDenyButtonTextForType(ExclusiveAccessBubbleType type);
base::string16 GetAllowButtonTextForType(ExclusiveAccessBubbleType type,
const GURL& url);
bool ShowButtonsForType(ExclusiveAccessBubbleType type);
void PermissionRequestedByType(ExclusiveAccessBubbleType type,
bool* tab_fullscreen,
......
......@@ -340,14 +340,21 @@ bool FullscreenController::OnAcceptExclusiveAccessPermission() {
// succeed no matter what the embedder is. For example, if youtube.com
// is visited and user selects ALLOW. Later user visits example.com which
// embeds youtube.com in an iframe, which is then ALLOWED to go fullscreen.
GURL requester = GetRequestingOrigin();
GURL embedder = GetEmbeddingOrigin();
ContentSettingsPattern primary_pattern =
ContentSettingsPattern::FromURLNoWildcard(GetRequestingOrigin());
ContentSettingsPattern::FromURLNoWildcard(requester);
ContentSettingsPattern secondary_pattern =
ContentSettingsPattern::FromURLNoWildcard(GetEmbeddingOrigin());
ContentSettingsPattern::FromURLNoWildcard(embedder);
// ContentSettings requires valid patterns and the patterns might be invalid
// in some edge cases like if the current frame is about:blank.
if (primary_pattern.IsValid() && secondary_pattern.IsValid()) {
//
// Do not store preference on file:// URLs, they don't have a clean
// origin policy.
// TODO(estark): Revisit this when crbug.com/455882 is fixed.
if (!requester.SchemeIsFile() && !embedder.SchemeIsFile() &&
primary_pattern.IsValid() && secondary_pattern.IsValid()) {
HostContentSettingsMap* settings_map =
profile()->GetHostContentSettingsMap();
settings_map->SetContentSetting(
......@@ -502,7 +509,13 @@ ContentSetting FullscreenController::GetFullscreenSetting() const {
GURL url = GetRequestingOrigin();
if (IsPrivilegedFullscreenForTab() || url.SchemeIsFile())
// Always ask on file:// URLs, since we can't meaningfully make the
// decision stick for a particular origin.
// TODO(estark): Revisit this when crbug.com/455882 is fixed.
if (url.SchemeIsFile())
return CONTENT_SETTING_ASK;
if (IsPrivilegedFullscreenForTab())
return CONTENT_SETTING_ALLOW;
// If the permission was granted to the website with no embedder, it should
......
......@@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "base/compiler_specific.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/browser_tabstrip.h"
......@@ -11,18 +9,13 @@
#include "chrome/browser/ui/exclusive_access/fullscreen_controller.h"
#include "chrome/browser/ui/exclusive_access/fullscreen_controller_test.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "content/public/browser/render_view_host.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/url_constants.h"
#include "content/public/test/test_navigation_observer.h"
using url::kAboutBlankURL;
using content::WebContents;
using ui::PAGE_TRANSITION_TYPED;
class FullscreenControllerBrowserTest: public FullscreenControllerTest {
};
IN_PROC_BROWSER_TEST_F(FullscreenControllerTest,
PendingMouseLockExitsOnTabSwitch) {
AddTabAtIndex(0, GURL(url::kAboutBlankURL), PAGE_TRANSITION_TYPED);
......@@ -74,3 +67,27 @@ IN_PROC_BROWSER_TEST_F(FullscreenControllerTest,
}
ASSERT_FALSE(IsFullscreenBubbleDisplayed());
}
IN_PROC_BROWSER_TEST_F(FullscreenControllerTest, MouseLockOnFileURL) {
static const base::FilePath::CharType* kEmptyFile =
FILE_PATH_LITERAL("empty.html");
GURL file_url(ui_test_utils::GetTestUrl(
base::FilePath(base::FilePath::kCurrentDirectory),
base::FilePath(kEmptyFile)));
AddTabAtIndex(0, file_url, PAGE_TRANSITION_TYPED);
RequestToLockMouse(true, false);
ASSERT_TRUE(IsFullscreenBubbleDisplayed());
ASSERT_TRUE(IsFullscreenBubbleDisplayingButtons());
}
IN_PROC_BROWSER_TEST_F(FullscreenControllerTest, FullscreenOnFileURL) {
static const base::FilePath::CharType* kEmptyFile =
FILE_PATH_LITERAL("empty.html");
GURL file_url(ui_test_utils::GetTestUrl(
base::FilePath(base::FilePath::kCurrentDirectory),
base::FilePath(kEmptyFile)));
AddTabAtIndex(0, file_url, PAGE_TRANSITION_TYPED);
RequestToLockMouse(true, false);
ASSERT_TRUE(IsFullscreenBubbleDisplayed());
ASSERT_TRUE(IsFullscreenBubbleDisplayingButtons());
}
......@@ -143,7 +143,11 @@ bool MouseLockController::OnAcceptExclusiveAccessPermission() {
ContentSettingsPattern pattern = ContentSettingsPattern::FromURL(url);
// TODO(markusheintz): We should allow patterns for all possible URLs here.
if (pattern.IsValid()) {
//
// Do not store preference on file:// URLs, they don't have a clean
// origin policy.
// TODO(estark): Revisit this when crbug.com/455882 is fixed.
if (!url.SchemeIsFile() && pattern.IsValid()) {
settings_map->SetContentSetting(pattern,
ContentSettingsPattern::Wildcard(),
CONTENT_SETTINGS_TYPE_MOUSELOCK,
......@@ -222,10 +226,15 @@ void MouseLockController::UnlockMouse() {
}
ContentSetting MouseLockController::GetMouseLockSetting(const GURL& url) const {
// Always ask on file:// URLs, since we can't meaningfully make the
// decision stick for a particular origin.
// TODO(estark): Revisit this when crbug.com/455882 is fixed.
if (url.SchemeIsFile())
return CONTENT_SETTING_ASK;
if (exclusive_access_manager()
->fullscreen_controller()
->IsPrivilegedFullscreenForTab() ||
url.SchemeIsFile())
->IsPrivilegedFullscreenForTab())
return CONTENT_SETTING_ALLOW;
HostContentSettingsMap* settings_map = profile()->GetHostContentSettingsMap();
......
......@@ -166,7 +166,6 @@ ExclusiveAccessBubbleViews::ExclusiveAccessView::ExclusiveAccessView(
mouse_lock_exit_instruction_->SetBackgroundColor(background()->get_color());
button_view_ = new ButtonView(this, kPaddingPx);
button_view_->accept_button()->SetText(bubble->GetAllowButtonText());
views::GridLayout* layout = new views::GridLayout(this);
views::ColumnSet* columns = layout->AddColumnSet(0);
......@@ -224,6 +223,9 @@ void ExclusiveAccessBubbleViews::ExclusiveAccessView::UpdateContent(
button_view_->SetVisible(true);
button_view_->deny_button()->SetText(bubble_->GetCurrentDenyButtonText());
button_view_->deny_button()->SetMinSize(gfx::Size());
button_view_->accept_button()->SetText(
bubble_->GetCurrentAllowButtonText());
button_view_->accept_button()->SetMinSize(gfx::Size());
} else {
bool link_visible = true;
base::string16 accelerator;
......
......@@ -41,9 +41,19 @@ PermissionMenuModel::PermissionMenuModel(
}
AddCheckItem(CONTENT_SETTING_DEFAULT, label);
// Media only support CONTENTE_SETTTING_ALLOW for https.
if (permission_.type != CONTENT_SETTINGS_TYPE_MEDIASTREAM ||
url.SchemeIsSecure()) {
// CONTENT_SETTING_ALLOW and CONTENT_SETTING_BLOCK are not allowed for
// fullscreen or mouse lock on file:// URLs, because there wouldn't be
// a reasonable origin with which to associate the preference.
// TODO(estark): Revisit this when crbug.com/455882 is fixed.
bool is_exclusive_access_on_file =
(permission_.type == CONTENT_SETTINGS_TYPE_FULLSCREEN ||
permission_.type == CONTENT_SETTINGS_TYPE_MOUSELOCK) &&
url.SchemeIsFile();
// Media only support CONTENT_SETTTING_ALLOW for https.
if ((permission_.type != CONTENT_SETTINGS_TYPE_MEDIASTREAM ||
url.SchemeIsSecure()) &&
!is_exclusive_access_on_file) {
label = l10n_util::GetStringUTF16(
IDS_WEBSITE_SETTINGS_MENU_ITEM_ALLOW);
AddCheckItem(CONTENT_SETTING_ALLOW, label);
......@@ -55,7 +65,8 @@ PermissionMenuModel::PermissionMenuModel(
AddCheckItem(CONTENT_SETTING_DETECT_IMPORTANT_CONTENT, label);
}
if (permission_.type != CONTENT_SETTINGS_TYPE_FULLSCREEN) {
if (permission_.type != CONTENT_SETTINGS_TYPE_FULLSCREEN &&
!is_exclusive_access_on_file) {
label = l10n_util::GetStringUTF16(
IDS_WEBSITE_SETTINGS_MENU_ITEM_BLOCK);
AddCheckItem(CONTENT_SETTING_BLOCK, label);
......
......@@ -3,10 +3,9 @@
// found in the LICENSE file.
#include "chrome/browser/ui/website_settings/permission_menu_model.h"
#include "components/content_settings/core/common/content_settings.h"
#include "components/content_settings/core/common/content_settings_types.h"
#include "chrome/grit/generated_resources.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
#include "ui/base/l10n/l10n_util.h"
namespace {
......@@ -55,3 +54,25 @@ TEST(PermissionMenuModelTest, TestAllowBlock) {
callback.callback());
EXPECT_EQ(2, model.GetItemCount());
}
TEST(PermissionMenuModelTest, TestFullscreenMouseLockFileUrl) {
TestCallback callback;
WebsiteSettingsUI::PermissionInfo permission;
permission.type = CONTENT_SETTINGS_TYPE_FULLSCREEN;
permission.setting = CONTENT_SETTING_ASK;
permission.default_setting = CONTENT_SETTING_ASK;
PermissionMenuModel fullscreen_model(GURL("file:///test.html"), permission,
callback.callback());
EXPECT_EQ(1, fullscreen_model.GetItemCount());
EXPECT_EQ(
l10n_util::GetStringUTF16(IDS_WEBSITE_SETTINGS_MENU_ITEM_DEFAULT_ASK),
fullscreen_model.GetLabelAt(0));
permission.type = CONTENT_SETTINGS_TYPE_MOUSELOCK;
PermissionMenuModel mouselock_model(GURL("file:///test.html"), permission,
callback.callback());
EXPECT_EQ(1, mouselock_model.GetItemCount());
EXPECT_EQ(
l10n_util::GetStringUTF16(IDS_WEBSITE_SETTINGS_MENU_ITEM_DEFAULT_ASK),
fullscreen_model.GetLabelAt(0));
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment