0

<browser>: Always read <browser>.src attribute from <object>.src.

If we navigate to 'http://www.google.com', then navigate to '' (empty src), reading <browser>.src would now give us (expected) 'http://www.google.com' since we ignore setting empty src attribute in browser_plugin.cc.

BUG=149001
TEST=In web inspector, setting src on <browser> element and reading it.
Added platform app test for verifying the change.
Ran content_browsertests for BrowserPluginTest* and BrowserPluginHostTest*

Review URL: https://chromiumcodereview.appspot.com/11111020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@162011 0039d316-1c4b-4281-b951-d872f2087c98
This commit is contained in:
lazyboy@chromium.org
2012-10-16 00:15:10 +00:00
parent ccaae54c58
commit 61b98c6d9e
8 changed files with 126 additions and 31 deletions
chrome
browser
renderer
resources
extensions
test
data
extensions
platform_apps
browser_tag_src_attribute
content
browser
renderer
browser_plugin

@@ -30,6 +30,11 @@ IN_PROC_BROWSER_TEST_F(BrowserTagTest, Shim) {
ASSERT_TRUE(RunPlatformAppTest("platform_apps/browser_tag")) << message_;
}
IN_PROC_BROWSER_TEST_F(BrowserTagTest, ShimSrcAttribute) {
ASSERT_TRUE(RunPlatformAppTest("platform_apps/browser_tag_src_attribute"))
<< message_;
}
IN_PROC_BROWSER_TEST_F(BrowserTagTest, Isolation) {
ASSERT_TRUE(StartTestServer());
const std::wstring kExpire =

@@ -72,10 +72,17 @@ function BrowserTag(node) {
this.node_,
{attributes: true, attributeFilter: BROWSER_TAG_ATTRIBUTES});
var objectNode = this.objectNode_;
// Expose getters and setters for the attributes.
BROWSER_TAG_ATTRIBUTES.forEach(function(attributeName) {
Object.defineProperty(this.node_, attributeName, {
get: function() {
if (attributeName == 'src') {
// Always read src attribute from the plugin <object> since: a) It can
// have different value when empty src is set. b) BrowserPlugin
// updates its src attribute on guest-initiated navigations.
return objectNode.src;
}
var value = node.getAttribute(attributeName);
var numericValue = parseInt(value, 10);
return isNaN(numericValue) ? value : numericValue;

@@ -0,0 +1,15 @@
<!--
* Copyright (c) 2012 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.
-->
<html>
<body>
<browser src="data:text/html,<body>One</body>"
width="300"
height="200">
</browser>
<script src="main.js"></script>
</body>
</html>

@@ -0,0 +1,56 @@
// Copyright (c) 2012 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.
var checkSrc = function(element, expectedValue) {
// Note that element.getAttribute('src') should not be used, it can be out of
// sync with element.src.
chrome.test.assertEq(expectedValue, element.src);
};
onload = function() {
chrome.test.runTests([
function browserTag() {
var expectedSrcOne = 'data:text/html,<body>One</body>';
var expectedSrcTwo = 'data:text/html,<body>Two</body>';
var expectedSrcThree = 'data:text/html,<body>Three</body>';
// For setting src, we check if both browserTag.setAttribute('src', ?);
// and browserTag.src = ?; works propertly.
var browserTag = document.querySelector('browser');
// Check if initial src is set correctly.
checkSrc(browserTag, expectedSrcOne);
// Change the src.
browserTag.setAttribute('src', expectedSrcTwo);
// Timeout is necessary to give the mutation observers a chance to fire.
setTimeout(function() {
// Expect the src change to be reflected.
checkSrc(browserTag, expectedSrcTwo);
// Set src attribute directly on the element.
browserTag.src = expectedSrcThree;
setTimeout(function() {
// Expect the src change to be reflected.
checkSrc(browserTag, expectedSrcThree);
// Set empty src, this will be ignored.
browserTag.setAttribute('src', '');
setTimeout(function() {
// Expect empty src to be ignored.
checkSrc(browserTag, expectedSrcThree);
// Set empty src again, directly changing the src attribute.
browserTag.src = '';
setTimeout(function() {
// Expect empty src to be ignored.
checkSrc(browserTag, expectedSrcThree);
chrome.test.succeed();
}, 0);
}, 0);
}, 0);
}, 0);
}
]);
};

@@ -0,0 +1,12 @@
{
"name": "Platform App Test: <browser> src",
"version": "1",
"permissions": [
"browserTag"
],
"app": {
"background": {
"scripts": ["test.js"]
}
}
}

@@ -0,0 +1,7 @@
// Copyright (c) 2012 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.
chrome.app.runtime.onLaunched.addListener(function() {
chrome.app.window.create('main.html', {}, function () {});
});

@@ -123,12 +123,13 @@ void BrowserPluginEmbedder::NavigateGuest(
WebContentsImpl* guest_web_contents =
static_cast<WebContentsImpl*>(guest->GetWebContents());
// We ignore loading empty urls in web_contents.
// We do not load empty urls in web_contents.
// If a guest sets empty src attribute after it has navigated to some
// non-empty page, the action is considered no-op.
// TODO(lazyboy): The js shim for browser-plugin might need to reflect empty
// src ignoring in the shadow DOM element: http://crbug.com/149001.
// non-empty page, the action is considered no-op. This empty src navigation
// should never be sent to BrowserPluginEmbedder (browser process).
DCHECK(!src.empty());
if (!src.empty()) {
// TODO(creis): Check the validity of the URL: http://crbug.com/139397.
guest_web_contents->GetController().LoadURL(url,
Referrer(),
PAGE_TRANSITION_AUTO_SUBFRAME,

@@ -102,13 +102,13 @@ std::string BrowserPlugin::GetSrcAttribute() const {
}
void BrowserPlugin::SetSrcAttribute(const std::string& src) {
if (src == src_ && !guest_crashed_)
if (src.empty() || (src == src_ && !guest_crashed_))
return;
// If we haven't created the guest yet, do so now, if |src| is not empty and
// we will navigate it right after creation. If |src| is empty, we can delay
// the creation until we acutally need it.
if (!navigate_src_sent_ && !src.empty()) {
// If we haven't created the guest yet, do so now. We will navigate it right
// after creation. If |src| is empty, we can delay the creation until we
// acutally need it.
if (!navigate_src_sent_) {
BrowserPluginManager::Get()->Send(
new BrowserPluginHostMsg_CreateGuest(
render_view_->GetRoutingID(),
@@ -117,7 +117,6 @@ void BrowserPlugin::SetSrcAttribute(const std::string& src) {
persist_storage_));
}
if (navigate_src_sent_ || !src.empty()) {
scoped_ptr<BrowserPluginHostMsg_ResizeGuest_Params> params(
GetPendingResizeParams());
DCHECK(!params->resize_pending);
@@ -128,16 +127,10 @@ void BrowserPlugin::SetSrcAttribute(const std::string& src) {
instance_id_,
src,
*params));
// Record that we sent a NavigateGuest message to embedder. Once we send
// such a message, subsequent SetSrcAttribute() calls must always send
// NavigateGuest messages to the embedder (even if |src| is empty), so
// resize works correctly for all cases (e.g. The embedder can reset the
// guest's |src| to empty value, resize and then set the |src| to a
// non-empty value).
// Additionally, once this instance has navigated, the storage partition
// cannot be changed, so this value is used for enforcing this.
// Record that we sent a NavigateGuest message to embedder.
// Once this instance has navigated, the storage partition cannot be changed,
// so this value is used for enforcing this.
navigate_src_sent_ = true;
}
src_ = src;
}
@@ -207,7 +200,6 @@ void BrowserPlugin::ParseAttributes(const WebKit::WebPluginParams& params) {
// Set the 'src' attribute last, as it will set the has_navigated_ flag to
// true, which prevents changing the 'partition' attribute.
if (!src.empty())
SetSrcAttribute(src);
}