Chromium Code Reviews Chromium Code Reviews
Issue
2505753003:
Revert of Fix three causes of blank thumbnails. (Closed)
DescriptionRevert 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
Committed: https://crrev.com/8004ebdebfca6a9d02ebb4d059f3ffc4dafa646f
Cr-Commit-Position: refs/heads/master@{#432330}
Patch Set 1 #
Messages
Total messages: 7 (3 generated)
|
|||||||||||||||||||||||||||||||||||||
