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

Issue 2257273002: Fix extension bindings injection for iframes (reland) (Closed)

Created:
4 years, 4 months ago by asargent_no_longer_on_chrome
Modified:
4 years, 4 months ago
Reviewers:
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@2785
Target Ref:
refs/pending/branch-heads/2785
Project:
chromium
Visibility:
Public.

Description

Fix extension bindings injection for iframes (reland) For iframes, we don't want to use the source url for determining the associated extension because it starts out with an about:blank context that is scriptable by its parent. This originally landed in codereview.chromium.org/2151693002/ but was reverted because of bug 630928 as well as the test failing under DrMemory (not with memory errors; just not succeeding which likely indicates some kind of race condition in the test). I've added a fix for bug 630928 but haven't been able to locally reproduce the test failure under DrMemory, so I've added some extra logging to the test to hopefully better understand what might be going wrong. Memory sheriffs: If the FramesExtensionBindingsApiTest.FramesBeforeNavigation test fails again without any actual memory errors, please do not revert the entire CL (since it is an important security fix); instead just disable the test or add it to a suppression file so I can iterate on a fix. BUG=573131, 630928 Review-Url: https://codereview.chromium.org/2208483002 Cr-Commit-Position: refs/heads/master@{#409819} (cherry picked from commit 79b64c3e741cc9c6afbb23885945831a45c6baa5)

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+276 lines, -17 lines) Patch
M chrome/browser/extensions/extension_bindings_apitest.cc View 2 chunks +61 lines, -0 lines 0 comments Download
M chrome/browser/extensions/extension_messages_apitest.cc View 2 chunks +41 lines, -0 lines 0 comments Download
A chrome/test/data/extensions/api_test/bindings/external_message_listener/background.js View 1 chunk +32 lines, -0 lines 0 comments Download
A chrome/test/data/extensions/api_test/bindings/external_message_listener/manifest.json View 1 chunk +11 lines, -0 lines 0 comments Download
A chrome/test/data/extensions/api_test/bindings/frames_before_navigation.html View 1 chunk +77 lines, -0 lines 0 comments Download
A + chrome/test/data/extensions/api_test/bindings/message_sender/background.js View 1 chunk +2 lines, -2 lines 0 comments Download
A chrome/test/data/extensions/api_test/bindings/message_sender/manifest.json View 1 chunk +13 lines, -0 lines 0 comments Download
A + chrome/test/data/extensions/api_test/bindings/message_sender/public.html View 1 chunk +1 line, -1 line 0 comments Download
A + chrome/test/data/extensions/api_test/messaging/externally_connectable/sites/popup_opener.html View 1 chunk +4 lines, -9 lines 0 comments Download
M extensions/renderer/script_context.h View 2 chunks +8 lines, -0 lines 0 comments Download
M extensions/renderer/script_context.cc View 3 chunks +18 lines, -3 lines 0 comments Download
M extensions/renderer/script_context_set.cc View 1 chunk +8 lines, -2 lines 0 comments Download

Messages

Total messages: 1 (0 generated)
asargent_no_longer_on_chrome
4 years, 4 months ago (2016-08-18 21:59:52 UTC) #1
Message was sent while issue was closed.
Committed patchset #1 (id:1) to pending queue manually as
14c1b8936d451938bb361fb9c81dc4c569b563ad.

Powered by Google App Engine
This is Rietveld 408576698