Skip to content

GitLab

T
tangled
Commit abb035a0 authored by paulmeyer's avatar paulmeyer Committed by Commit bot
Browse files
Options

Got rid of the internal copies of webview attributes. 

Webview attributes are now only stored in one location (in the webview node), so that all the code needed to sync up the copies could be removed (and was).

Also reworked the behavior of the |allowtransparency| and |autosize| to be both consistent with each other and more intuitive in general (they are both treated like booleans now). I updated the tests to reflect this new behavior.

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

Cr-Commit-Position: refs/heads/master@{#302661}
parent 71ffa214
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 143 additions and 160 deletions
chrome/browser/apps/web_view_browsertest.cc
View file @ abb035a0
...@@ -950,8 +950,10 @@ IN_PROC_BROWSER_TEST_F(WebViewTest, Shim_TestCannotMutateEventName) { ...@@ -950,8 +950,10 @@ IN_PROC_BROWSER_TEST_F(WebViewTest, Shim_TestCannotMutateEventName) {
TestHelper("testCannotMutateEventName", "web_view/shim", NO_TEST_SERVER); TestHelper("testCannotMutateEventName", "web_view/shim", NO_TEST_SERVER);
} }
IN_PROC_BROWSER_TEST_F(WebViewTest, Shim_TestPartitionRaisesException) { IN_PROC_BROWSER_TEST_F(WebViewTest, Shim_TestPartitionChangeAfterNavigation) {
TestHelper("testPartitionRaisesException", "web_view/shim", NO_TEST_SERVER); TestHelper("testPartitionChangeAfterNavigation",
"web_view/shim",
NO_TEST_SERVER);
} }
IN_PROC_BROWSER_TEST_F(WebViewTest, IN_PROC_BROWSER_TEST_F(WebViewTest,
......
chrome/test/data/extensions/platform_apps/web_view/shim/main.js
View file @ abb035a0
...@@ -97,20 +97,28 @@ embedder.test.assertFalse = function(condition) { ...@@ -97,20 +97,28 @@ embedder.test.assertFalse = function(condition) {
// Tests begin. // Tests begin.
// This test verifies that the allowtransparency property cannot be changed // This test verifies that the allowtransparency property is interpreted as true
// once set. The attribute can only be deleted. // if it exists (regardless of its value), and can be removed by setting it to
// to anything false.
function testAllowTransparencyAttribute() { function testAllowTransparencyAttribute() {
var webview = document.createElement('webview'); var webview = document.createElement('webview');
webview.src = 'data:text/html,webview test'; webview.src = 'data:text/html,webview test';
embedder.test.assertFalse(webview.hasAttribute('allowtransparency'));
embedder.test.assertFalse(webview.allowtransparency);
webview.allowtransparency = true; webview.allowtransparency = true;
webview.addEventListener('loadstop', function(e) { webview.addEventListener('loadstop', function(e) {
embedder.test.assertTrue(webview.hasAttribute('allowtransparency')); embedder.test.assertTrue(webview.hasAttribute('allowtransparency'));
webview.allowtransparency = false;
embedder.test.assertTrue(webview.allowtransparency); embedder.test.assertTrue(webview.allowtransparency);
embedder.test.assertTrue(webview.hasAttribute('allowtransparency')); webview.allowtransparency = false;
webview.removeAttribute('allowtransparency'); embedder.test.assertFalse(webview.hasAttribute('allowtransparency'));
embedder.test.assertFalse(webview.allowtransparency);
webview.allowtransparency = '';
embedder.test.assertFalse(webview.hasAttribute('allowtransparency'));
embedder.test.assertFalse(webview.allowtransparency); embedder.test.assertFalse(webview.allowtransparency);
webview.allowtransparency = 'some string';
embedder.test.assertTrue(webview.hasAttribute('allowtransparency'));
embedder.test.assertTrue(webview.allowtransparency);
embedder.test.succeed(); embedder.test.succeed();
}); });
...@@ -616,7 +624,7 @@ function testLoadProgressEvent() { ...@@ -616,7 +624,7 @@ function testLoadProgressEvent() {
// Current expected behavior is that the second event listener will still // Current expected behavior is that the second event listener will still
// fire without crashing. // fire without crashing.
function testDestroyOnEventListener() { function testDestroyOnEventListener() {
var webview = util.createWebViewTagInDOM(arguments.callee.name); var webview = document.createElement('webview');
var url = 'data:text/html,<body>Destroy test</body>'; var url = 'data:text/html,<body>Destroy test</body>';
var loadCommitCount = 0; var loadCommitCount = 0;
...@@ -647,13 +655,14 @@ function testDestroyOnEventListener() { ...@@ -647,13 +655,14 @@ function testDestroyOnEventListener() {
loadCommitCommon(e); loadCommitCommon(e);
}); });
webview.setAttribute('src', url); webview.setAttribute('src', url);
document.body.appendChild(webview);
} }
// This test registers two event listeners on a same event (loadcommit). // This test registers two event listeners on a same event (loadcommit).
// Each of the listener tries to change some properties on the event param, // Each of the listener tries to change some properties on the event param,
// which should not be possible. // which should not be possible.
function testCannotMutateEventName() { function testCannotMutateEventName() {
var webview = util.createWebViewTagInDOM(arguments.callee.name); var webview = document.createElement('webview');
var url = 'data:text/html,<body>Two</body>'; var url = 'data:text/html,<body>Two</body>';
var loadCommitACalled = false; var loadCommitACalled = false;
...@@ -695,23 +704,20 @@ function testCannotMutateEventName() { ...@@ -695,23 +704,20 @@ function testCannotMutateEventName() {
webview.addEventListener('loadcommit', onLoadCommitA); webview.addEventListener('loadcommit', onLoadCommitA);
webview.addEventListener('loadcommit', onLoadCommitB); webview.addEventListener('loadcommit', onLoadCommitB);
webview.setAttribute('src', url); webview.setAttribute('src', url);
document.body.appendChild(webview);
} }
// This test verifies that setting the partition attribute after the src has // This test verifies that the partion attribute cannot be changed after the src
// been set raises an exception. // has been set.
function testPartitionRaisesException() { function testPartitionChangeAfterNavigation() {
var webview = document.createElement('webview'); var webview = document.createElement('webview');
var partitionAttribute = arguments.callee.name; var partitionAttribute = arguments.callee.name;
webview.setAttribute('partition', partitionAttribute); webview.setAttribute('partition', partitionAttribute);
var loadstopHandler = function(e) { var loadstopHandler = function(e) {
try { webview.partition = 'illegal';
webview.partition = 'illegal'; embedder.test.assertEq(partitionAttribute, webview.partition);
embedder.test.fail(); embedder.test.succeed();
} catch (e) {
embedder.test.assertEq(partitionAttribute, webview.partition);
embedder.test.succeed();
}
}; };
webview.addEventListener('loadstop', loadstopHandler); webview.addEventListener('loadstop', loadstopHandler);
...@@ -1999,7 +2005,7 @@ embedder.test.testList = { ...@@ -1999,7 +2005,7 @@ embedder.test.testList = {
'testLoadProgressEvent': testLoadProgressEvent, 'testLoadProgressEvent': testLoadProgressEvent,
'testDestroyOnEventListener': testDestroyOnEventListener, 'testDestroyOnEventListener': testDestroyOnEventListener,
'testCannotMutateEventName': testCannotMutateEventName, 'testCannotMutateEventName': testCannotMutateEventName,
'testPartitionRaisesException': testPartitionRaisesException, 'testPartitionChangeAfterNavigation': testPartitionChangeAfterNavigation,
'testPartitionRemovalAfterNavigationFails': 'testPartitionRemovalAfterNavigationFails':
testPartitionRemovalAfterNavigationFails, testPartitionRemovalAfterNavigationFails,
'testExecuteScriptFail': testExecuteScriptFail, 'testExecuteScriptFail': testExecuteScriptFail,
......
extensions/browser/guest_view/web_view/web_view_apitest.cc
View file @ abb035a0
...@@ -615,8 +615,8 @@ IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestOnEventProperty) { ...@@ -615,8 +615,8 @@ IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestOnEventProperty) {
RunTest("testOnEventProperties", "web_view/apitest"); RunTest("testOnEventProperties", "web_view/apitest");
} }
IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestPartitionRaisesException) { IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestPartitionChangeAfterNavigation) {
RunTest("testPartitionRaisesException", "web_view/apitest"); RunTest("testPartitionChangeAfterNavigation", "web_view/apitest");
} }
IN_PROC_BROWSER_TEST_F(WebViewAPITest, IN_PROC_BROWSER_TEST_F(WebViewAPITest,
......
extensions/renderer/resources/guest_view/web_view_attributes.js
View file @ abb035a0
...@@ -13,63 +13,69 @@ var WebViewConstants = require('webViewConstants').WebViewConstants; ...@@ -13,63 +13,69 @@ var WebViewConstants = require('webViewConstants').WebViewConstants;
// Default implementation of a WebView attribute. // Default implementation of a WebView attribute.
function WebViewAttribute(name, webViewImpl) { function WebViewAttribute(name, webViewImpl) {
this.name = name; this.name = name;
this.value = '';
this.webViewImpl = webViewImpl; this.webViewImpl = webViewImpl;
this.ignoreNextMutation = false;
} }
// Retrieves and returns the attribute's value.
WebViewAttribute.prototype.getValue = function() { WebViewAttribute.prototype.getValue = function() {
return this.value || ''; return this.webViewImpl.webviewNode.getAttribute(this.name) || '';
}; };
// Sets the attribute's value.
WebViewAttribute.prototype.setValue = function(value) { WebViewAttribute.prototype.setValue = function(value) {
this.value = value; this.webViewImpl.webviewNode.setAttribute(this.name, value || '');
}; };
// Called when the attribute's value changes.
WebViewAttribute.prototype.handleMutation = function() {}
// Attribute specifying whether transparency is allowed in the webview.
function BooleanAttribute(name, webViewImpl) {
WebViewAttribute.call(this, name, webViewImpl);
}
BooleanAttribute.prototype = new WebViewAttribute();
BooleanAttribute.prototype.getValue = function() {
// This attribute is treated as a boolean, and is retrieved as such.
return this.webViewImpl.webviewNode.hasAttribute(this.name);
}
BooleanAttribute.prototype.setValue = function(value) {
if (!value) {
this.webViewImpl.webviewNode.removeAttribute(this.name);
} else {
this.webViewImpl.webviewNode.setAttribute(this.name, '');
}
}
// Attribute representing the state of the storage partition. // Attribute representing the state of the storage partition.
function Partition(webViewImpl) { function Partition(webViewImpl) {
WebViewAttribute.call(this,
WebViewConstants.ATTRIBUTE_PARTITION,
webViewImpl);
this.validPartitionId = true; this.validPartitionId = true;
this.persistStorage = false;
this.storagePartitionId = '';
this.webViewImpl = webViewImpl;
} }
Partition.prototype = new WebViewAttribute( Partition.prototype = new WebViewAttribute();
WebViewConstants.ATTRIBUTE_PARTITION);
Partition.prototype.getValue = function() { Partition.prototype.handleMutation = function(oldValue, newValue) {
if (!this.validPartitionId) { newValue = newValue || '';
return '';
}
return (this.persistStorage ? 'persist:' : '') + this.storagePartitionId;
};
Partition.prototype.setValue = function(value) { // The partition cannot change if the webview has already navigated.
var result = {}; if (!this.webViewImpl.beforeFirstNavigation) {
var hasNavigated = !this.webViewImpl.beforeFirstNavigation; window.console.error(WebViewConstants.ERROR_MSG_ALREADY_NAVIGATED);
if (hasNavigated) { this.ignoreNextMutation = true;
result.error = WebViewConstants.ERROR_MSG_ALREADY_NAVIGATED; this.webViewImpl.webviewNode.setAttribute(this.name, oldValue);
return result; return;
}
if (!value) {
value = '';
} }
if (newValue == 'persist:') {
var LEN = 'persist:'.length; this.validPartitionId = false;
if (value.substr(0, LEN) == 'persist:') { window.console.error(
value = value.substr(LEN); WebViewConstants.ERROR_MSG_INVALID_PARTITION_ATTRIBUTE);
if (!value) {
this.validPartitionId = false;
result.error = WebViewConstants.ERROR_MSG_INVALID_PARTITION_ATTRIBUTE;
return result;
}
this.persistStorage = true;
} else {
this.persistStorage = false;
} }
}
this.storagePartitionId = value;
return result;
};
// ----------------------------------------------------------------------------- // -----------------------------------------------------------------------------
...@@ -79,12 +85,14 @@ WebView.prototype.setupWebViewAttributes = function() { ...@@ -79,12 +85,14 @@ WebView.prototype.setupWebViewAttributes = function() {
// Initialize the attributes with special behavior (and custom attribute // Initialize the attributes with special behavior (and custom attribute
// objects). // objects).
this.attributes[WebViewConstants.ATTRIBUTE_ALLOWTRANSPARENCY] =
new BooleanAttribute(WebViewConstants.ATTRIBUTE_ALLOWTRANSPARENCY, this);
this.attributes[WebViewConstants.ATTRIBUTE_AUTOSIZE] =
new BooleanAttribute(WebViewConstants.ATTRIBUTE_AUTOSIZE, this);
this.attributes[WebViewConstants.ATTRIBUTE_PARTITION] = new Partition(this); this.attributes[WebViewConstants.ATTRIBUTE_PARTITION] = new Partition(this);
// Initialize the remaining attributes, which have default behavior. // Initialize the remaining attributes, which have default behavior.
var defaultAttributes = [WebViewConstants.ATTRIBUTE_ALLOWTRANSPARENCY, var defaultAttributes = [WebViewConstants.ATTRIBUTE_MAXHEIGHT,
WebViewConstants.ATTRIBUTE_AUTOSIZE,
WebViewConstants.ATTRIBUTE_MAXHEIGHT,
WebViewConstants.ATTRIBUTE_MAXWIDTH, WebViewConstants.ATTRIBUTE_MAXWIDTH,
WebViewConstants.ATTRIBUTE_MINHEIGHT, WebViewConstants.ATTRIBUTE_MINHEIGHT,
WebViewConstants.ATTRIBUTE_MINWIDTH, WebViewConstants.ATTRIBUTE_MINWIDTH,
......
extensions/test/data/web_view/apitest/main.js
View file @ abb035a0
...@@ -71,15 +71,22 @@ embedder.test.succeed = function() { ...@@ -71,15 +71,22 @@ embedder.test.succeed = function() {
function testAllowTransparencyAttribute() { function testAllowTransparencyAttribute() {
var webview = document.createElement('webview'); var webview = document.createElement('webview');
webview.src = 'data:text/html,webview test'; webview.src = 'data:text/html,webview test';
embedder.test.assertFalse(webview.hasAttribute('allowtransparency'));
embedder.test.assertFalse(webview.allowtransparency);
webview.allowtransparency = true; webview.allowtransparency = true;
webview.addEventListener('loadstop', function(e) { webview.addEventListener('loadstop', function(e) {
embedder.test.assertTrue(webview.hasAttribute('allowtransparency')); embedder.test.assertTrue(webview.hasAttribute('allowtransparency'));
webview.allowtransparency = false;
embedder.test.assertTrue(webview.allowtransparency); embedder.test.assertTrue(webview.allowtransparency);
embedder.test.assertTrue(webview.hasAttribute('allowtransparency')); webview.allowtransparency = false;
webview.removeAttribute('allowtransparency'); embedder.test.assertFalse(webview.hasAttribute('allowtransparency'));
embedder.test.assertFalse(webview.allowtransparency); embedder.test.assertFalse(webview.allowtransparency);
webview.allowtransparency = '';
embedder.test.assertFalse(webview.hasAttribute('allowtransparency'));
embedder.test.assertFalse(webview.allowtransparency);
webview.allowtransparency = 'some string';
embedder.test.assertTrue(webview.hasAttribute('allowtransparency'));
embedder.test.assertTrue(webview.allowtransparency);
embedder.test.succeed(); embedder.test.succeed();
}); });
...@@ -1282,21 +1289,17 @@ function testOnEventProperties() { ...@@ -1282,21 +1289,17 @@ function testOnEventProperties() {
document.body.appendChild(webview); document.body.appendChild(webview);
} }
// This test verifies that setting the partition attribute after the src has // This test verifies that the partion attribute cannot be changed after the src
// been set raises an exception. // has been set.
function testPartitionRaisesException() { function testPartitionChangeAfterNavigation() {
var webview = document.createElement('webview'); var webview = document.createElement('webview');
var partitionAttribute = arguments.callee.name; var partitionAttribute = arguments.callee.name;
webview.setAttribute('partition', partitionAttribute); webview.setAttribute('partition', partitionAttribute);
var loadstopHandler = function(e) { var loadstopHandler = function(e) {
try { webview.partition = 'illegal';
webview.partition = 'illegal'; embedder.test.assertEq(partitionAttribute, webview.partition);
embedder.test.fail(); embedder.test.succeed();
} catch (e) {
embedder.test.assertEq(partitionAttribute, webview.partition);
embedder.test.succeed();
}
}; };
webview.addEventListener('loadstop', loadstopHandler); webview.addEventListener('loadstop', loadstopHandler);
...@@ -1707,7 +1710,7 @@ embedder.test.testList = { ...@@ -1707,7 +1710,7 @@ embedder.test.testList = {
'testNewWindowNoReferrerLink': testNewWindowNoReferrerLink, 'testNewWindowNoReferrerLink': testNewWindowNoReferrerLink,
'testNewWindowTwoListeners': testNewWindowTwoListeners, 'testNewWindowTwoListeners': testNewWindowTwoListeners,
'testOnEventProperties': testOnEventProperties, 'testOnEventProperties': testOnEventProperties,
'testPartitionRaisesException': testPartitionRaisesException, 'testPartitionChangeAfterNavigation': testPartitionChangeAfterNavigation,
'testPartitionRemovalAfterNavigationFails': 'testPartitionRemovalAfterNavigationFails':
testPartitionRemovalAfterNavigationFails, testPartitionRemovalAfterNavigationFails,
'testReassignSrcAttribute': testReassignSrcAttribute, 'testReassignSrcAttribute': testReassignSrcAttribute,
......
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