Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2487713002:
Resolves layering violation in SynchronousCompositorHost creation (Closed)
|
Created:
4 years, 1 month ago by Jinsuk Kim Modified:
4 years, 1 month ago Reviewers:
boliu CC:
chromium-reviews, yusukes+watch_chromium.org, shuchen+watch_chromium.org, jam, nona+watch_chromium.org, darin-cc_chromium.org, James Su Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionResolves layering violation in SynchronousCompositorHost creation
SynchronousCompositorHost created in RWHVA has a reference to
WebContents, which violates the hierarchical access rule. This
CL resolves it by having WebContentsViewAndroid keep
SynchronousCompositorClient instance and pass it to RWHVA.
BUG=626765
Committed: https://crrev.com/8f60271614652b6110081128604470460af5dadb
Committed: https://crrev.com/b53b9955b59a6884ccf6f57ee906e37e2caac26f
Cr-Original-Commit-Position: refs/heads/master@{#431383}
Cr-Commit-Position: refs/heads/master@{#431565}
Patch Set 1 #
Total comments: 8
Patch Set 2 : comments #
Total comments: 2
Patch Set 3 : comments #Patch Set 4 : fixing tests #
Total comments: 4
Patch Set 5 : sc/cvc part ways #
Total comments: 2
Patch Set 6 : checking synchronous_compositor_client_ #Messages
Total messages: 70 (40 generated)
jinsukkim@chromium.org changed reviewers: + boliu@chromium.org
https://codereview.chromium.org/2487713002/diff/1/content/browser/web_content... File content/browser/web_contents/web_contents_view_android.cc (right): https://codereview.chromium.org/2487713002/diff/1/content/browser/web_content... content/browser/web_contents/web_contents_view_android.cc:93: SetSynchronousCompositorClientFor(rwhv); I don't think this needs to be tied to CVC events The only thing that matters is the order of "new RWHVA" vs SetClientForWebContents. Unfortunately the are allowed to happen in both orders (in particular, RWHVA is created first for pop ups) so only need to do this in CreateViewForWidget and in SetClientForWebContents https://codereview.chromium.org/2487713002/diff/1/content/browser/web_content... content/browser/web_contents/web_contents_view_android.cc:111: if (synchronous_compositor_client_) no need for this check https://codereview.chromium.org/2487713002/diff/1/content/browser/web_content... content/browser/web_contents/web_contents_view_android.cc:155: RenderWidgetHostViewAndroid* rwhv = static_cast<RenderWidgetHostViewAndroid*>( unrelated change
https://codereview.chromium.org/2487713002/diff/1/content/browser/android/syn... File content/browser/android/synchronous_compositor_host.cc (right): https://codereview.chromium.org/2487713002/diff/1/content/browser/android/syn... content/browser/android/synchronous_compositor_host.cc:16: #include "content/browser/web_contents/web_contents_android.h" can these two includes be removed now?
Patchset #2 (id:20001) has been deleted
https://codereview.chromium.org/2487713002/diff/1/content/browser/android/syn... File content/browser/android/synchronous_compositor_host.cc (right): https://codereview.chromium.org/2487713002/diff/1/content/browser/android/syn... content/browser/android/synchronous_compositor_host.cc:16: #include "content/browser/web_contents/web_contents_android.h" On 2016/11/08 16:50:47, boliu wrote: > can these two includes be removed now? Yes removed both. https://codereview.chromium.org/2487713002/diff/1/content/browser/web_content... File content/browser/web_contents/web_contents_view_android.cc (right): https://codereview.chromium.org/2487713002/diff/1/content/browser/web_content... content/browser/web_contents/web_contents_view_android.cc:93: SetSynchronousCompositorClientFor(rwhv); On 2016/11/08 16:50:13, boliu wrote: > I don't think this needs to be tied to CVC events > > The only thing that matters is the order of "new RWHVA" vs > SetClientForWebContents. Unfortunately the are allowed to happen in both orders > (in particular, RWHVA is created first for pop ups) > > so only need to do this in CreateViewForWidget and in SetClientForWebContents Good to know. Done. https://codereview.chromium.org/2487713002/diff/1/content/browser/web_content... content/browser/web_contents/web_contents_view_android.cc:111: if (synchronous_compositor_client_) On 2016/11/08 16:50:13, boliu wrote: > no need for this check Done. Inlined now that it became just one liner. https://codereview.chromium.org/2487713002/diff/1/content/browser/web_content... content/browser/web_contents/web_contents_view_android.cc:155: RenderWidgetHostViewAndroid* rwhv = static_cast<RenderWidgetHostViewAndroid*>( On 2016/11/08 16:50:13, boliu wrote: > unrelated change Oops.. reverted.
https://codereview.chromium.org/2487713002/diff/40001/content/browser/web_con... File content/browser/web_contents/web_contents_view_android.cc (right): https://codereview.chromium.org/2487713002/diff/40001/content/browser/web_con... content/browser/web_contents/web_contents_view_android.cc:59: RenderWidgetHostViewAndroid* rwhv = static_cast<RenderWidgetHostViewAndroid*>( probably needs null check
https://codereview.chromium.org/2487713002/diff/40001/content/browser/web_con... File content/browser/web_contents/web_contents_view_android.cc (right): https://codereview.chromium.org/2487713002/diff/40001/content/browser/web_con... content/browser/web_contents/web_contents_view_android.cc:59: RenderWidgetHostViewAndroid* rwhv = static_cast<RenderWidgetHostViewAndroid*>( On 2016/11/08 17:41:05, boliu wrote: > probably needs null check Done.
lgtm
The CQ bit was checked by jinsukkim@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
Patchset #4 (id:80001) has been deleted
The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...) android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
Patchset #4 (id:100001) has been deleted
The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by jinsukkim@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from boliu@chromium.org Link to the patchset: https://codereview.chromium.org/2487713002/#ps120001 (title: "fixing tests")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by boliu@chromium.org
this is a big enough additional change that it needs to be reviewed again https://codereview.chromium.org/2487713002/diff/120001/content/browser/render... File content/browser/renderer_host/render_widget_host_view_android.h (right): https://codereview.chromium.org/2487713002/diff/120001/content/browser/render... content/browser/renderer_host/render_widget_host_view_android.h:83: SynchronousCompositorClient* compositor_client); is the constructor arg required? can't we just construct and set immediately after?
https://codereview.chromium.org/2487713002/diff/120001/content/browser/render... File content/browser/renderer_host/render_widget_host_view_android.h (right): https://codereview.chromium.org/2487713002/diff/120001/content/browser/render... content/browser/renderer_host/render_widget_host_view_android.h:83: SynchronousCompositorClient* compositor_client); On 2016/11/10 17:52:34, boliu wrote: > is the constructor arg required? can't we just construct and set immediately > after? That's how it was done but it caused a problem. The constructor calls setContentViewCore() where it expects compositor client to be already set. Or I could pull setContentViewCore out of ctr but there may be other consequence I have to check.
https://codereview.chromium.org/2487713002/diff/120001/content/browser/render... File content/browser/renderer_host/render_widget_host_view_android.h (right): https://codereview.chromium.org/2487713002/diff/120001/content/browser/render... content/browser/renderer_host/render_widget_host_view_android.h:83: SynchronousCompositorClient* compositor_client); On 2016/11/10 17:55:15, Jinsuk Kim wrote: > On 2016/11/10 17:52:34, boliu wrote: > > is the constructor arg required? can't we just construct and set immediately > > after? > > That's how it was done but it caused a problem. The constructor calls > setContentViewCore() where it expects compositor client to be already set. > > Or I could pull setContentViewCore out of ctr but there may be other consequence > I have to check. > I don't think there is any more need to tie the lifetime of sync_compositor_ to whether there's a CVC. they are unrelated. it was just convenient to have a place to poll for the scclient I think with this change, you can just create compositor in the set call, if the client is not null, and then never destroy it
The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run
Patchset #5 (id:140001) has been deleted
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #5 (id:140001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2487713002/diff/120001/content/browser/render... File content/browser/renderer_host/render_widget_host_view_android.h (right): https://codereview.chromium.org/2487713002/diff/120001/content/browser/render... content/browser/renderer_host/render_widget_host_view_android.h:83: SynchronousCompositorClient* compositor_client); On 2016/11/10 17:59:21, boliu wrote: > On 2016/11/10 17:55:15, Jinsuk Kim wrote: > > On 2016/11/10 17:52:34, boliu wrote: > > > is the constructor arg required? can't we just construct and set immediately > > > after? > > > > That's how it was done but it caused a problem. The constructor calls > > setContentViewCore() where it expects compositor client to be already set. > > > > Or I could pull setContentViewCore out of ctr but there may be other > consequence > > I have to check. > > > > I don't think there is any more need to tie the lifetime of sync_compositor_ to > whether there's a CVC. they are unrelated. it was just convenient to have a > place to poll for the scclient > > I think with this change, you can just create compositor in the set call, if the > client is not null, and then never destroy it SG. Reverted.
https://codereview.chromium.org/2487713002/diff/160001/content/browser/render... File content/browser/renderer_host/render_widget_host_view_android.cc (right): https://codereview.chromium.org/2487713002/diff/160001/content/browser/render... content/browser/renderer_host/render_widget_host_view_android.cc:1212: if (!sync_compositor_) { || !synchronous_compositor_client_ too maybe? just to make it easier to read
The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2487713002/diff/160001/content/browser/render... File content/browser/renderer_host/render_widget_host_view_android.cc (right): https://codereview.chromium.org/2487713002/diff/160001/content/browser/render... content/browser/renderer_host/render_widget_host_view_android.cc:1212: if (!sync_compositor_) { On 2016/11/10 21:24:33, boliu wrote: > || !synchronous_compositor_client_ > > too maybe? just to make it easier to read Done.
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by jinsukkim@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #6 (id:180001)
Message was sent while issue was closed.
Description was changed from ========== Resolves layering violation in SynchronousCompositorHost creation SynchronousCompositorHost created in RWHVA has a reference to WebContents, which violates the hierarchical access rule. This CL resolves it by having WebContentsViewAndroid keep SynchronousCompositorClient instance and pass it to RWHVA. BUG=626765 ========== to ========== Resolves layering violation in SynchronousCompositorHost creation SynchronousCompositorHost created in RWHVA has a reference to WebContents, which violates the hierarchical access rule. This CL resolves it by having WebContentsViewAndroid keep SynchronousCompositorClient instance and pass it to RWHVA. BUG=626765 Committed: https://crrev.com/273cccbc60b358d06c332f64a8b3cc9f5ee42dd6 Cr-Commit-Position: refs/heads/master@{#431380} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/273cccbc60b358d06c332f64a8b3cc9f5ee42dd6 Cr-Commit-Position: refs/heads/master@{#431380}
Message was sent while issue was closed.
Description was changed from ========== Resolves layering violation in SynchronousCompositorHost creation SynchronousCompositorHost created in RWHVA has a reference to WebContents, which violates the hierarchical access rule. This CL resolves it by having WebContentsViewAndroid keep SynchronousCompositorClient instance and pass it to RWHVA. BUG=626765 Committed: https://crrev.com/273cccbc60b358d06c332f64a8b3cc9f5ee42dd6 Cr-Commit-Position: refs/heads/master@{#431380} ========== to ========== Resolves layering violation in SynchronousCompositorHost creation SynchronousCompositorHost created in RWHVA has a reference to WebContents, which violates the hierarchical access rule. This CL resolves it by having WebContentsViewAndroid keep SynchronousCompositorClient instance and pass it to RWHVA. BUG=626765 Committed: https://crrev.com/8f60271614652b6110081128604470460af5dadb Cr-Commit-Position: refs/heads/master@{#431383} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/8f60271614652b6110081128604470460af5dadb Cr-Commit-Position: refs/heads/master@{#431383}
Message was sent while issue was closed.
A revert of this CL (patchset #6 id:180001) has been created in https://codereview.chromium.org/2490383002/ by boliu@chromium.org. The reason for reverting is: Broke compile FAILED: obj/content/browser/browser/render_widget_host_view_android.o /b/c/cipd/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/content/browser/browser/render_widget_host_view_android.o.d -DENABLE_SCREEN_CAPTURE=1 -DAPPCACHE_USE_SIMPLE_CACHE -DUSE_MINIKIN_HYPHENATION=1 -DV8_DEPRECATION_WARNINGS -DENABLE_NOTIFICATIONS -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1 -DUSE_OPENSSL_CERTS=1 -DNO_TCMALLOC -DMEMORY_TOOL_REPLACES_ALLOCATOR -DMEMORY_SANITIZER_INITIAL_SIZE -DADDRESS_SANITIZER -DUSE_EXTERNAL_POPUP_MENU=1 -DENABLE_WEBRTC=1 -DDISABLE_NACL -DUSE_PROPRIETARY_CODECS -DSAFE_BROWSING_DB_REMOTE -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DENABLE_WEBVR -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=284979-1 -D_FILE_OFFSET_BITS=64 -DANDROID -DHAVE_SYS_UIO_H -DANDROID_NDK_VERSION=r12b -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -DCOMPONENT_BUILD -D__GNU_SOURCE=1 -D__compiler_offsetof=__builtin_offsetof -Dnan=__builtin_nan -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DCONTENT_IMPLEMENTATION -DV8_USE_EXTERNAL_STARTUP_DATA -DENABLE_MOJO_MEDIA -DENABLE_MOJO_CDM -DENABLE_MOJO_AUDIO_DECODER -DENABLE_MOJO_MEDIA_IN_GPU_PROCESS -DUSE_EGL -DDISABLE_FFMPEG_VIDEO_DECODERS -DLEVELDB_PLATFORM_CHROMIUM=1 -DSK_IGNORE_DW_GRAY_FIX -DSK_IGNORE_LINEONLY_AA_CONVEX_PATH_OPTS -DSKIA_DLL -DGR_GL_IGNORE_ES3_MSAA=0 -DSK_SUPPORT_GPU=1 -DSK_BUILD_FOR_ANDROID -DUSE_CHROMIUM_SKIA -DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DHAVE_PTHREAD -DPROTOBUF_USE_DLLS -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DMESA_EGL_NO_X11_HEADERS -DOPUS_FIXED_POINT -DBORINGSSL_SHARED_LIBRARY -DUSING_V8_SHARED -DUSING_V8_SHARED -DFEATURE_ENABLE_SSL -DFEATURE_ENABLE_VOICEMAIL -DEXPAT_RELATIVE_PATH -DGTEST_RELATIVE_PATH -DNO_MAIN_THREAD_WRAPPING -DNO_SOUND_SYSTEM -DWEBRTC_CHROMIUM_BUILD -DWEBRTC_POSIX -DWEBRTC_LINUX -DWEBRTC_ANDROID -DXML_STATIC -DSSL_USE_OPENSSL -DHAVE_OPENSSL_SSL_H -DFEATURE_ENABLE_SSL -DNO_MAIN_THREAD_WRAPPING -I../.. -Igen -I../../third_party/khronos -I../../gpu -I../../third_party/leveldatabase -I../../third_party/leveldatabase/src -I../../third_party/leveldatabase/src/include -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/protobuf/src -Igen/protoc_out -I../../third_party/protobuf/src -Igen/blink -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -I../../third_party/mesa/src/include -I../../third_party/libwebm/source -Igen/media/base/android/media_jni_headers -Igen/media/base/android/media_jni_headers/media -I../../third_party/opus/src/include -I../../third_party/boringssl/src/include -Igen -I../../third_party/ced/src -I../../third_party/WebKit -Igen/third_party/WebKit -I../../v8/include -Igen/v8/include -I../../v8/include -Igen/v8/include -I../../third_party/angle/src/common/third_party/numerics -Igen/angle -I../../third_party/libyuv/include -I../../third_party/re2/src -I../../third_party/zlib -Igen/ui/resources -Igen/ui/resources -I../../third_party/webrtc_overrides -I../../testing/gtest/include -I../../third_party -I../../third_party/webrtc_overrides -I../../third_party -I../../third_party/expat/files/lib -Igen/content/public/android/content_jni_headers -Igen/content/public/android/content_jni_headers/content -Igen/jar_jni/content -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 -gline-tables-only -gcolumn-info -fno-omit-frame-pointer -fsanitize=address -fsanitize-blacklist=../../tools/memory/asan/blacklist.txt -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang check-ipc -Wheader-hygiene -Wstring-conversion -Wexit-time-destructors -Wno-unused-function -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 ../../content/browser/renderer_host/render_widget_host_view_android.cc -o obj/content/browser/browser/render_widget_host_view_android.o ../../content/browser/renderer_host/render_widget_host_view_android.cc:1205:35: error: redefinition of 'SetSynchronousCompositorClient' void RenderWidgetHostViewAndroid::SetSynchronousCompositorClient( ^ ../../content/browser/renderer_host/render_widget_host_view_android.cc:1197:35: note: previous definition is here void RenderWidgetHostViewAndroid::SetSynchronousCompositorClient( ^ 1 error generated..
Message was sent while issue was closed.
wait, wat, this got committed twice? https://chromium.googlesource.com/chromium/src/+/273cccbc60b358d06c332f64a8b3... https://chromium.googlesource.com/chromium/src/+/8f60271614652b61100811286044...
Message was sent while issue was closed.
On 2016/11/10 23:31:00, boliu wrote: > wait, wat, this got committed twice? > https://chromium.googlesource.com/chromium/src/+/273cccbc60b358d06c332f64a8b3... > https://chromium.googlesource.com/chromium/src/+/8f60271614652b61100811286044... oh crap, might have reverted the wrong one..
Message was sent while issue was closed.
On 2016/11/10 23:32:01, boliu wrote: > On 2016/11/10 23:31:00, boliu wrote: > > wait, wat, this got committed twice? > > > https://chromium.googlesource.com/chromium/src/+/273cccbc60b358d06c332f64a8b3... > > > https://chromium.googlesource.com/chromium/src/+/8f60271614652b61100811286044... > > oh crap, might have reverted the wrong one.. Not sure why this happened. The smaller one caused the issue - came out of nowhere and somehow even made it through cq. Still clueless.
Message was sent while issue was closed.
On 2016/11/10 23:32:01, boliu wrote: > On 2016/11/10 23:31:00, boliu wrote: > > wait, wat, this got committed twice? > > > https://chromium.googlesource.com/chromium/src/+/273cccbc60b358d06c332f64a8b3... > > > https://chromium.googlesource.com/chromium/src/+/8f60271614652b61100811286044... > > oh crap, might have reverted the wrong one.. Not sure why this happened. The smaller one caused the issue - came out of nowhere and somehow even made it through cq. Still clueless.
Message was sent while issue was closed.
FYI: Findit identified this CL at revision 431383 as the culprit for failures in the build cycles as shown on: https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb...
Message was sent while issue was closed.
Description was changed from ========== Resolves layering violation in SynchronousCompositorHost creation SynchronousCompositorHost created in RWHVA has a reference to WebContents, which violates the hierarchical access rule. This CL resolves it by having WebContentsViewAndroid keep SynchronousCompositorClient instance and pass it to RWHVA. BUG=626765 Committed: https://crrev.com/8f60271614652b6110081128604470460af5dadb Cr-Commit-Position: refs/heads/master@{#431383} ========== to ========== Resolves layering violation in SynchronousCompositorHost creation SynchronousCompositorHost created in RWHVA has a reference to WebContents, which violates the hierarchical access rule. This CL resolves it by having WebContentsViewAndroid keep SynchronousCompositorClient instance and pass it to RWHVA. BUG=626765 Committed: https://crrev.com/8f60271614652b6110081128604470460af5dadb Cr-Commit-Position: refs/heads/master@{#431383} ==========
The CQ bit was checked by jinsukkim@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Resolves layering violation in SynchronousCompositorHost creation SynchronousCompositorHost created in RWHVA has a reference to WebContents, which violates the hierarchical access rule. This CL resolves it by having WebContentsViewAndroid keep SynchronousCompositorClient instance and pass it to RWHVA. BUG=626765 Committed: https://crrev.com/8f60271614652b6110081128604470460af5dadb Cr-Commit-Position: refs/heads/master@{#431383} ========== to ========== Resolves layering violation in SynchronousCompositorHost creation SynchronousCompositorHost created in RWHVA has a reference to WebContents, which violates the hierarchical access rule. This CL resolves it by having WebContentsViewAndroid keep SynchronousCompositorClient instance and pass it to RWHVA. BUG=626765 Committed: https://crrev.com/8f60271614652b6110081128604470460af5dadb Cr-Commit-Position: refs/heads/master@{#431383} ==========
Message was sent while issue was closed.
Committed patchset #6 (id:180001)
Message was sent while issue was closed.
Description was changed from ========== Resolves layering violation in SynchronousCompositorHost creation SynchronousCompositorHost created in RWHVA has a reference to WebContents, which violates the hierarchical access rule. This CL resolves it by having WebContentsViewAndroid keep SynchronousCompositorClient instance and pass it to RWHVA. BUG=626765 Committed: https://crrev.com/8f60271614652b6110081128604470460af5dadb Cr-Commit-Position: refs/heads/master@{#431383} ========== to ========== Resolves layering violation in SynchronousCompositorHost creation SynchronousCompositorHost created in RWHVA has a reference to WebContents, which violates the hierarchical access rule. This CL resolves it by having WebContentsViewAndroid keep SynchronousCompositorClient instance and pass it to RWHVA. BUG=626765 Committed: https://crrev.com/8f60271614652b6110081128604470460af5dadb Committed: https://crrev.com/b53b9955b59a6884ccf6f57ee906e37e2caac26f Cr-Original-Commit-Position: refs/heads/master@{#431383} Cr-Commit-Position: refs/heads/master@{#431565} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/b53b9955b59a6884ccf6f57ee906e37e2caac26f Cr-Commit-Position: refs/heads/master@{#431565} |
