Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2825133002:
Ensure android:configChanges="uiMode" is handled for all ChromeActivitys (Closed)
DescriptionEnsure android:configChanges="uiMode" is handled for all ChromeActivitys
BUG=711713
Review-Url: https://codereview.chromium.org/2825133002
Cr-Commit-Position: refs/heads/master@{#465388}
Committed: https://chromium.googlesource.com/chromium/src/+/3a7aacd3d5a42bf2f8271e66f4eecff5deae7535
Patch Set 1 #
Total comments: 4
Patch Set 2 : add newline #
Total comments: 1
Messages
Total messages: 15 (7 generated)
mthiesse@chromium.org changed reviewers: + dfalcantara@chromium.org
PTAL. I've done some refactoring here to make future changes easier, can revert the refactoring if you don't like it.
Description was changed from ========== Ensure android:configChanges="uiMode" is handled for all ChromeActivitys BUG=711713 ========== to ========== Ensure android:configChanges="uiMode" is handled for all ChromeActivitys BUG=711713 ==========
lgtm https://codereview.chromium.org/2825133002/diff/1/chrome/android/java/Android... File chrome/android/java/AndroidManifest.xml (right): https://codereview.chromium.org/2825133002/diff/1/chrome/android/java/Android... chrome/android/java/AndroidManifest.xml:356: android:hardwareAccelerated="false" {% endblock %} endblock on newline?
https://codereview.chromium.org/2825133002/diff/1/chrome/android/java/Android... File chrome/android/java/AndroidManifest.xml (right): https://codereview.chromium.org/2825133002/diff/1/chrome/android/java/Android... chrome/android/java/AndroidManifest.xml:356: android:hardwareAccelerated="false" {% endblock %} On 2017/04/18 21:04:49, ping past 24hrs (dfalcantara) wrote: > endblock on newline? If I put the endblock on a newline we get extra blank lines in the generated output. Do we care? Will defer to your judgement on which way to go here.
https://codereview.chromium.org/2825133002/diff/1/chrome/android/java/Android... File chrome/android/java/AndroidManifest.xml (right): https://codereview.chromium.org/2825133002/diff/1/chrome/android/java/Android... chrome/android/java/AndroidManifest.xml:356: android:hardwareAccelerated="false" {% endblock %} On 2017/04/18 21:09:35, mthiesse wrote: > On 2017/04/18 21:04:49, ping past 24hrs (dfalcantara) wrote: > > endblock on newline? > > If I put the endblock on a newline we get extra blank lines in the generated > output. Do we care? > > Will defer to your judgement on which way to go here. We already do it on line 352, just above your new block.
https://codereview.chromium.org/2825133002/diff/1/chrome/android/java/Android... File chrome/android/java/AndroidManifest.xml (right): https://codereview.chromium.org/2825133002/diff/1/chrome/android/java/Android... chrome/android/java/AndroidManifest.xml:356: android:hardwareAccelerated="false" {% endblock %} On 2017/04/18 21:13:54, ping past 24hrs (dfalcantara) wrote: > On 2017/04/18 21:09:35, mthiesse wrote: > > On 2017/04/18 21:04:49, ping past 24hrs (dfalcantara) wrote: > > > endblock on newline? > > > > If I put the endblock on a newline we get extra blank lines in the generated > > output. Do we care? > > > > Will defer to your judgement on which way to go here. > > We already do it on line 352, just above your new block. Done.
The CQ bit was checked by mthiesse@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dfalcantara@chromium.org Link to the patchset: https://codereview.chromium.org/2825133002/#ps20001 (title: "add newline")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492550232954200,
"parent_rev": "25800fa49e1e6176b11bac60f20e031363e1bd97", "commit_rev":
"3a7aacd3d5a42bf2f8271e66f4eecff5deae7535"}
Message was sent while issue was closed.
Description was changed from ========== Ensure android:configChanges="uiMode" is handled for all ChromeActivitys BUG=711713 ========== to ========== Ensure android:configChanges="uiMode" is handled for all ChromeActivitys BUG=711713 Review-Url: https://codereview.chromium.org/2825133002 Cr-Commit-Position: refs/heads/master@{#465388} Committed: https://chromium.googlesource.com/chromium/src/+/3a7aacd3d5a42bf2f8271e66f4ee... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/3a7aacd3d5a42bf2f8271e66f4ee...
Message was sent while issue was closed.
peconn@chromium.org changed reviewers: + peconn@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/2825133002/diff/20001/chrome/android/java/And... File chrome/android/java/AndroidManifest.xml (left): https://codereview.chromium.org/2825133002/diff/20001/chrome/android/java/And... chrome/android/java/AndroidManifest.xml:376: {{ self.supports_video_persistence() }} I'm guessing you removed this line by accident? I'm adding it back in in https://codereview.chromium.org/2851433002. |
