Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(9)

Issue 9703039: Extensions: Don't pass the selected unpacked extension path from JS to C++ (Closed)

Created:
6 years, 10 months ago by James Hawkins
Modified:
6 years, 10 months ago
CC:
chromium-reviews, Aaron Boodman, arv (Not doing code reviews), mihaip+watch_chromium.org, DO NOT USE (jschuh), Chris Evans, abarth-chromium
Visibility:
Public.

Description

Extensions: Don't pass the selected unpacked extension path from JS to C++ handler. This is an unecessary hoop to jump through since the C++ is the original callback of the file dialog. BUG=117715 TEST=none TBR=sky R=csilv,aa Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=128801

Patch Set 1 #

Total comments: 8

Patch Set 2 : Review fixes. #

Total comments: 4

Patch Set 3 : Rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+468 lines, -443 lines) Patch
M chrome/browser/resources/extensions/extensions.js View 1 chunk +3 lines, -20 lines 0 comments Download
M chrome/browser/resources/extensions/pack_extension_overlay.js View 1 2 chunks +9 lines, -9 lines 0 comments Download
M chrome/browser/ui/select_file_dialog.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/webui/extensions/extension_settings_handler.h View 1 2 4 chunks +27 lines, -38 lines 0 comments Download
M chrome/browser/ui/webui/extensions/extension_settings_handler.cc View 1 2 9 chunks +323 lines, -361 lines 0 comments Download
M chrome/browser/ui/webui/extensions/pack_extension_handler.h View 1 2 3 chunks +23 lines, -3 lines 0 comments Download
M chrome/browser/ui/webui/extensions/pack_extension_handler.cc View 1 2 5 chunks +75 lines, -6 lines 0 comments Download
M chrome/browser/ui/webui/options2/options_ui2.cc View 1 2 1 chunk +7 lines, -5 lines 0 comments Download

Messages

Total messages: 19 (0 generated)
James Hawkins
6 years, 10 months ago (2012-03-14 22:25:52 UTC) #1
James Hawkins
+cc: jschuh, abarth, cevans
6 years, 10 months ago (2012-03-14 22:39:34 UTC) #2
Aaron Boodman
LGTM, but would be easier to review if you made the cleanup change separately. http://codereview.chromium.org/9703039/diff/1/chrome/browser/ui/webui/options/extension_settings_handler.h ...
6 years, 10 months ago (2012-03-14 23:12:44 UTC) #3
Evan Stade
http://codereview.chromium.org/9703039/diff/1/chrome/browser/resources/extensions/extensions.js File chrome/browser/resources/extensions/extensions.js (right): http://codereview.chromium.org/9703039/diff/1/chrome/browser/resources/extensions/extensions.js#newcode77 chrome/browser/resources/extensions/extensions.js:77: // TODO(jhawkins): Refactor metrics support out of options and ...
6 years, 10 months ago (2012-03-14 23:13:45 UTC) #4
James Hawkins
On Wed, Mar 14, 2012 at 4:12 PM, <aa@chromium.org> wrote: > LGTM, but would be ...
6 years, 10 months ago (2012-03-14 23:15:06 UTC) #5
csilv
lgtm http://codereview.chromium.org/9703039/diff/1/chrome/browser/ui/webui/options/extension_settings_handler.cc File chrome/browser/ui/webui/options/extension_settings_handler.cc (right): http://codereview.chromium.org/9703039/diff/1/chrome/browser/ui/webui/options/extension_settings_handler.cc#newcode635 chrome/browser/ui/webui/options/extension_settings_handler.cc:635: int file_type_index = 0; You can remove file_type_index, ...
6 years, 10 months ago (2012-03-14 23:20:19 UTC) #6
James Hawkins
http://codereview.chromium.org/9703039/diff/1/chrome/browser/resources/extensions/extensions.js File chrome/browser/resources/extensions/extensions.js (right): http://codereview.chromium.org/9703039/diff/1/chrome/browser/resources/extensions/extensions.js#newcode77 chrome/browser/resources/extensions/extensions.js:77: // TODO(jhawkins): Refactor metrics support out of options and ...
6 years, 10 months ago (2012-03-15 00:24:13 UTC) #7
James Hawkins
estade: ping
6 years, 10 months ago (2012-03-15 16:28:07 UTC) #8
James Hawkins
estade: I'm checking in, will reply to any further review comments in a follow-up CL.
6 years, 10 months ago (2012-03-15 22:18:38 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jhawkins@chromium.org/9703039/4003
6 years, 10 months ago (2012-03-20 18:38:39 UTC) #10
commit-bot: I haz the power
Can't apply patch for file chrome/browser/ui/webui/options/extension_settings_handler.cc. While running patch -p1 --forward --force; patching file chrome/browser/ui/webui/options/extension_settings_handler.cc ...
6 years, 10 months ago (2012-03-20 18:38:52 UTC) #11
Evan Stade
lgtm http://codereview.chromium.org/9703039/diff/4003/chrome/browser/ui/webui/options/pack_extension_handler.cc File chrome/browser/ui/webui/options/pack_extension_handler.cc (right): http://codereview.chromium.org/9703039/diff/4003/chrome/browser/ui/webui/options/pack_extension_handler.cc#newcode115 chrome/browser/ui/webui/options/pack_extension_handler.cc:115: !args->GetString(1, &private_key_path_)) curlies
6 years, 10 months ago (2012-03-20 19:56:07 UTC) #12
James Hawkins
http://codereview.chromium.org/9703039/diff/4003/chrome/browser/ui/webui/options/pack_extension_handler.cc File chrome/browser/ui/webui/options/pack_extension_handler.cc (right): http://codereview.chromium.org/9703039/diff/4003/chrome/browser/ui/webui/options/pack_extension_handler.cc#newcode115 chrome/browser/ui/webui/options/pack_extension_handler.cc:115: !args->GetString(1, &private_key_path_)) On 2012/03/20 19:56:07, Evan Stade wrote: > ...
6 years, 10 months ago (2012-03-23 21:49:38 UTC) #13
Evan Stade
http://codereview.chromium.org/9703039/diff/4003/chrome/browser/ui/webui/options/pack_extension_handler.cc File chrome/browser/ui/webui/options/pack_extension_handler.cc (right): http://codereview.chromium.org/9703039/diff/4003/chrome/browser/ui/webui/options/pack_extension_handler.cc#newcode115 chrome/browser/ui/webui/options/pack_extension_handler.cc:115: !args->GetString(1, &private_key_path_)) On 2012/03/23 21:49:38, James Hawkins wrote: > ...
6 years, 10 months ago (2012-03-23 21:54:09 UTC) #14
James Hawkins
http://codereview.chromium.org/9703039/diff/4003/chrome/browser/ui/webui/options/pack_extension_handler.cc File chrome/browser/ui/webui/options/pack_extension_handler.cc (right): http://codereview.chromium.org/9703039/diff/4003/chrome/browser/ui/webui/options/pack_extension_handler.cc#newcode115 chrome/browser/ui/webui/options/pack_extension_handler.cc:115: !args->GetString(1, &private_key_path_)) On 2012/03/23 21:54:09, Evan Stade wrote: > ...
6 years, 10 months ago (2012-03-23 22:04:33 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jhawkins@chromium.org/9703039/12001
6 years, 10 months ago (2012-03-24 23:55:11 UTC) #16
commit-bot: I haz the power
Presubmit check for 9703039-12001 failed and returned exit status 1. Running presubmit commit checks ...
6 years, 10 months ago (2012-03-24 23:55:17 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jhawkins@chromium.org/9703039/12001
6 years, 10 months ago (2012-03-25 00:12:05 UTC) #18
commit-bot: I haz the power
6 years, 10 months ago (2012-03-25 01:49:04 UTC) #19
Change committed as 128801

Powered by Google App Engine
This is Rietveld 408576698