Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(444)

Issue 11548020: Use element.createShadowRoot since WebKitShadowRoot is deprecated. (Closed)

Created:
8 years ago by hayato
Modified:
8 years ago
CC:
chromium-reviews, melevin, samarth, gideonwald, sreeram, dominich, Aaron Boodman, David Black, chromium-apps-reviews_chromium.org, darin-cc_chromium.org, Jered
Visibility:
Public.

Description

Use element.createShadowRoot() since WebKitShadowRoot constructor is deprecated and will be removed soon in WebKit. BUG=165641 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173412

Patch Set 1 #

Total comments: 6

Patch Set 2 : remove the shadow_dom api test #

Patch Set 3 : protect webkitCreateShadowRoot #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -53 lines) Patch
D chrome/browser/extensions/shadow_dom_apitest.cc View 1 1 chunk +0 lines, -15 lines 0 comments Download
M chrome/chrome_tests.gypi View 1 1 chunk +0 lines, -1 line 0 comments Download
M chrome/renderer/resources/extensions/inject_app_titlebar.js View 1 chunk +1 line, -1 line 0 comments Download
M chrome/renderer/resources/extensions/searchbox_api.js View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/renderer/resources/extensions/web_view.js View 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/data/extensions/api_test/shadow_dom/background.js View 1 1 chunk +0 lines, -16 lines 0 comments Download
D chrome/test/data/extensions/api_test/shadow_dom/empty.html View 1 1 chunk +0 lines, -8 lines 0 comments Download
D chrome/test/data/extensions/api_test/shadow_dom/manifest.json View 1 1 chunk +0 lines, -9 lines 0 comments Download

Messages

Total messages: 20 (0 generated)
hayato
It turned out that WebKitShadowRoot constructor is still used in the chromium side. https://bugs.webkit.org/show_bug.cgi?id=104770 After ...
8 years ago (2012-12-12 09:30:43 UTC) #1
dglazkov
LGTM.
8 years ago (2012-12-12 15:21:26 UTC) #2
dglazkov
https://codereview.chromium.org/11548020/diff/1/chrome/renderer/resources/extensions/searchbox_api.js File chrome/renderer/resources/extensions/searchbox_api.js (right): https://codereview.chromium.org/11548020/diff/1/chrome/renderer/resources/extensions/searchbox_api.js#newcode60 chrome/renderer/resources/extensions/searchbox_api.js:60: var nodeShadow = node.webkitCreateShadowRoot(); This is no longer "safe" ...
8 years ago (2012-12-12 15:22:14 UTC) #3
not at google - send to devlin
https://codereview.chromium.org/11548020/diff/1/chrome/test/data/extensions/api_test/shadow_dom/background.js File chrome/test/data/extensions/api_test/shadow_dom/background.js (right): https://codereview.chromium.org/11548020/diff/1/chrome/test/data/extensions/api_test/shadow_dom/background.js#newcode6 chrome/test/data/extensions/api_test/shadow_dom/background.js:6: function createShadowRootIsAvailableInElement() { when would this test fail?
8 years ago (2012-12-12 16:47:28 UTC) #4
dougw
lgtm https://codereview.chromium.org/11548020/diff/1/chrome/test/data/extensions/api_test/shadow_dom/background.js File chrome/test/data/extensions/api_test/shadow_dom/background.js (right): https://codereview.chromium.org/11548020/diff/1/chrome/test/data/extensions/api_test/shadow_dom/background.js#newcode6 chrome/test/data/extensions/api_test/shadow_dom/background.js:6: function createShadowRootIsAvailableInElement() { On 2012/12/12 16:47:29, kalman wrote: ...
8 years ago (2012-12-12 21:37:28 UTC) #5
not at google - send to devlin
https://codereview.chromium.org/11548020/diff/1/chrome/test/data/extensions/api_test/shadow_dom/background.js File chrome/test/data/extensions/api_test/shadow_dom/background.js (right): https://codereview.chromium.org/11548020/diff/1/chrome/test/data/extensions/api_test/shadow_dom/background.js#newcode6 chrome/test/data/extensions/api_test/shadow_dom/background.js:6: function createShadowRootIsAvailableInElement() { On 2012/12/12 21:37:29, dougw wrote: > ...
8 years ago (2012-12-12 21:45:37 UTC) #6
hayato
https://codereview.chromium.org/11548020/diff/1/chrome/renderer/resources/extensions/searchbox_api.js File chrome/renderer/resources/extensions/searchbox_api.js (right): https://codereview.chromium.org/11548020/diff/1/chrome/renderer/resources/extensions/searchbox_api.js#newcode60 chrome/renderer/resources/extensions/searchbox_api.js:60: var nodeShadow = node.webkitCreateShadowRoot(); Yeah, I have no idea ...
8 years ago (2012-12-13 01:09:20 UTC) #7
hayato
On 2012/12/12 21:45:37, kalman wrote: > https://codereview.chromium.org/11548020/diff/1/chrome/test/data/extensions/api_test/shadow_dom/background.js > File chrome/test/data/extensions/api_test/shadow_dom/background.js (right): > > https://codereview.chromium.org/11548020/diff/1/chrome/test/data/extensions/api_test/shadow_dom/background.js#newcode6 > ...
8 years ago (2012-12-13 01:19:52 UTC) #8
not at google - send to devlin
On 2012/12/13 01:19:52, hayato wrote: > On 2012/12/12 21:45:37, kalman wrote: > > > https://codereview.chromium.org/11548020/diff/1/chrome/test/data/extensions/api_test/shadow_dom/background.js ...
8 years ago (2012-12-13 01:55:11 UTC) #9
hayato
On 2012/12/13 01:55:11, kalman wrote: > On 2012/12/13 01:19:52, hayato wrote: > > On 2012/12/12 ...
8 years ago (2012-12-13 02:06:53 UTC) #10
not at google - send to devlin
lgtm after test is deleted
8 years ago (2012-12-13 02:29:24 UTC) #11
hayato
+jeremycho to reviewers. We need to know how searchbox_api.js should be modified. On 2012/12/13 01:09:20, ...
8 years ago (2012-12-13 03:55:30 UTC) #12
David Black
https://codereview.chromium.org/11548020/diff/1/chrome/renderer/resources/extensions/searchbox_api.js File chrome/renderer/resources/extensions/searchbox_api.js (right): https://codereview.chromium.org/11548020/diff/1/chrome/renderer/resources/extensions/searchbox_api.js#newcode60 chrome/renderer/resources/extensions/searchbox_api.js:60: var nodeShadow = node.webkitCreateShadowRoot(); Above in the searchBoxOnWindowReady function, ...
8 years ago (2012-12-13 04:31:08 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/hayato@chromium.org/11548020/12002
8 years ago (2012-12-14 05:10:25 UTC) #14
hayato
On 2012/12/14 05:10:25, I haz the power (commit-bot) wrote: > CQ is trying da patch. ...
8 years ago (2012-12-14 05:18:04 UTC) #15
David Black
LGTM - thanks!
8 years ago (2012-12-14 05:25:03 UTC) #16
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) content_browsertests
8 years ago (2012-12-14 07:00:48 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/hayato@chromium.org/11548020/12002
8 years ago (2012-12-14 07:39:44 UTC) #18
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) browser_tests
8 years ago (2012-12-14 12:23:00 UTC) #19
commit-bot: I haz the power
8 years ago (2012-12-17 02:20:18 UTC) #20

Powered by Google App Engine
This is Rietveld 408576698