Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2281263003:
Use AuthorizedInstall to move Chrome into place and set up permissions (Closed)
DescriptionUse AuthorizedInstall to move Chrome into place and set up permissions
Committed: https://crrev.com/eba3e4e9951a7e5577326001531070076d3f2869
Cr-Commit-Position: refs/heads/master@{#415306}
Patch Set 1 #Patch Set 2 : Bridge pointer bug fixes, integration of AuthorizedInstall #
Total comments: 2
Patch Set 3 : Edited according to updated dependency CL #
Depends on Patchset: Messages
Total messages: 19 (8 generated)
Description was changed from ========== edited before submitting added in attempt at verification, should fix this! sidney's suggestion!! work on this! fixed sidney's comments! got rid of the rest of the comments mostly fixed some bugs and such filepath used in tests, added arc, changed cfbundleexe name, took out extractor, hide window after launch, removed semaphore from unpacker code, wrote clean up code to remove temporary directory regardless of time of exit, removed checksum from hdiutil task made test not depend on real google chrome dmg brushed up on a testing detail got rid of useless item thing fixed pre-cl warning fixed the rest of the comments!! fixed up a couple things changed an error of sorts? also fixed test file merging cl5_working to new master wonder why maindelegate showed up again. deleted! redoing a cl thing lol added a comment made the application url a property -- and then also attempted nstemporarydirectory moved moving the dmg into unpacker it works it works! omg! and look at all these todos.... added comments and moved download path out of downloader added saved paths as an import for nstemporarydirectory content took advice from sidney addressing some of sidney's comments cleaned up for submitting to code review small edit again so it seems like copyfile will not work able to test and check if disk is there added a test to take care of test-like things happening added launch app eliminated comments fixed formattin fixed runloop problem by switching to dispatch queue! changed way download path is called cleaned up a bit for cr moved eject to callback fix the unsuccessful mounting unmounting works but ejecting does not fixed unpacker to work now! add ivans code added unpacker BUG= ========== to ========== Bridge pointer fixes, added AuthorizedInstall and Unpacker together ==========
zengster@google.com changed reviewers: + ellyjones@chromium.org, ivanhernandez@google.com, mark@chromium.org, sdy@chromium.org
Looking for review!
https://codereview.chromium.org/2281263003/diff/20001/chrome/installer/mac/ap... File chrome/installer/mac/app/Unpacker.m (right): https://codereview.chromium.org/2281263003/diff/20001/chrome/installer/mac/ap... chrome/installer/mac/app/Unpacker.m:10: #include <Security/Security.h> Move this #include to the other file.
Description was changed from ========== Bridge pointer fixes, added AuthorizedInstall and Unpacker together ========== to ========== Added AuthorizedInstall and Unpacker together ==========
https://codereview.chromium.org/2281263003/diff/20001/chrome/installer/mac/ap... File chrome/installer/mac/app/Unpacker.m (right): https://codereview.chromium.org/2281263003/diff/20001/chrome/installer/mac/ap... chrome/installer/mac/app/Unpacker.m:10: #include <Security/Security.h> On 2016/08/29 18:51:25, Mark Mentovai wrote: > Move this #include to the other file. Done.
Patch set 3 has lost Unpacker. Can you check this?
On 2016/08/29 22:02:28, Mark Mentovai wrote: > Patch set 3 has lost Unpacker. Can you check this? Checked. It seems that I had to make pretty much the same changes in this CL to the dependent CL (the main unpacker CL) in order to get rid of the segfault showing up earlier. It made sense to simply transition those changes to the other CL so it would actually pass the trybots.
OK, then you should update the CL description, and once you’ve done that, LGTM
Description was changed from ========== Added AuthorizedInstall and Unpacker together ========== to ========== Use AuthorizedInstall to move Chrome into place and set up permissions ==========
The CQ bit was checked by zengster@google.com
The CQ bit was unchecked by commit-bot@chromium.org
This CL has an open dependency (Issue 2203583002 Patch 360001). Please resolve the dependency and try again. If you are sure that there is no real dependency, please use one of the options listed in https://goo.gl/9Es4OR to land the CL.
The CQ bit was checked by zengster@google.com
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.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Use AuthorizedInstall to move Chrome into place and set up permissions ========== to ========== Use AuthorizedInstall to move Chrome into place and set up permissions Committed: https://crrev.com/eba3e4e9951a7e5577326001531070076d3f2869 Cr-Commit-Position: refs/heads/master@{#415306} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/eba3e4e9951a7e5577326001531070076d3f2869 Cr-Commit-Position: refs/heads/master@{#415306} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
