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

Issue 315143003: Call InitOriginPermissions when extension is added to the renderer process. (Closed)

Created:
6 years, 6 months ago by kouhei (in TOK)
Modified:
6 years, 6 months ago
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org, haraken
Visibility:
Public.

Description

Call InitOriginPermissions when extension is added to the renderer process. This is to fix permission breakage on extension background pages after blink patch r173044. Before this patch, permission settings were loaded on V8 context initialize. However, the blink patch deferred the V8 context initialize until its first use. This patch adds a call to |Dispatcher::InitOriginPermissions| on |Dispatcher::OnActivateExtension| which is called when render process for the extension is created. TEST=Manually confirmed that Files.app image viewer can load images on its first load. BUG=380346, 380502 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=276585

Patch Set 1 #

Total comments: 1

Patch Set 2 : remove context_type from param #

Patch Set 3 : avoid duplicated init #

Total comments: 4

Patch Set 4 : address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -2 lines) Patch
M extensions/renderer/dispatcher.cc View 1 2 3 3 chunks +7 lines, -2 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
kouhei (in TOK)
kalman, hirono: Would you take a look? I'd like to add a test if possible. ...
6 years, 6 months ago (2014-06-05 07:41:23 UTC) #1
not at google - send to devlin
as for testing, I would try to reduce the bug into a test case. https://codereview.chromium.org/315143003/diff/1/extensions/renderer/dispatcher.cc ...
6 years, 6 months ago (2014-06-05 16:48:23 UTC) #2
kouhei (in TOK)
Thanks for your comments! On 2014/06/05 16:48:23, kalman wrote: > as for testing, I would ...
6 years, 6 months ago (2014-06-06 05:41:31 UTC) #3
not at google - send to devlin
> Removing this lead to regression of > https://code.google.com/p/chromium/issues/detail?id=302548 so it looks like we > ...
6 years, 6 months ago (2014-06-06 19:12:55 UTC) #4
kouhei (in TOK)
On 2014/06/06 19:12:55, kalman wrote: > > Removing this lead to regression of > > ...
6 years, 6 months ago (2014-06-09 04:56:56 UTC) #5
kouhei (in TOK)
kalman: PTAL > > > Separated this change to CL: https://codereview.chromium.org/315413004/ > > > > ...
6 years, 6 months ago (2014-06-11 01:00:22 UTC) #6
not at google - send to devlin
some changes needed, but other than that lgtm. https://codereview.chromium.org/315143003/diff/40001/extensions/renderer/dispatcher.cc File extensions/renderer/dispatcher.cc (right): https://codereview.chromium.org/315143003/diff/40001/extensions/renderer/dispatcher.cc#newcode263 extensions/renderer/dispatcher.cc:263: if ...
6 years, 6 months ago (2014-06-11 14:41:16 UTC) #7
kouhei (in TOK)
Thanks for the review! https://codereview.chromium.org/315143003/diff/40001/extensions/renderer/dispatcher.cc File extensions/renderer/dispatcher.cc (right): https://codereview.chromium.org/315143003/diff/40001/extensions/renderer/dispatcher.cc#newcode263 extensions/renderer/dispatcher.cc:263: if (!extension && IsExtensionActive(extension->id())) On ...
6 years, 6 months ago (2014-06-12 01:07:38 UTC) #8
kouhei (in TOK)
The CQ bit was checked by kouhei@chromium.org
6 years, 6 months ago (2014-06-12 01:07:41 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kouhei@chromium.org/315143003/60001
6 years, 6 months ago (2014-06-12 01:08:50 UTC) #10
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_dbg_triggered_tests on tryserver.chromium ...
6 years, 6 months ago (2014-06-12 07:14:33 UTC) #11
commit-bot: I haz the power
6 years, 6 months ago (2014-06-12 08:34:41 UTC) #12
Message was sent while issue was closed.
Change committed as 276585

Powered by Google App Engine
This is Rietveld 408576698