Extension APIs replace duplicate objects (in args) with null |
||
Issue description
Chrome version: 50.0.2661.75
Steps to reproduce:
1. Open the console in the context of an extension that uses the storage API (a content script is also good).
2. Run the following snippet
var v = {};
chrome.storage.local.set({x: [v, v]}, function() {
chrome.storage.local.get('x', function(items) {
console.log(items.x);
chrome.storage.local.remove('x'); // Just to clean our mess.
});
});
3. Look at the console output.
Expected result:
[Object, Object]
Actual result:
[Object, null]
More information:
This is caused by the naive counter-measure against circular references * in V8ValueConverterImpl ( bug 139933 ). It saves the identity of an object in a map, and skips objects that are in the map... A better way to counter circular references is to use depth-first-search and keep the duplicates on a stack.
* a circular reference is an object like this: v = {}; v.key = v;
,
May 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/275ec3e2a5ce1d16fc9c2a0dab3b58f8a7b9fa38 commit 275ec3e2a5ce1d16fc9c2a0dab3b58f8a7b9fa38 Author: lazyboy <lazyboy@chromium.org> Date: Wed May 04 18:04:24 2016 Properly detect cycles in V8ValueConverter, current impl is too strict. Currently, just reusing a value in extension API would null out the second occurrence of that value. Ideally, we only need to null out a value if it introduces a cycle. Examples of cycle: 1) var v = {}, v = {key: v}; 2) var a = [], a = [1, a]; However, these are not cycles and this CL will correctly detect them as non-cycles: 1) var v = {}, w = {keyA: v, keyB: v}; 2) var a = [], b = [a, a] BUG= 606955 Test=See steps in OP https://crbug.com/detail?id=606955 Review-Url: https://codereview.chromium.org/1939233002 Cr-Commit-Position: refs/heads/master@{#391559} [modify] https://crrev.com/275ec3e2a5ce1d16fc9c2a0dab3b58f8a7b9fa38/chrome/browser/extensions/api/declarative_content/declarative_content_apitest.cc [modify] https://crrev.com/275ec3e2a5ce1d16fc9c2a0dab3b58f8a7b9fa38/content/child/v8_value_converter_impl.cc [modify] https://crrev.com/275ec3e2a5ce1d16fc9c2a0dab3b58f8a7b9fa38/content/child/v8_value_converter_impl.h [modify] https://crrev.com/275ec3e2a5ce1d16fc9c2a0dab3b58f8a7b9fa38/content/child/v8_value_converter_impl_unittest.cc
,
May 9 2016
Verified fixed in 52.0.2730.0 |
||
►
Sign in to add a comment |
||
Comment 1 by lazyboy@chromium.org
, Apr 29 2016Status: Available (was: Untriaged)
Summary: Extension APIs replace duplicate objects (in args) with null (was: chrome.storage replaces duplicate objects with null)