Fix three causes of blank thumbnails.

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}

[aelias_OOO_until_Jul13, 2016-11-15 04:21:25]

aelias@chromium.org changed reviewers:
+ tedchoc@chromium.org

[aelias_OOO_until_Jul13, 2016-11-15 04:21:36]

aelias@chromium.org changed reviewers:
+ aelias@chromium.org

[aelias_OOO_until_Jul13, 2016-11-15 04:21:36]

Hi tedchoc@, PTAL.

[Ted C, 2016-11-15 17:18:41]

lgtm

[aelias_OOO_until_Jul13, 2016-11-15 20:37:19]

The CQ bit was checked by aelias@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-15 20:38:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-15 20:53:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-15 20:53:18]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[aelias_OOO_until_Jul13, 2016-11-16 00:31:54]

Description was changed from

==========
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.

BUG=636630,640561,659459,646336
==========

to

==========
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
==========

[aelias_OOO_until_Jul13, 2016-11-16 00:31:59]

The CQ bit was checked by aelias@chromium.org

[commit-bot: I haz the power, 2016-11-16 00:33:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-16 00:43:21]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-16 00:45:33]

Description was changed from

==========
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
==========

to

==========
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}
==========

[commit-bot: I haz the power, 2016-11-16 00:45:34]

Patchset 1 (id:??) landed as
https://crrev.com/834bda0d8d9e1f244539e898b211542ba061db25
Cr-Commit-Position: refs/heads/master@{#432322}

[horo, 2016-11-16 01:13:14]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2505753003/ by horo@chromium.org.

The reason for reverting is: Caused compile error.
https://build.chromium.org/p/chromium.linux/builders/Cast%20Android%20%28dbg%...


[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.
.

[findit-for-me, 2016-11-16 01:37:03]

FYI: Findit identified this CL at revision 432322 as the culprit for
failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb...