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

Issue 12319073: Delete the extensions i18n message map when extension unload is received. (Closed)

Created:
7 years, 10 months ago by Patrick Riordan
Modified:
7 years, 9 months ago
Reviewers:
Devlin, Yoyo Zhou
CC:
chromium-reviews, Aaron Boodman, darin-cc_chromium.org, chromium-apps-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Delete the extensions i18n message map cache when extension unload is received. Invalidate the messages map for the extension in case the extension is reloaded with a new messages map. BUG=53628 TEST=Load an unpacked extension with i18n messages used in a content script. Open a window or tab that loads the i18n messages. Change the messages.json file. Reload the extension and the messages should be changed. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=188460

Patch Set 1 #

Total comments: 24

Patch Set 2 : Updated style #

Patch Set 3 : Fixed syntax errors #

Total comments: 2

Patch Set 4 : Changed to i18n_apitest and fixed up misc #

Total comments: 6

Patch Set 5 : #

Patch Set 6 : Took out base/file_path.h #

Patch Set 7 : Included base/files/file_path.h #

Patch Set 8 : Merge #

Patch Set 9 : Fixed "", windows should be happy now #

Total comments: 14

Patch Set 10 : Now uses ScopedTempDir #

Patch Set 11 : Merge #

Patch Set 12 : Added License to contentscript.js #

Patch Set 13 : Merge AUTHORS #

Unified diffs Side-by-side diffs Delta from patch set Stats (+94 lines, -2 lines) Patch
M AUTHORS View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/api/i18n/i18n_apitest.cc View 1 2 3 4 5 6 7 8 9 1 chunk +56 lines, -0 lines 0 comments Download
M chrome/common/extensions/message_bundle.h View 1 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/common/extensions/message_bundle.cc View 1 2 3 4 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/renderer/extensions/dispatcher.cc View 1 2 3 4 5 6 7 8 9 2 chunks +5 lines, -0 lines 0 comments Download
A chrome/test/data/extensions/api_test/i18nUpdate/_locales/en/messages.json View 1 2 3 4 5 6 7 8 9 1 chunk +5 lines, -0 lines 0 comments Download
A + chrome/test/data/extensions/api_test/i18nUpdate/contentscript.js View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -1 line 0 comments Download
A chrome/test/data/extensions/api_test/i18nUpdate/manifest.json View 1 2 3 4 5 6 7 8 9 1 chunk +12 lines, -0 lines 0 comments Download
A chrome/test/data/extensions/api_test/i18nUpdate/messages2.json View 1 2 3 4 5 6 7 8 9 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 24 (0 generated)
Devlin
https://codereview.chromium.org/12319073/diff/1/chrome/browser/extensions/content_script_apitest.cc File chrome/browser/extensions/content_script_apitest.cc (right): https://codereview.chromium.org/12319073/diff/1/chrome/browser/extensions/content_script_apitest.cc#newcode180 chrome/browser/extensions/content_script_apitest.cc:180: { Bracket on same line as function name. https://codereview.chromium.org/12319073/diff/1/chrome/browser/extensions/content_script_apitest.cc#newcode182 ...
7 years, 10 months ago (2013-02-22 19:59:07 UTC) #1
Patrick Riordan
Fixed up style. https://codereview.chromium.org/12319073/diff/1/chrome/browser/extensions/content_script_apitest.cc File chrome/browser/extensions/content_script_apitest.cc (right): https://codereview.chromium.org/12319073/diff/1/chrome/browser/extensions/content_script_apitest.cc#newcode180 chrome/browser/extensions/content_script_apitest.cc:180: { On 2013/02/22 19:59:07, D Cronin ...
7 years, 10 months ago (2013-02-26 05:05:20 UTC) #2
Devlin
Two other things: 1) If there's no message_reload2, there's no reason to call this message_reload1 ...
7 years, 10 months ago (2013-02-26 05:57:12 UTC) #3
Patrick Riordan
Changed message_reload1 to i18nUpdate I agree that looks like a much better place. Changed. https://codereview.chromium.org/12319073/diff/1/chrome/browser/extensions/content_script_apitest.cc ...
7 years, 9 months ago (2013-02-27 18:30:16 UTC) #4
Devlin
https://codereview.chromium.org/12319073/diff/11/chrome/browser/extensions/api/i18n/i18n_apitest.cc File chrome/browser/extensions/api/i18n/i18n_apitest.cc (right): https://codereview.chromium.org/12319073/diff/11/chrome/browser/extensions/api/i18n/i18n_apitest.cc#newcode3 chrome/browser/extensions/api/i18n/i18n_apitest.cc:3: // found in the LICENSE file. Newline after the ...
7 years, 9 months ago (2013-02-27 23:10:41 UTC) #5
Patrick Riordan
Patch 5 untitled on accident. Title should be Updated includes and fixed sytle. On 2013/02/27 ...
7 years, 9 months ago (2013-03-01 17:28:54 UTC) #6
Patrick Riordan
https://codereview.chromium.org/12319073/diff/11/chrome/browser/extensions/api/i18n/i18n_apitest.cc File chrome/browser/extensions/api/i18n/i18n_apitest.cc (right): https://codereview.chromium.org/12319073/diff/11/chrome/browser/extensions/api/i18n/i18n_apitest.cc#newcode3 chrome/browser/extensions/api/i18n/i18n_apitest.cc:3: // found in the LICENSE file. On 2013/02/27 23:10:41, ...
7 years, 9 months ago (2013-03-01 17:29:22 UTC) #7
Devlin
lgtm
7 years, 9 months ago (2013-03-04 17:07:15 UTC) #8
Patrick Riordan
On 2013/03/04 17:07:15, D Cronin wrote: > lgtm I took out #include base/file_path.h, it turns ...
7 years, 9 months ago (2013-03-04 18:44:00 UTC) #9
Patrick Riordan
7 years, 9 months ago (2013-03-04 18:49:08 UTC) #10
Devlin
Hey Yoyo, this is a CL from one of our new guys, Patrick. Mind taking ...
7 years, 9 months ago (2013-03-08 20:04:55 UTC) #11
Yoyo Zhou
Thanks for the patch. It looks good for the most part; the biggest thing to ...
7 years, 9 months ago (2013-03-08 22:17:50 UTC) #12
Patrick Riordan
https://codereview.chromium.org/12319073/diff/35001/AUTHORS File AUTHORS (right): https://codereview.chromium.org/12319073/diff/35001/AUTHORS#newcode158 AUTHORS:158: Patrick Riordan <patrickriordan177@gmail.com> On 2013/03/08 22:17:50, Yoyo Zhou wrote: ...
7 years, 9 months ago (2013-03-11 17:32:42 UTC) #13
Yoyo Zhou
LGTM minor nit: I would write the CL description as "Delete..." and "Invalidate..." instead of ...
7 years, 9 months ago (2013-03-11 19:44:25 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/12319073/46001
7 years, 9 months ago (2013-03-13 16:29:59 UTC) #15
commit-bot: I haz the power
Failed to apply patch for AUTHORS: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...
7 years, 9 months ago (2013-03-13 16:30:02 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/12319073/56001
7 years, 9 months ago (2013-03-13 19:13:09 UTC) #17
commit-bot: I haz the power
Presubmit check for 12319073-56001 failed and returned exit status 1. INFO:root:Found 9 file(s). Running presubmit ...
7 years, 9 months ago (2013-03-13 19:13:16 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/12319073/60001
7 years, 9 months ago (2013-03-13 19:25:58 UTC) #19
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
7 years, 9 months ago (2013-03-13 21:32:52 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/12319073/67001
7 years, 9 months ago (2013-03-15 17:30:13 UTC) #21
commit-bot: I haz the power
Retried try job too often on mac_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&number=109410
7 years, 9 months ago (2013-03-15 18:11:14 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/patrickriordan177@gmail.com/12319073/67001
7 years, 9 months ago (2013-03-15 19:02:34 UTC) #23
commit-bot: I haz the power
7 years, 9 months ago (2013-03-15 20:12:22 UTC) #24
Message was sent while issue was closed.
Change committed as 188460

Powered by Google App Engine
This is Rietveld 408576698