Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1772843002:
Fix incremental install for Android N (Closed)
|
Created:
4 years, 9 months ago by agrieve Modified:
4 years, 9 months ago Reviewers:
Yaron CC:
chromium-reviews, jbudorick+watch_chromium.org, mikecase+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@depfile-minimize Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix incremental install for Android N
BUG=590394
Committed: https://crrev.com/38ac32e6609e58bb1530dc53b70118532d4a57a3
Cr-Commit-Position: refs/heads/master@{#379664}
Patch Set 1 #
Total comments: 2
Patch Set 2 : add comments #Patch Set 3 : rebase #
Depends on Patchset: Messages
Total messages: 16 (8 generated)
Description was changed from ========== Fix incremental install for Android N BUG=590394 ========== to ========== Fix incremental install for Android N BUG=590394 ==========
agrieve@chromium.org changed reviewers: + yfriedman@chromium.org
On 2016/03/07 19:14:55, agrieve wrote: > mailto:agrieve@chromium.org changed reviewers: > + mailto:yfriedman@chromium.org 🔶
https://codereview.chromium.org/1772843002/diff/1/build/android/incremental_i... File build/android/incremental_install/java/org/chromium/incrementalinstall/ClassLoaderPatcher.java (right): https://codereview.chromium.org/1772843002/diff/1/build/android/incremental_i... build/android/incremental_install/java/org/chromium/incrementalinstall/ClassLoaderPatcher.java:229: Reflect.concatArrays(curDexElements, curDexElements, new Object[files.length]); still seems cleaner to allocate a new array and then concat after
https://codereview.chromium.org/1772843002/diff/1/build/android/incremental_i... File build/android/incremental_install/java/org/chromium/incrementalinstall/ClassLoaderPatcher.java (right): https://codereview.chromium.org/1772843002/diff/1/build/android/incremental_i... build/android/incremental_install/java/org/chromium/incrementalinstall/ClassLoaderPatcher.java:229: Reflect.concatArrays(curDexElements, curDexElements, new Object[files.length]); On 2016/03/07 19:39:20, Yaron wrote: > still seems cleaner to allocate a new array and then concat after Added a comment: loadDexFile requires that ret contain all previously added elements.
On 2016/03/07 20:04:43, agrieve wrote: > https://codereview.chromium.org/1772843002/diff/1/build/android/incremental_i... > File > build/android/incremental_install/java/org/chromium/incrementalinstall/ClassLoaderPatcher.java > (right): > > https://codereview.chromium.org/1772843002/diff/1/build/android/incremental_i... > build/android/incremental_install/java/org/chromium/incrementalinstall/ClassLoaderPatcher.java:229: > Reflect.concatArrays(curDexElements, curDexElements, new Object[files.length]); > On 2016/03/07 19:39:20, Yaron wrote: > > still seems cleaner to allocate a new array and then concat after > > Added a comment: loadDexFile requires that ret contain all previously added > elements. lgtm - that explanation was helpful ;)
The CQ bit was checked by agrieve@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
This CL has an open dependency (Issue 1730993002 Patch 1). Please resolve the dependency and try again.
The CQ bit was checked by agrieve@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yfriedman@chromium.org Link to the patchset: https://codereview.chromium.org/1772843002/#ps30001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1772843002/30001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1772843002/30001
Message was sent while issue was closed.
Description was changed from ========== Fix incremental install for Android N BUG=590394 ========== to ========== Fix incremental install for Android N BUG=590394 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:30001)
Message was sent while issue was closed.
Description was changed from ========== Fix incremental install for Android N BUG=590394 ========== to ========== Fix incremental install for Android N BUG=590394 Committed: https://crrev.com/38ac32e6609e58bb1530dc53b70118532d4a57a3 Cr-Commit-Position: refs/heads/master@{#379664} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/38ac32e6609e58bb1530dc53b70118532d4a57a3 Cr-Commit-Position: refs/heads/master@{#379664} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
