Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2341963006:
[ios] Fixed some checkTypes errors in autofill_controller.js. (Closed)
|
Created:
4 years, 3 months ago by Eugene But (OOO till 7-30) Modified:
4 years, 3 months ago CC:
chromium-reviews, rouslan+autofill_chromium.org, estade+watch_chromium.org, vabr+watchlistautofill_chromium.org, browser-components-watch_chromium.org, jdonnelly+autofillwatch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[ios] Fixed some checkTypes errors in autofill_controller.js.
Moved typedefs out from anonymous function because typedefs can
only be defined at the top level, not inside functions.
R=thestig@chromium.org
BUG=487804
Committed: https://crrev.com/07193eeccb33be4f7e3fab9e1b6172462ff5eff5
Cr-Commit-Position: refs/heads/master@{#419423}
Patch Set 1 #Patch Set 2 : Removed using doc syntact from property creation #Messages
Total messages: 18 (9 generated)
eugenebut@chromium.org changed reviewers: + sdefresne@chromium.org
On 2016/09/15 22:56:04, Eugene But wrote: Q: wasn't the code using ['property'] syntax so that the name would not be optimised out by closure during compilation? If we use dot notation and enable variable renaming, then code won't be callable from C++, won't it?
The CQ bit was checked by eugenebut@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks for the explanation. All tests (including downstream translate integration tests work fine), so I guess closure does not optimize the name anymore.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== [ios] Fixed multiple checkTypes errors in ios scripts. Used dot syntax to add properties, so Closeure recognizes them later. Moved typedefs out of anonymous namespace. R=thestig@chromium.org BUG=487804 ========== to ========== [ios] Fixed some checkTypes errors in autofill_controller.js. Moved typedefs out from anonymous function because typedefs can only be defined at the top level, not inside functions. R=thestig@chromium.org BUG=487804 ==========
Ok, so this worked because we are not doing JS minification. I reverted doc syntax changes and will replace with subscript syntax instead in separate CLs. I think we still should have an option of turning on JS minification w/o rewriting all our scripts again. PTAL
lgtm
Thanks!
The CQ bit was checked by eugenebut@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [ios] Fixed some checkTypes errors in autofill_controller.js. Moved typedefs out from anonymous function because typedefs can only be defined at the top level, not inside functions. R=thestig@chromium.org BUG=487804 ========== to ========== [ios] Fixed some checkTypes errors in autofill_controller.js. Moved typedefs out from anonymous function because typedefs can only be defined at the top level, not inside functions. R=thestig@chromium.org BUG=487804 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [ios] Fixed some checkTypes errors in autofill_controller.js. Moved typedefs out from anonymous function because typedefs can only be defined at the top level, not inside functions. R=thestig@chromium.org BUG=487804 ========== to ========== [ios] Fixed some checkTypes errors in autofill_controller.js. Moved typedefs out from anonymous function because typedefs can only be defined at the top level, not inside functions. R=thestig@chromium.org BUG=487804 Committed: https://crrev.com/07193eeccb33be4f7e3fab9e1b6172462ff5eff5 Cr-Commit-Position: refs/heads/master@{#419423} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/07193eeccb33be4f7e3fab9e1b6172462ff5eff5 Cr-Commit-Position: refs/heads/master@{#419423} |
