Android: Capture a placeholder for the current foreground tab when the app goes to the background |
|||||||||||||||||
Issue descriptionCapturing a placeholder at this time will make sure we have something to show when resuming the activity until we get a frame from the renderer. There are a bunch of quirks with achieving this as we have to make sure any pending readback requests are completed which races with the context being torn down. We also want to make sure we keep the bitmap around and neither upload the bitmap (and *move* it to GPU memory) before the context goes away. Also need to avoid triggering Thumbnail eviction which currently happens when the context goes away. Oh, and blocked on 636628 since we only allow one pending readback and that bug can mess with initiating and completing a second one on activity pause/stop.
,
Aug 11 2016
,
Aug 11 2016
Specific to Android. The goal is to help with glitches when resuming the activity, esp. on Android N, by making sure we always have an up-to-date placeholder ready when we wait for the renderer to reraster the contest (for which we freed the tiles).
,
Aug 11 2016
s/contest/content
,
Aug 11 2016
,
Aug 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/962aa721a0de39986c7c393e154f773f7b74bdf4 commit 962aa721a0de39986c7c393e154f773f7b74bdf4 Author: sievers <sievers@chromium.org> Date: Fri Aug 12 18:26:14 2016 Android: Force draw for pending readbacks during Display teardown The Surface goes away when the activity is stopped. Since this is an interesting point in time to grab a placeholder for the web contents, try to complete pending readbacks that might have just been issues before tearing down the Display. BUG= 636630 ,637035 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Review-Url: https://codereview.chromium.org/2242633002 Cr-Commit-Position: refs/heads/master@{#411706} [modify] https://crrev.com/962aa721a0de39986c7c393e154f773f7b74bdf4/cc/surfaces/display.cc [modify] https://crrev.com/962aa721a0de39986c7c393e154f773f7b74bdf4/cc/surfaces/display.h [modify] https://crrev.com/962aa721a0de39986c7c393e154f773f7b74bdf4/content/browser/renderer_host/compositor_impl_android.cc [modify] https://crrev.com/962aa721a0de39986c7c393e154f773f7b74bdf4/content/browser/renderer_host/compositor_impl_android.h
,
Aug 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/df099f06101f2b01d5aa39d96661ab77ad4d6621 commit df099f06101f2b01d5aa39d96661ab77ad4d6621 Author: sievers <sievers@chromium.org> Date: Fri Aug 26 19:31:31 2016 Android: Fix Thumbnail CreateSmallHolderBitmap() Need to alloc bitmap before creating canvas or it will create an empty bitmap instead of one in WHITE. BUG= 636630 Review-Url: https://codereview.chromium.org/2238173003 Cr-Commit-Position: refs/heads/master@{#414778} [modify] https://crrev.com/df099f06101f2b01d5aa39d96661ab77ad4d6621/chrome/browser/android/thumbnail/thumbnail.cc
,
Aug 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8afa2d49f71b2a3ce36709ba69610d7d1bf85262 commit 8afa2d49f71b2a3ce36709ba69610d7d1bf85262 Author: sievers <sievers@chromium.org> Date: Fri Aug 26 19:45:15 2016 cc: Don't upload UI resources twice after eviction This can happen if you create a new UI resource (through LTH) which queues a request while there are any evicted resources (as tracked by LTHImpl). RecreateUIResources() will then iterate over all existing client ids and queue another CREATE request. Simply check for duplicates before queuing more requests. The double-upload behavior didn't play well with Android's Thumbnail class which drops the bitmap after every UIResourceClient::GetBitmap() call. BUG= 636630 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Review-Url: https://codereview.chromium.org/2247573002 Cr-Commit-Position: refs/heads/master@{#414785} [modify] https://crrev.com/8afa2d49f71b2a3ce36709ba69610d7d1bf85262/cc/test/fake_scoped_ui_resource.cc [modify] https://crrev.com/8afa2d49f71b2a3ce36709ba69610d7d1bf85262/cc/test/fake_scoped_ui_resource.h [modify] https://crrev.com/8afa2d49f71b2a3ce36709ba69610d7d1bf85262/cc/trees/layer_tree_host.cc [modify] https://crrev.com/8afa2d49f71b2a3ce36709ba69610d7d1bf85262/cc/trees/layer_tree_host_unittest_context.cc
,
Aug 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1d3bd9ca2dee42d4908ebe358f98df12c6aca327 commit 1d3bd9ca2dee42d4908ebe358f98df12c6aca327 Author: sievers <sievers@chromium.org> Date: Fri Aug 26 19:58:06 2016 Android: Don't evict thumbnails for last visible tab on stop When the activity stops (or rather the context goes away and resident textures incl. UI resources are evicted), don't evict the thumbnails for the tab that was visible *if* the bitmap is still available and wasn't uploaded to a texture yet. Also, don't force the UIResource to be created proactively when the ETC1 compression completes *if* there are no running activities, since it's racy with uploading it into the old context that's going away, while throwing away the bitmap. When there's critical memory pressure, free all thumbnails. BUG= 636630 Review-Url: https://codereview.chromium.org/2244783005 Cr-Commit-Position: refs/heads/master@{#414789} [modify] https://crrev.com/1d3bd9ca2dee42d4908ebe358f98df12c6aca327/chrome/browser/android/thumbnail/scoped_ptr_expiring_cache.h [modify] https://crrev.com/1d3bd9ca2dee42d4908ebe358f98df12c6aca327/chrome/browser/android/thumbnail/thumbnail_cache.cc [modify] https://crrev.com/1d3bd9ca2dee42d4908ebe358f98df12c6aca327/chrome/browser/android/thumbnail/thumbnail_cache.h [modify] https://crrev.com/1d3bd9ca2dee42d4908ebe358f98df12c6aca327/content/browser/renderer_host/render_widget_host_view_android.cc
,
Aug 29 2016
,
Sep 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/be54d2fa0b7a766b0391de082eebeeee53888397 commit be54d2fa0b7a766b0391de082eebeeee53888397 Author: mustaq <mustaq@chromium.org> Date: Thu Sep 01 17:44:45 2016 Revert of Android: Trigger tab placeholder update when activity is paused (patchset #3 id:40001 of https://codereview.chromium.org/2186453004/ ) Reason for revert: Seeing lots of memory regression since this landed. Bisects point to this CL with high confidence. See crbug.com/641962 . Original issue's description: > Android: Trigger tab placeholder update when activity is paused > > This makes sure we have a placeholder bitmap when we resume > and don't have a frame from the renderer yet. > > BUG= 636630 > > Committed: https://crrev.com/0a1f48d5cb6a5d263f85bc5322912b7f703c40ac > Cr-Commit-Position: refs/heads/master@{#414822} TBR=dtrainor@chromium.org,sievers@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 636630 , 641962 Review-Url: https://codereview.chromium.org/2306623002 Cr-Commit-Position: refs/heads/master@{#415990} [modify] https://crrev.com/be54d2fa0b7a766b0391de082eebeeee53888397/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
,
Sep 15 2016
,
Sep 27 2016
,
Sep 27 2016
,
Sep 27 2016
,
Sep 27 2016
,
Sep 28 2016
We would like to see this be addressed in M54 if possible. Raise to P1.
,
Sep 28 2016
The CL got re-landed on 421390 and, unfortunately, again appears to have caused some 2MB regressions on overall pss and ashmem. https://chrome-health.googleplex.com/health-plan/android-chrome/memory/nexus5/?from_commit=421108&to_commit=421431
,
Sep 28 2016
Is the memory increase transient? Reading the CL description, we are getting a new snapshot. I assume we will release the current snapshot after we successfully capture a new one.
,
Sep 28 2016
[Bulk edit] This issue is listed as a release block stable for M54 Android. We'll be cutting our stable candidate in just about two weeks, so time is running out to fix this bug - please prioritize working on it ASAP. Are you sure this issue shouldn't block the release? Remove the ReleaseBlock-Stable label. Unsure if this issue should block the release, or know the issue should block the release but we won't be able to fix it in time? CC me so that we can discuss. Thanks!
,
Sep 28 2016
See 641962. This is ashmem which we always allocate lazily for readbacks. It just happens that the simple test case never triggered a readback before. I had landed an improvement (https://codereview.chromium.org/2336043004/) yesterday to actually free this ashmem region when there is memory or we go to the background (note that this would be a real world improvement regardless). It worked in my local testing, but still doesn't seem to make the test happy. So I'll need to take another look.
,
Sep 28 2016
> when there is memory + pressure
,
Sep 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5d34220f9b827cfd514e03ba6215286f2467f29d commit 5d34220f9b827cfd514e03ba6215286f2467f29d Author: sievers <sievers@chromium.org> Date: Tue Sep 27 23:59:31 2016 Reland of Android: Trigger tab placeholder update when activity is paused (patchset #1 id:1 of https://codereview.chromium.org/2306623002/ ) Reason for reland (revert-of-revert): Now that https://codereview.chromium.org/2336043004/ has landed, we will free GL context-related ashmem after the readback and when the app goes to the background. Original issue's description: > Revert of Android: Trigger tab placeholder update when activity is paused (patchset #3 id:40001 of https://codereview.chromium.org/2186453004/ ) > > Reason for revert: > Seeing lots of memory regression since this landed. Bisects point to this CL with high confidence. See crbug.com/641962 . > > Original issue's description: > > Android: Trigger tab placeholder update when activity is paused > > > > This makes sure we have a placeholder bitmap when we resume > > and don't have a frame from the renderer yet. > > > > BUG= 636630 > > > > Committed: https://crrev.com/0a1f48d5cb6a5d263f85bc5322912b7f703c40ac > > Cr-Commit-Position: refs/heads/master@{#414822} > > TBR=dtrainor@chromium.org,sievers@chromium.org > # Not skipping CQ checks because original CL landed more than 1 days ago. > BUG= 636630 , 641962 > > Committed: https://crrev.com/be54d2fa0b7a766b0391de082eebeeee53888397 > Cr-Commit-Position: refs/heads/master@{#415990} TBR=dtrainor@chromium.org,primiano@chromium.org,mustaq@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 636630 , 641962 Review-Url: https://codereview.chromium.org/2372393002 Cr-Commit-Position: refs/heads/master@{#421390} [modify] https://crrev.com/5d34220f9b827cfd514e03ba6215286f2467f29d/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
,
Sep 28 2016
Re #21, we should try hard to make it for M54, per b/30345539. If it is not possible, we will release M54 without the fix.
,
Sep 28 2016
Ah I think I know what this might be. Because of the other blocker here (640561, which looks like a driver bug keeping us from doing commitless readbacks) the readbacks actually don't generally finish when you go to the homescreen. THerefore the ashmem is not 'unused' and got allocated but can't be freed.
,
Sep 28 2016
Re #25: That will need merging of: https://codereview.chromium.org/2242633002 https://codereview.chromium.org/2244783005 https://codereview.chromium.org/2247573002 https://codereview.chromium.org/2306623002 https://codereview.chromium.org/2336043004 (potentially, to make the test happy) finding a way to revert (the revert in) https://codereview.chromium.org/2282053003 (the alleged driver bug issue)
,
Sep 30 2016
Ok, figured out why the test was still passing (the ashmem wasn't actually getting freed). Will fix that tomorrow.
,
Sep 30 2016
> Ok, figured out why the test was still passing err *not* passing
,
Oct 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c9afe77bc5411c3bf0760c9acc37941e6e24540a commit c9afe77bc5411c3bf0760c9acc37941e6e24540a Author: sievers <sievers@chromium.org> Date: Wed Oct 05 01:42:52 2016 Android: Second attempt at freeing GLHelper ashmem This tears down the GLHelperHolder in RenderWidgetHostViewAndroid when all activities get stopped. But it only does this if there are no more readback requests in flight. BUG= 641962 , 636630 Review-Url: https://codereview.chromium.org/2383613005 Cr-Commit-Position: refs/heads/master@{#423045} [modify] https://crrev.com/c9afe77bc5411c3bf0760c9acc37941e6e24540a/content/browser/renderer_host/render_widget_host_view_android.cc [modify] https://crrev.com/c9afe77bc5411c3bf0760c9acc37941e6e24540a/content/browser/renderer_host/render_widget_host_view_android.h
,
Oct 5 2016
I split off http://crbug.com/653249 for a safer M54 workaround. Keeping this bug to track the fix in 55.
,
Oct 5 2016
,
Nov 14 2016
,
Nov 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/834bda0d8d9e1f244539e898b211542ba061db25 commit 834bda0d8d9e1f244539e898b211542ba061db25 Author: aelias <aelias@google.com> Date: Wed Nov 16 00:42:51 2016 Fix three causes of blank thumbnails. All three of these fixes are required to address the devil case http://crbug.com/659459 , and some of them also help with some of the other flickers. 1. In onPageLoadStarted, the URL can be the same as before, in particular because history-item scroll offset restores count as load starts. These history restores often fire late in page load, *after* the thumbnail was taken, leading to immediately throwing out the thumbnail we just took earlier in page load. Switch to the "invalidate" path which only ejects the thumbnail if the URL changed. 2. In CacheTab, which is often called just to refresh the thumbnail with the latest state, if readback is currently impossible, we used to remove the old thumbnail. Keep it instead. 3. Switch to the more reliable Surfaces-based mechanism for readback. The layer-based mechanism often outputs black screenshots in difficult racy cases like onStop readbacks. The Surface-based mechanism had been reverted because it tickles a driver bug on Nexus 7 2013 that itself causes some flickers, but I verified that the repro scenario for that is quite niche, and even on Nexus 7 it's probably less severe than the black screenshot problem this fixes. NOTRY=true BUG= 636630 ,640561, 659459 ,646336 Review-Url: https://codereview.chromium.org/2498253002 Cr-Commit-Position: refs/heads/master@{#432322} [modify] https://crrev.com/834bda0d8d9e1f244539e898b211542ba061db25/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorImpl.java [modify] https://crrev.com/834bda0d8d9e1f244539e898b211542ba061db25/chrome/browser/android/compositor/tab_content_manager.cc [modify] https://crrev.com/834bda0d8d9e1f244539e898b211542ba061db25/ui/android/delegated_frame_host_android.cc
,
Nov 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8004ebdebfca6a9d02ebb4d059f3ffc4dafa646f commit 8004ebdebfca6a9d02ebb4d059f3ffc4dafa646f Author: horo <horo@chromium.org> Date: Wed Nov 16 01:14:05 2016 Revert of Fix three causes of blank thumbnails. (patchset #1 id:1 of https://codereview.chromium.org/2498253002/ ) Reason for revert: Caused compile error. https://build.chromium.org/p/chromium.linux/builders/Cast%20Android%20%28dbg%29/builds/56195 [920/3963] CXX obj/ui/android/android/delegated_frame_host_android.o FAILED: obj/ui/android/android/delegated_frame_host_android.o /b/c/cipd/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/ui/android/android/delegated_frame_host_android.o.d -DUI_ANDROID_IMPLEMENTATION -DV8_DEPRECATION_WARNINGS -DENABLE_NOTIFICATIONS -DENABLE_PLUGINS=1 -DUSE_OPENSSL_CERTS=1 -DNO_TCMALLOC -DUSE_EXTERNAL_POPUP_MENU=1 -DENABLE_WEBRTC=1 -DDISABLE_NACL -DUSE_PROPRIETARY_CODECS -DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=284979-2 -D_FILE_OFFSET_BITS=64 -DANDROID -DHAVE_SYS_UIO_H -DANDROID_NDK_VERSION=r12b -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__GNU_SOURCE=1 -D__compiler_offsetof=__builtin_offsetof -Dnan=__builtin_nan -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DUSE_EGL -DDISABLE_FFMPEG_VIDEO_DECODERS -DSK_IGNORE_DW_GRAY_FIX -DSK_IGNORE_LINEONLY_AA_CONVEX_PATH_OPTS -DSK_SUPPORT_GPU=1 -DSK_BUILD_FOR_ANDROID -DUSE_CHROMIUM_SKIA -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -I../.. -Igen -I../../third_party/khronos -I../../gpu -Igen/ui/android/ui_android_jni_headers -Igen/ui/android/ui_android_jni_headers/ui_android -I../../skia/config -I../../skia/ext -I../../third_party/skia/include/c -I../../third_party/skia/include/config -I../../third_party/skia/include/core -I../../third_party/skia/include/effects -I../../third_party/skia/include/images -I../../third_party/skia/include/lazy -I../../third_party/skia/include/pathops -I../../third_party/skia/include/pdf -I../../third_party/skia/include/pipe -I../../third_party/skia/include/ports -I../../third_party/skia/include/utils -I../../third_party/skia/include/gpu -I../../third_party/skia/src/gpu -I../../third_party/skia/src/sksl -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -funwind-tables -fPIC -pipe -fcolor-diagnostics -ffunction-sections -fno-short-enums --target=arm-linux-androideabi -march=armv7-a -mfloat-abi=softfp -mthumb -mtune=generic-armv7-a -mfpu=neon -Wall -Werror -Wextra -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-deprecated-register -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-shift-negative-value -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Os -fno-omit-frame-pointer -gdwarf-3 -g1 --sysroot=../../third_party/android_tools/ndk/platforms/android-16/arch-arm -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang -add-plugin -Xclang find-bad-constructs -Wheader-hygiene -Wstring-conversion -fno-threadsafe-statics -fvisibility-inlines-hidden -Wno-undefined-bool-conversion -Wno-tautological-undefined-compare -std=gnu++11 -fno-rtti -isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include -isystem../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++abi/libcxxabi/include -isystem../../third_party/android_tools/ndk/sources/android/support/include -fno-exceptions -c ../../ui/android/delegated_frame_host_android.cc -o obj/ui/android/android/delegated_frame_host_android.o ../../ui/android/delegated_frame_host_android.cc:184:42: error: too many arguments to function call, expected single argument 'copy_request', have 2 arguments std::move(copy_output_request)); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../cc/surfaces/surface_factory.h:70:3: note: 'RequestCopyOfSurface' declared here void RequestCopyOfSurface(std::unique_ptr<CopyOutputRequest> copy_request); ^ 1 error generated. Original issue's description: > Fix three causes of blank thumbnails. > > All three of these fixes are required to address the devil case > http://crbug.com/659459 , and some of them also help with some of the > other flickers. > > 1. In onPageLoadStarted, the URL can be the same as before, in > particular because history-item scroll offset restores count as load > starts. These history restores often fire late in page load, *after* > the thumbnail was taken, leading to immediately throwing out the > thumbnail we just took earlier in page load. Switch to the "invalidate" > path which only ejects the thumbnail if the URL changed. > > 2. In CacheTab, which is often called just to refresh the thumbnail with > the latest state, if readback is currently impossible, we used to remove > the old thumbnail. Keep it instead. > > 3. Switch to the more reliable Surfaces-based mechanism for readback. > The layer-based mechanism often outputs black screenshots in difficult > racy cases like onStop readbacks. The Surface-based mechanism had been > reverted because it tickles a driver bug on Nexus 7 2013 that itself causes > some flickers, but I verified that the repro scenario for that is quite > niche, and even on Nexus 7 it's probably less severe than the black > screenshot problem this fixes. > > NOTRY=true > BUG= 636630 ,640561, 659459 ,646336 > > Committed: https://crrev.com/834bda0d8d9e1f244539e898b211542ba061db25 > Cr-Commit-Position: refs/heads/master@{#432322} TBR=aelias@chromium.org,tedchoc@chromium.org,aelias@google.com # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 636630 ,640561, 659459 ,646336 Review-Url: https://codereview.chromium.org/2505753003 Cr-Commit-Position: refs/heads/master@{#432330} [modify] https://crrev.com/8004ebdebfca6a9d02ebb4d059f3ffc4dafa646f/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorImpl.java [modify] https://crrev.com/8004ebdebfca6a9d02ebb4d059f3ffc4dafa646f/chrome/browser/android/compositor/tab_content_manager.cc [modify] https://crrev.com/8004ebdebfca6a9d02ebb4d059f3ffc4dafa646f/ui/android/delegated_frame_host_android.cc
,
Nov 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d9c930e47745312f5cc8a5b9d4e6802c18f8b5de commit d9c930e47745312f5cc8a5b9d4e6802c18f8b5de Author: aelias <aelias@chromium.org> Date: Wed Nov 16 06:48:22 2016 Fix three causes of blank thumbnails [2nd land] All three of these fixes are required to address the devil case http://crbug.com/659459 , and some of them also help with some of the other flickers. 1. In onPageLoadStarted, the URL can be the same as before, in particular because history-item scroll offset restores count as load starts. These history restores often fire late in page load, *after* the thumbnail was taken, leading to immediately throwing out the thumbnail we just took earlier in page load. Switch to the "invalidate" path which only ejects the thumbnail if the URL changed. 2. In CacheTab, which is often called just to refresh the thumbnail with the latest state, if readback is currently impossible, we used to remove the old thumbnail. Keep it instead. 3. Switch to the more reliable Surfaces-based mechanism for readback. The layer-based mechanism often outputs black screenshots in difficult racy cases like onStop readbacks. The Surface-based mechanism had been reverted because it tickles a driver bug on Nexus 7 2013 that itself causes some flickers, but I verified that the repro scenario for that is quite niche, and even on Nexus 7 it's probably less severe than the black screenshot problem this fixes. TBR=tedchoc@chromium.org BUG= 636630 ,640561, 659459 ,646336 Review-Url: https://codereview.chromium.org/2503183003 Cr-Commit-Position: refs/heads/master@{#432411} [modify] https://crrev.com/d9c930e47745312f5cc8a5b9d4e6802c18f8b5de/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorImpl.java [modify] https://crrev.com/d9c930e47745312f5cc8a5b9d4e6802c18f8b5de/chrome/browser/android/compositor/tab_content_manager.cc [modify] https://crrev.com/d9c930e47745312f5cc8a5b9d4e6802c18f8b5de/ui/android/delegated_frame_host_android.cc
,
Nov 16 2016
Removing 55-RBS because I'm requesting merge under 659459.
,
Nov 16 2016
,
Nov 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b27a7d7a86cfdc06c04ab3728e08ff10f786b975 commit b27a7d7a86cfdc06c04ab3728e08ff10f786b975 Author: Alexandre Elias <aelias@chromium.org> Date: Thu Nov 17 02:02:30 2016 Fix three causes of blank thumbnails [2nd land] All three of these fixes are required to address the devil case http://crbug.com/659459 , and some of them also help with some of the other flickers. 1. In onPageLoadStarted, the URL can be the same as before, in particular because history-item scroll offset restores count as load starts. These history restores often fire late in page load, *after* the thumbnail was taken, leading to immediately throwing out the thumbnail we just took earlier in page load. Switch to the "invalidate" path which only ejects the thumbnail if the URL changed. 2. In CacheTab, which is often called just to refresh the thumbnail with the latest state, if readback is currently impossible, we used to remove the old thumbnail. Keep it instead. 3. Switch to the more reliable Surfaces-based mechanism for readback. The layer-based mechanism often outputs black screenshots in difficult racy cases like onStop readbacks. The Surface-based mechanism had been reverted because it tickles a driver bug on Nexus 7 2013 that itself causes some flickers, but I verified that the repro scenario for that is quite niche, and even on Nexus 7 it's probably less severe than the black screenshot problem this fixes. TBR=tedchoc@chromium.org BUG= 636630 ,640561, 659459 ,646336 Review URL: https://codereview.chromium.org/2503363003 . Review-Url: https://codereview.chromium.org/2503183003 Cr-Original-Commit-Position: refs/heads/master@{#432411} Cr-Commit-Position: refs/branch-heads/2883@{#597} Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768} [modify] https://crrev.com/b27a7d7a86cfdc06c04ab3728e08ff10f786b975/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorImpl.java [modify] https://crrev.com/b27a7d7a86cfdc06c04ab3728e08ff10f786b975/chrome/browser/android/compositor/tab_content_manager.cc [modify] https://crrev.com/b27a7d7a86cfdc06c04ab3728e08ff10f786b975/ui/android/delegated_frame_host_android.cc |
|||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||
Comment 1 by siev...@chromium.org
, Aug 11 2016