Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2393793002:
Mac: Port deprecated methods to 10.8+ alternatives in install_from_dmg.mm. (Closed)
DescriptionMac: Port deprecated methods to 10.8+ alternatives in install_from_dmg.mm.
FSPathMoveObjectToTrashSync and FNNotifyByPath are deprecated. Replace the
former with an equivalent NSFileManager trashItemAtURL:: call, which doesn't
have the problem of not notifying the trash can to update its icon (if
previously) empty. This means there is no need to call FNNotifyByPath to update
the icon manually, so delete it.
BUG=650796
Committed: https://crrev.com/680f9d60ee6c44ef2677d869860017c2350fab63
Cr-Commit-Position: refs/heads/master@{#424109}
Patch Set 1 #Patch Set 2 : Add to forward declarations. #Patch Set 3 : Delete incorrect & unneeded sys_string_conversations_mac.mm import. #
Total comments: 2
Patch Set 4 : Print out the returned error message on trashing failure. #
Messages
Total messages: 29 (20 generated)
The CQ bit was checked by patricialor@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
The CQ bit was checked by patricialor@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by patricialor@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
patricialor@chromium.org changed reviewers: + tapted@chromium.org
Hi Trent, PTAL - it's a CL from Wednesday that I was finally able to get tested after making a static build :)
https://codereview.chromium.org/2393793002/diff/40001/chrome/browser/mac/inst... File chrome/browser/mac/install_from_dmg.mm (right): https://codereview.chromium.org/2393793002/diff/40001/chrome/browser/mac/inst... chrome/browser/mac/install_from_dmg.mm:687: error:nil]) { We're probably not interested in resultingItemURL, but we should capture the NSError and include it in the log message (OSSTATUS_LOG would have included a description of the error as well). Below, probably just LOG(ERROR) << base::SysNSStringToUTF8([ns_error localizedDescription]);
The CQ bit was checked by patricialor@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2393793002/diff/40001/chrome/browser/mac/inst... File chrome/browser/mac/install_from_dmg.mm (right): https://codereview.chromium.org/2393793002/diff/40001/chrome/browser/mac/inst... chrome/browser/mac/install_from_dmg.mm:687: error:nil]) { On 2016/10/07 04:33:15, tapted wrote: > We're probably not interested in resultingItemURL, but we should capture the > NSError and include it in the log message (OSSTATUS_LOG would have included a > description of the error as well). > > Below, probably just > > LOG(ERROR) << base::SysNSStringToUTF8([ns_error localizedDescription]); Done.
lgtm
patricialor@chromium.org changed reviewers: + rsesek@chromium.org
Hi rsesek@, PTAL! Thanks.
lgtm
The CQ bit was checked by patricialor@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.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Mac: Port deprecated methods to 10.8+ alternatives in install_from_dmg.mm. FSPathMoveObjectToTrashSync and FNNotifyByPath are deprecated. Replace the former with an equivalent NSFileManager trashItemAtURL:: call, which doesn't have the problem of not notifying the trash can to update its icon (if previously) empty. This means there is no need to call FNNotifyByPath to update the icon manually, so delete it. BUG=650796 ========== to ========== Mac: Port deprecated methods to 10.8+ alternatives in install_from_dmg.mm. FSPathMoveObjectToTrashSync and FNNotifyByPath are deprecated. Replace the former with an equivalent NSFileManager trashItemAtURL:: call, which doesn't have the problem of not notifying the trash can to update its icon (if previously) empty. This means there is no need to call FNNotifyByPath to update the icon manually, so delete it. BUG=650796 Committed: https://crrev.com/680f9d60ee6c44ef2677d869860017c2350fab63 Cr-Commit-Position: refs/heads/master@{#424109} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/680f9d60ee6c44ef2677d869860017c2350fab63 Cr-Commit-Position: refs/heads/master@{#424109} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
