New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 626765 link

Starred by 5 users
Status: Fixed
Owner:
jinsuk...@chromium.org
Closed: Mar 2018
Cc:
yus...@chromium.org
█ aelias@chromium.org
tedc...@chromium.org
█ bshe@chromium.org
wychen@chromium.org
boliu@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 598880



Sign in to add a comment

Android: Remove RenderWidgetHostViewAndroid::content_view_core_ pointer and WebContents access

Project Member Reported by siev...@chromium.org, Jul 8 2016 
Master bug for eliminating this back reference and the SetContentViewCore() hack.

Instead use the appropriate functionality in RenderView/WidgetHost(Delegate), ViewAndroid etc.

Also, RenderWidgetHostViewAndroid should not know about WebContents.
This is sort of a layering violation (even if it's not enforced through DEPS).

Comment 1 by siev...@chromium.org, Jul 8 2016

Labels: OS-Android
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 11 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e6343711183c73be584725b1b0b7f70a60dd2a5b

commit e6343711183c73be584725b1b0b7f70a60dd2a5b
Author: sievers <sievers@chromium.org>
Date: Mon Jul 11 22:35:30 2016

Android: Extend ViewAndroid

This moves cc::Layer ownership from
RenderWidgetHostViewAndroid and ContentViewCore to
ViewAndroid.

Also, ContentViewCore changes from 'is a' ViewAndroid to
'has a' ViewAndroid.

RenderWidgetHostViewAndroid also gets its own ViewAndroid now.
For getting to the WindowAndroid or ViewAndroidDelegate it
will ask its parent.

BUG= 624666 , 598880 ,581521, 626765 
TBR=dtrainor@chromium.org
NOTRY=True

Review-Url: https://codereview.chromium.org/2122403002
Cr-Commit-Position: refs/heads/master@{#404740}

[modify] https://crrev.com/e6343711183c73be584725b1b0b7f70a60dd2a5b/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java
[modify] https://crrev.com/e6343711183c73be584725b1b0b7f70a60dd2a5b/chrome/browser/android/bottombar/overlay_panel_content.cc
[modify] https://crrev.com/e6343711183c73be584725b1b0b7f70a60dd2a5b/chrome/browser/android/compositor/layer/overlay_panel_layer.cc
[modify] https://crrev.com/e6343711183c73be584725b1b0b7f70a60dd2a5b/chrome/browser/android/tab_android.cc
[modify] https://crrev.com/e6343711183c73be584725b1b0b7f70a60dd2a5b/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/e6343711183c73be584725b1b0b7f70a60dd2a5b/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/e6343711183c73be584725b1b0b7f70a60dd2a5b/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/e6343711183c73be584725b1b0b7f70a60dd2a5b/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/e6343711183c73be584725b1b0b7f70a60dd2a5b/content/browser/web_contents/web_contents_view_android.cc
[modify] https://crrev.com/e6343711183c73be584725b1b0b7f70a60dd2a5b/content/public/browser/android/content_view_core.h
[modify] https://crrev.com/e6343711183c73be584725b1b0b7f70a60dd2a5b/ui/android/BUILD.gn
[add] https://crrev.com/e6343711183c73be584725b1b0b7f70a60dd2a5b/ui/android/view_android.cc
[modify] https://crrev.com/e6343711183c73be584725b1b0b7f70a60dd2a5b/ui/android/view_android.h

Comment 3 by siev...@chromium.org, Aug 23 2016

Cc: bshe@chromium.org

Comment 4 by siev...@chromium.org, Sep 14 2016

Cc: boliu@chromium.org

Comment 5 by jinsuk...@chromium.org, Oct 24 2016

Owner: jinsuk...@chromium.org
Status: Assigned (was: Untriaged)
Next step I'm planning to do is to transfer the ownership of ViewAndroid from ContentViewCoreImpl to WebContentsViewAndroid, as illustrated in the diagram. https://docs.google.com/document/d/13obX4RB5nbX3w7vwLzYFuo9LvHzCobIPCDNv09vbHxI/edit This will help RWHVA not reference CVCI, and further remove other references to CVC/CVCI. Note that it is the only user of content::ContentViewCoreImplObserver which I believe can be replaced with a simple plumbing from embedder -> WebContents/View -> RWHVA.

Comment 6 by aelias@chromium.org, Oct 24 2016

Cc: aelias@chromium.org
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 1 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4101e961ba3d80e91683f1c7e28e178831595bcc

commit 4101e961ba3d80e91683f1c7e28e178831595bcc
Author: jinsukkim <jinsukkim@chromium.org>
Date: Tue Nov 01 07:43:44 2016

Have WebContentsViewAndroid own ViewAndroid

This CL transfers the ownership of ViewAndroid associated with Java
content view from CVC to WebContentsViewAndroid, as proposed in
the design https://goo.gl/tQqbZS This is also a step toward removing
references to CVC in RWHVA.

Also simplified an access method to WindowAndroid using the view
hierarchy.

BUG= 598880 ,  626765 

Review-Url: https://codereview.chromium.org/2453213003
Cr-Commit-Position: refs/heads/master@{#428960}

[modify] https://crrev.com/4101e961ba3d80e91683f1c7e28e178831595bcc/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/4101e961ba3d80e91683f1c7e28e178831595bcc/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/4101e961ba3d80e91683f1c7e28e178831595bcc/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/4101e961ba3d80e91683f1c7e28e178831595bcc/content/browser/web_contents/web_contents_view_android.cc
[modify] https://crrev.com/4101e961ba3d80e91683f1c7e28e178831595bcc/content/browser/web_contents/web_contents_view_android.h
[modify] https://crrev.com/4101e961ba3d80e91683f1c7e28e178831595bcc/ui/android/view_android.cc
[modify] https://crrev.com/4101e961ba3d80e91683f1c7e28e178831595bcc/ui/android/view_android.h
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 9 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/16b5a22bcbb4fc166b893ded0ca8116e8632e6d0

commit 16b5a22bcbb4fc166b893ded0ca8116e8632e6d0
Author: jinsukkim <jinsukkim@chromium.org>
Date: Wed Nov 09 22:41:13 2016

Remove access to WebContents in RWHVA::SynchronousFrameMetadata()

This CL helps resolve layering violation done in
RenderWidgetHostViewAndroid by obtaining DevToolsAgentHost
instance using RenderFrameHost instead.

BUG= 626765 

Review-Url: https://codereview.chromium.org/2484793002
Cr-Commit-Position: refs/heads/master@{#431061}

[modify] https://crrev.com/16b5a22bcbb4fc166b893ded0ca8116e8632e6d0/content/browser/devtools/render_frame_devtools_agent_host.cc
[modify] https://crrev.com/16b5a22bcbb4fc166b893ded0ca8116e8632e6d0/content/browser/devtools/render_frame_devtools_agent_host.h
[modify] https://crrev.com/16b5a22bcbb4fc166b893ded0ca8116e8632e6d0/content/browser/renderer_host/render_widget_host_view_android.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 10 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ced620227dc7d5b43473251f75314bd22db969a9

commit ced620227dc7d5b43473251f75314bd22db969a9
Author: jinsukkim <jinsukkim@chromium.org>
Date: Thu Nov 10 00:34:52 2016

Remove access to WebContents in RWHVA::Focus()

Handles the layering violation in RWHVA::Focus() by moving
the relevant part up to WebContentsAndroid. There is no functional
changes.

BUG= 626765 

Review-Url: https://codereview.chromium.org/2485763003
Cr-Commit-Position: refs/heads/master@{#431101}

[modify] https://crrev.com/ced620227dc7d5b43473251f75314bd22db969a9/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/ced620227dc7d5b43473251f75314bd22db969a9/content/browser/web_contents/web_contents_view_android.cc
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 10 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/273cccbc60b358d06c332f64a8b3cc9f5ee42dd6

commit 273cccbc60b358d06c332f64a8b3cc9f5ee42dd6
Author: jinsukkim <jinsukkim@chromium.org>
Date: Thu Nov 10 22:31:16 2016

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 

Review-Url: https://codereview.chromium.org/2487713002
Cr-Commit-Position: refs/heads/master@{#431380}

[modify] https://crrev.com/273cccbc60b358d06c332f64a8b3cc9f5ee42dd6/content/browser/android/synchronous_compositor_host.cc
[modify] https://crrev.com/273cccbc60b358d06c332f64a8b3cc9f5ee42dd6/content/browser/android/synchronous_compositor_host.h
[modify] https://crrev.com/273cccbc60b358d06c332f64a8b3cc9f5ee42dd6/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/273cccbc60b358d06c332f64a8b3cc9f5ee42dd6/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/273cccbc60b358d06c332f64a8b3cc9f5ee42dd6/content/browser/web_contents/web_contents_android.cc
[modify] https://crrev.com/273cccbc60b358d06c332f64a8b3cc9f5ee42dd6/content/browser/web_contents/web_contents_android.h
[modify] https://crrev.com/273cccbc60b358d06c332f64a8b3cc9f5ee42dd6/content/browser/web_contents/web_contents_view_android.cc
[modify] https://crrev.com/273cccbc60b358d06c332f64a8b3cc9f5ee42dd6/content/browser/web_contents/web_contents_view_android.h
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 10 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8f60271614652b6110081128604470460af5dadb

commit 8f60271614652b6110081128604470460af5dadb
Author: jinsukkim <jinsukkim@chromium.org>
Date: Thu Nov 10 22:36:44 2016

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 

Review-Url: https://codereview.chromium.org/2487713002
Cr-Commit-Position: refs/heads/master@{#431383}

[modify] https://crrev.com/8f60271614652b6110081128604470460af5dadb/content/browser/renderer_host/render_widget_host_view_android.cc
Project Member

Comment 12 by bugdroid1@chromium.org, Nov 10 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e7fe2f0dc16cc131804c41ab821e3ac193a87262

commit e7fe2f0dc16cc131804c41ab821e3ac193a87262
Author: boliu <boliu@chromium.org>
Date: Thu Nov 10 23:29:40 2016

Revert of Resolves layering violation in SynchronousCompositorHost creation (patchset #6 id:180001 of https://codereview.chromium.org/2487713002/ )

Reason for revert:
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.

Original issue's description:
> 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}

TBR=jinsukkim@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 626765 

Review-Url: https://codereview.chromium.org/2490383002
Cr-Commit-Position: refs/heads/master@{#431396}

[modify] https://crrev.com/e7fe2f0dc16cc131804c41ab821e3ac193a87262/content/browser/android/synchronous_compositor_host.cc
[modify] https://crrev.com/e7fe2f0dc16cc131804c41ab821e3ac193a87262/content/browser/android/synchronous_compositor_host.h
[modify] https://crrev.com/e7fe2f0dc16cc131804c41ab821e3ac193a87262/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/e7fe2f0dc16cc131804c41ab821e3ac193a87262/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/e7fe2f0dc16cc131804c41ab821e3ac193a87262/content/browser/web_contents/web_contents_android.cc
[modify] https://crrev.com/e7fe2f0dc16cc131804c41ab821e3ac193a87262/content/browser/web_contents/web_contents_android.h
[modify] https://crrev.com/e7fe2f0dc16cc131804c41ab821e3ac193a87262/content/browser/web_contents/web_contents_view_android.cc
[modify] https://crrev.com/e7fe2f0dc16cc131804c41ab821e3ac193a87262/content/browser/web_contents/web_contents_view_android.h

Comment 13 by jinsuk...@chromium.org, Nov 10 2016 

Something inexplicable created an extra CL containing only a part of the original one and land it.

I'll create the CL again.

Comment 14 by jinsuk...@chromium.org, Nov 11 2016 

As for the glitch, see  Issue 662140  for the reference.
Reopening and landing it again now.
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 11 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b53b9955b59a6884ccf6f57ee906e37e2caac26f

commit b53b9955b59a6884ccf6f57ee906e37e2caac26f
Author: jinsukkim <jinsukkim@chromium.org>
Date: Fri Nov 11 15:33:43 2016

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
Review-Url: https://codereview.chromium.org/2487713002
Cr-Original-Commit-Position: refs/heads/master@{#431383}
Cr-Commit-Position: refs/heads/master@{#431565}

[modify] https://crrev.com/b53b9955b59a6884ccf6f57ee906e37e2caac26f/content/browser/android/synchronous_compositor_host.cc
[modify] https://crrev.com/b53b9955b59a6884ccf6f57ee906e37e2caac26f/content/browser/android/synchronous_compositor_host.h
[modify] https://crrev.com/b53b9955b59a6884ccf6f57ee906e37e2caac26f/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/b53b9955b59a6884ccf6f57ee906e37e2caac26f/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/b53b9955b59a6884ccf6f57ee906e37e2caac26f/content/browser/web_contents/web_contents_android.cc
[modify] https://crrev.com/b53b9955b59a6884ccf6f57ee906e37e2caac26f/content/browser/web_contents/web_contents_android.h
[modify] https://crrev.com/b53b9955b59a6884ccf6f57ee906e37e2caac26f/content/browser/web_contents/web_contents_view_android.cc
[modify] https://crrev.com/b53b9955b59a6884ccf6f57ee906e37e2caac26f/content/browser/web_contents/web_contents_view_android.h
Project Member

Comment 16 by bugdroid1@chromium.org, Apr 6 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cc9e77e7d9e05da0f6017f1bdea024f0ce669786

commit cc9e77e7d9e05da0f6017f1bdea024f0ce669786
Author: jinsukkim <jinsukkim@chromium.org>
Date: Thu Apr 06 11:03:39 2017

Migrate IME state update flow

Refactored the flow for IME state update so it bypasses CVCImpl
and go straight from RWHVA -> ImeAdapter native -> Java layer.

Other related changes are:

ImeAdapter provides EventObserver to reduce the dependency on CVC,
based on the suggestion made in https://goo.gl/pdtQCl. It is used
by an embedder (Chrome) and CVC to deal with IME notification.
This replaces IME state update done through CVC. Only the necessary
info (node editability, password attribute) are passed.

ImeAdapter.ImeAdapterDelegate is to delegate some work upon IME
events. But it is used by ContentViewCore only, and there is no
clear benefit of having the interface. It was removed for
simplification. All the stuff can be (and are now) handled inside
ImeAdapter.

BUG= 662908 , 626765 , 620172 

Review-Url: https://codereview.chromium.org/2777223004
Cr-Commit-Position: refs/heads/master@{#462419}

[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/public/android/BUILD.gn
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[add] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/public/android/java/src/org/chromium/content/browser/ViewUtils.java
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[add] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/public/android/java/src/org/chromium/content_public/browser/ImeEventObserver.java
[modify] https://crrev.com/cc9e77e7d9e05da0f6017f1bdea024f0ce669786/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
[delete] https://crrev.com/e3f5c08d551b9a818dfa63874b5db137694b4889/content/public/android/javatests/src/org/chromium/content/browser/input/TestImeAdapterDelegate.java
Project Member

Comment 17 by bugdroid1@chromium.org, Apr 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee89226f5c1820a579f50147cbfaf7df9462a0a2

commit ee89226f5c1820a579f50147cbfaf7df9462a0a2
Author: changwan <changwan@chromium.org>
Date: Fri Apr 07 23:42:41 2017

Revert of Migrate IME state update flow (patchset #4 id:100001 of https://codereview.chromium.org/2777223004/ )

Reason for revert:
This patchset caused a regression crbug.com/709349: 'cut' option disappeared from webview selection pop up.

Original issue's description:
> Migrate IME state update flow
>
> Refactored the flow for IME state update so it bypasses CVCImpl
> and go straight from RWHVA -> ImeAdapter native -> Java layer.
>
> Other related changes are:
>
> ImeAdapter provides EventObserver to reduce the dependency on CVC,
> based on the suggestion made in https://goo.gl/pdtQCl. It is used
> by an embedder (Chrome) and CVC to deal with IME notification.
> This replaces IME state update done through CVC. Only the necessary
> info (node editability, password attribute) are passed.
>
> ImeAdapter.ImeAdapterDelegate is to delegate some work upon IME
> events. But it is used by ContentViewCore only, and there is no
> clear benefit of having the interface. It was removed for
> simplification. All the stuff can be (and are now) handled inside
> ImeAdapter.
>
> BUG= 662908 , 626765 , 620172 
>
> Review-Url: https://codereview.chromium.org/2777223004
> Cr-Commit-Position: refs/heads/master@{#462419}
> Committed: https://chromium.googlesource.com/chromium/src/+/cc9e77e7d9e05da0f6017f1bdea024f0ce669786

TBR=boliu@chromium.org,tedchoc@chromium.org,jinsukkim@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 662908 , 626765 , 620172 

Review-Url: https://codereview.chromium.org/2803203003
Cr-Commit-Position: refs/heads/master@{#463053}

[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/public/android/BUILD.gn
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[delete] https://crrev.com/e3e4a4e437e837f6696861741dcde15693529b50/content/public/android/java/src/org/chromium/content/browser/ViewUtils.java
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[delete] https://crrev.com/e3e4a4e437e837f6696861741dcde15693529b50/content/public/android/java/src/org/chromium/content_public/browser/ImeEventObserver.java
[modify] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
[add] https://crrev.com/ee89226f5c1820a579f50147cbfaf7df9462a0a2/content/public/android/javatests/src/org/chromium/content/browser/input/TestImeAdapterDelegate.java
Project Member

Comment 18 by bugdroid1@chromium.org, Apr 11 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a

commit e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a
Author: jinsukkim <jinsukkim@chromium.org>
Date: Tue Apr 11 02:56:46 2017

Migrate IME state update flow

Refactored the flow for IME state update so it bypasses CVCImpl
and go straight from RWHVA -> ImeAdapter native -> Java layer.

Other related changes are:

ImeAdapter provides EventObserver to reduce the dependency on CVC,
based on the suggestion made in https://goo.gl/pdtQCl. It is used
by an embedder (Chrome) and CVC to deal with IME notification.
This replaces IME state update done through CVC. Only the necessary
info (node editability, password attribute) are passed.

ImeAdapter.ImeAdapterDelegate is to delegate some work upon IME
events. But it is used by ContentViewCore only, and there is no
clear benefit of having the interface. It was removed for
simplification. All the stuff can be (and are now) handled inside
ImeAdapter.

BUG= 662908 , 626765 , 620172 , 709349

Review-Url: https://codereview.chromium.org/2777223004
Cr-Original-Commit-Position: refs/heads/master@{#462419}
Committed: https://chromium.googlesource.com/chromium/src/+/cc9e77e7d9e05da0f6017f1bdea024f0ce669786
Review-Url: https://codereview.chromium.org/2777223004
Cr-Commit-Position: refs/heads/master@{#463508}

[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsGarbageCollectionTest.java
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/browser/android/ime_adapter_android.cc
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/browser/android/ime_adapter_android.h
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/public/android/BUILD.gn
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[add] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/public/android/java/src/org/chromium/content/browser/ViewUtils.java
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
[add] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/public/android/java/src/org/chromium/content_public/browser/ImeEventObserver.java
[modify] https://crrev.com/e4f39f538c7a9fe52a635d4fc4c92e3a5c82fe8a/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
[delete] https://crrev.com/4cede8d39db10321b053c0d9776cf6b23f290310/content/public/android/javatests/src/org/chromium/content/browser/input/TestImeAdapterDelegate.java
Project Member

Comment 19 by bugdroid1@chromium.org, Apr 25 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3fbb94e525cb002ae6b5cae23edc036df3daaac6

commit 3fbb94e525cb002ae6b5cae23edc036df3daaac6
Author: jinsukkim <jinsukkim@chromium.org>
Date: Tue Apr 25 11:42:40 2017

Let selection events bypass ContentViewCore

Introduces native SelectionPopupController to route selection
events directly to Java layer without going through ContentViewCore.
The native class leverages RenderWidgetHostConnector for updating
its connection to RWHVA when it gets swapped out or destroyed.

BUG= 626765 

Review-Url: https://codereview.chromium.org/2826303003
Cr-Commit-Position: refs/heads/master@{#466944}

[modify] https://crrev.com/3fbb94e525cb002ae6b5cae23edc036df3daaac6/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTapEventTest.java
[modify] https://crrev.com/3fbb94e525cb002ae6b5cae23edc036df3daaac6/content/browser/BUILD.gn
[modify] https://crrev.com/3fbb94e525cb002ae6b5cae23edc036df3daaac6/content/browser/android/browser_jni_registrar.cc
[modify] https://crrev.com/3fbb94e525cb002ae6b5cae23edc036df3daaac6/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/3fbb94e525cb002ae6b5cae23edc036df3daaac6/content/browser/android/content_view_core_impl.h
[add] https://crrev.com/3fbb94e525cb002ae6b5cae23edc036df3daaac6/content/browser/android/selection_popup_controller.cc
[add] https://crrev.com/3fbb94e525cb002ae6b5cae23edc036df3daaac6/content/browser/android/selection_popup_controller.h
[modify] https://crrev.com/3fbb94e525cb002ae6b5cae23edc036df3daaac6/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/3fbb94e525cb002ae6b5cae23edc036df3daaac6/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/3fbb94e525cb002ae6b5cae23edc036df3daaac6/content/public/android/BUILD.gn
[modify] https://crrev.com/3fbb94e525cb002ae6b5cae23edc036df3daaac6/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/3fbb94e525cb002ae6b5cae23edc036df3daaac6/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
[modify] https://crrev.com/3fbb94e525cb002ae6b5cae23edc036df3daaac6/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
Project Member

Comment 20 by bugdroid1@chromium.org, May 12 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fb293a5d27d9f233a801653e4983bef0a92e43e9

commit fb293a5d27d9f233a801653e4983bef0a92e43e9
Author: jinsukkim <jinsukkim@chromium.org>
Date: Fri May 12 05:30:42 2017

Store physical backing size in ViewAndroid

Stores physical backing size info in ViewAndroid tree nodes.
It rids CVC of methods related to physical backing size,
and help eliminate the access from RWHVA to CVC done against
layering hierarchy.

BUG= 626765 ,  622847 

Review-Url: https://codereview.chromium.org/2842693005
Cr-Commit-Position: refs/heads/master@{#471227}

[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/android_webview/browser/aw_contents.cc
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorView.java
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanel.java
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutRenderHost.java
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/android/javatests/src/org/chromium/chrome/browser/compositor/CompositorVisibilityTest.java
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/android/javatests/src/org/chromium/chrome/browser/compositor/layouts/MockLayoutHost.java
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/browser/android/bottombar/overlay_panel_content.cc
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/browser/android/bottombar/overlay_panel_content.h
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/browser/android/compositor/compositor_view.cc
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/browser/android/compositor/compositor_view.h
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/browser/android/tab_android.cc
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/browser/android/tab_android.h
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/browser/android/vr_shell/vr_shell.cc
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/chrome/browser/android/vr_shell/vr_shell.h
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/content/browser/android/content_view_render_view.cc
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/content/browser/android/content_view_render_view.h
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/content/browser/web_contents/web_contents_view_android.cc
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/content/browser/web_contents/web_contents_view_android.h
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/ui/android/view_android.cc
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/ui/android/view_android.h
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/ui/android/view_client.cc
[modify] https://crrev.com/fb293a5d27d9f233a801653e4983bef0a92e43e9/ui/android/view_client.h
Project Member

Comment 21 by bugdroid1@chromium.org, Jun 16 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4bd5a1d686abd3670a98e4767602a8b69ee51f4b

commit 4bd5a1d686abd3670a98e4767602a8b69ee51f4b
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Fri Jun 16 06:28:19 2017

Bypass ContentViewCore for selection popup methods

Some flows destined to SelectionPopupController are unnecessarily
routed through ContentViewCore. Since what WebContentsViewDelegate
do for embedders is basically same, factored the common part into
WebContentsViewAndroid, and routed it through
RenderWidgetHostViewAndroid -> SelectionPopupController.

This change got pulled from a bigger refactoring to break it down
to more manageable size https://crrev.com/2890173003 (still WIP).

BUG= 626830 , 626765 

Change-Id: Iaccd2b90288ebc65326b885be3830932e6a38486
Reviewed-on: https://chromium-review.googlesource.com/535676
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479967}
[modify] https://crrev.com/4bd5a1d686abd3670a98e4767602a8b69ee51f4b/android_webview/browser/aw_web_contents_view_delegate.cc
[modify] https://crrev.com/4bd5a1d686abd3670a98e4767602a8b69ee51f4b/android_webview/browser/aw_web_contents_view_delegate.h
[modify] https://crrev.com/4bd5a1d686abd3670a98e4767602a8b69ee51f4b/chrome/browser/ui/android/tab_contents/chrome_web_contents_view_delegate_android.cc
[modify] https://crrev.com/4bd5a1d686abd3670a98e4767602a8b69ee51f4b/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/4bd5a1d686abd3670a98e4767602a8b69ee51f4b/content/browser/android/content_view_core_impl.h
[modify] https://crrev.com/4bd5a1d686abd3670a98e4767602a8b69ee51f4b/content/browser/android/selection_popup_controller.cc
[modify] https://crrev.com/4bd5a1d686abd3670a98e4767602a8b69ee51f4b/content/browser/android/selection_popup_controller.h
[modify] https://crrev.com/4bd5a1d686abd3670a98e4767602a8b69ee51f4b/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/4bd5a1d686abd3670a98e4767602a8b69ee51f4b/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/4bd5a1d686abd3670a98e4767602a8b69ee51f4b/content/browser/web_contents/web_contents_view_android.cc
[modify] https://crrev.com/4bd5a1d686abd3670a98e4767602a8b69ee51f4b/content/browser/web_contents/web_contents_view_android.h
[modify] https://crrev.com/4bd5a1d686abd3670a98e4767602a8b69ee51f4b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/4bd5a1d686abd3670a98e4767602a8b69ee51f4b/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
[modify] https://crrev.com/4bd5a1d686abd3670a98e4767602a8b69ee51f4b/content/public/browser/android/content_view_core.h
[modify] https://crrev.com/4bd5a1d686abd3670a98e4767602a8b69ee51f4b/content/shell/browser/shell_web_contents_view_delegate_android.cc
Project Member

Comment 22 by bugdroid1@chromium.org, Aug 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9604d8b285ec422d4ba9d66f3f8446ccc4deb521

commit 9604d8b285ec422d4ba9d66f3f8446ccc4deb521
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Mon Aug 07 02:47:30 2017

Define public interface for MotionEventSynthesizer

MotionEventSynthesizer didn't have a proper public interface.
This CL provides one with a method |Create| for embedders to use.
Also defines action enum for both Java/native to share.

BUG= 626765 , 736704 

Change-Id: I1c4c14b3fe82314537ea75b2ac32311dc247d07c
Reviewed-on: https://chromium-review.googlesource.com/539457
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492260}
[add] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/AndroidUiGestureTarget.java
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/chrome/android/java_sources.gni
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/chrome/browser/android/vr_shell/BUILD.gn
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/chrome/browser/android/vr_shell/android_ui_gesture_target.cc
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/chrome/browser/android/vr_shell/android_ui_gesture_target.h
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/chrome/browser/android/vr_shell/vr_shell.cc
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/chrome/browser/android/vr_shell/vr_shell.h
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/content/browser/android/content_view_core.cc
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/content/browser/android/content_view_core.h
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/content/browser/renderer_host/input/synthetic_gesture_target_android.cc
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/content/browser/renderer_host/input/synthetic_gesture_target_android.h
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/content/public/android/BUILD.gn
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/content/public/android/java/src/org/chromium/content/browser/MotionEventSynthesizer.java
[add] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/content/public/android/java/src/org/chromium/content/browser/SyntheticGestureTarget.java
[modify] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/content/public/browser/BUILD.gn
[add] https://crrev.com/9604d8b285ec422d4ba9d66f3f8446ccc4deb521/content/public/browser/android/motion_event_action.h
Project Member

Comment 23 by bugdroid1@chromium.org, Aug 25 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8157e11225013c0768b8131d562f2af6600fc470

commit 8157e11225013c0768b8131d562f2af6600fc470
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Fri Aug 25 01:52:13 2017

Partially decouple PopupZoomer from ContentViewCore

This CL updates PopupZoomer not to go through ContentViewCore for
visibility and resolving disambiguation. This eliminate another
RWHVA -> CVC dependency which is done against layering hierarchy.

BUG= 626765 

Change-Id: I7ac65db4c85f6d41249134468f443b34b04b4efe
Reviewed-on: https://chromium-review.googlesource.com/630980
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497292}
[modify] https://crrev.com/8157e11225013c0768b8131d562f2af6600fc470/content/browser/BUILD.gn
[modify] https://crrev.com/8157e11225013c0768b8131d562f2af6600fc470/content/browser/android/content_view_core.cc
[modify] https://crrev.com/8157e11225013c0768b8131d562f2af6600fc470/content/browser/android/content_view_core.h
[add] https://crrev.com/8157e11225013c0768b8131d562f2af6600fc470/content/browser/android/popup_zoomer.cc
[add] https://crrev.com/8157e11225013c0768b8131d562f2af6600fc470/content/browser/android/popup_zoomer.h
[modify] https://crrev.com/8157e11225013c0768b8131d562f2af6600fc470/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/8157e11225013c0768b8131d562f2af6600fc470/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/8157e11225013c0768b8131d562f2af6600fc470/content/public/android/BUILD.gn
[modify] https://crrev.com/8157e11225013c0768b8131d562f2af6600fc470/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/8157e11225013c0768b8131d562f2af6600fc470/content/public/android/java/src/org/chromium/content/browser/PopupZoomer.java
[modify] https://crrev.com/8157e11225013c0768b8131d562f2af6600fc470/content/public/android/javatests/src/org/chromium/content/browser/PopupZoomerTest.java
Project Member

Comment 24 by bugdroid1@chromium.org, Aug 29 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d5d29a26c366c2cae17a849290c877993ac7f28c

commit d5d29a26c366c2cae17a849290c877993ac7f28c
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Tue Aug 29 01:39:44 2017

Remove ContentViewCoreObserver 1/2

ContentViewCoreObserver is used by RWHVA only, for being notified of
events of window attach/detaching and cvc destruction. All of them
can be done alternatively without RWHVA referring to CVC against
layering hierarchy. This CL moves window-related events handling
to WindowAndroid.

BUG= 626765 

Change-Id: I8a034ca29f121132523691a20fec8da7d919c817
Reviewed-on: https://chromium-review.googlesource.com/627537
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497978}
[modify] https://crrev.com/d5d29a26c366c2cae17a849290c877993ac7f28c/content/browser/android/content_view_core.cc
[modify] https://crrev.com/d5d29a26c366c2cae17a849290c877993ac7f28c/content/browser/android/content_view_core_observer.h
[modify] https://crrev.com/d5d29a26c366c2cae17a849290c877993ac7f28c/content/browser/android/dialog_overlay_impl.cc
[modify] https://crrev.com/d5d29a26c366c2cae17a849290c877993ac7f28c/content/browser/android/dialog_overlay_impl.h
[modify] https://crrev.com/d5d29a26c366c2cae17a849290c877993ac7f28c/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/d5d29a26c366c2cae17a849290c877993ac7f28c/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/d5d29a26c366c2cae17a849290c877993ac7f28c/ui/android/BUILD.gn
[modify] https://crrev.com/d5d29a26c366c2cae17a849290c877993ac7f28c/ui/android/view_android.cc
[modify] https://crrev.com/d5d29a26c366c2cae17a849290c877993ac7f28c/ui/android/view_android.h
[add] https://crrev.com/d5d29a26c366c2cae17a849290c877993ac7f28c/ui/android/view_android_observer.h
[modify] https://crrev.com/d5d29a26c366c2cae17a849290c877993ac7f28c/ui/android/view_android_unittests.cc
[modify] https://crrev.com/d5d29a26c366c2cae17a849290c877993ac7f28c/ui/android/window_android.cc
[modify] https://crrev.com/d5d29a26c366c2cae17a849290c877993ac7f28c/ui/android/window_android.h
Project Member

Comment 25 by bugdroid1@chromium.org, Sep 13 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/144909f612915cb29fe4a4901a32cebe25a3775a

commit 144909f612915cb29fe4a4901a32cebe25a3775a
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Wed Sep 13 23:23:22 2017

Remove ContentViewCoreObserver 2/2

RenderWidgetHostViewAndroid was observing ContentViewCore destruction
directly through an interface ContentViewCoreObserver, which is
a layering violation. This CL addresses the issue by introducing
ContentDestructionObserver that decouples RWHVA from ContentViewCore.
CDO instance is created for a RWHVA when it is linked to a certain
WebContents, and notifies it of the event by way of
WebContentsObserver::WebContentsDestroyed which can also be used
to detect the destruction of ContentViewCore.

Bug:  626765 
Change-Id: Iadd03512f4529506a5dc5576ef0e0820264d4513
Reviewed-on: https://chromium-review.googlesource.com/651988
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501796}
[modify] https://crrev.com/144909f612915cb29fe4a4901a32cebe25a3775a/content/browser/BUILD.gn
[modify] https://crrev.com/144909f612915cb29fe4a4901a32cebe25a3775a/content/browser/android/content_view_core.cc
[modify] https://crrev.com/144909f612915cb29fe4a4901a32cebe25a3775a/content/browser/android/content_view_core.h
[delete] https://crrev.com/3dc8ae617834dec3009b75428fe8f4eabeaba130/content/browser/android/content_view_core_observer.h
[modify] https://crrev.com/144909f612915cb29fe4a4901a32cebe25a3775a/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/144909f612915cb29fe4a4901a32cebe25a3775a/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/144909f612915cb29fe4a4901a32cebe25a3775a/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/144909f612915cb29fe4a4901a32cebe25a3775a/content/browser/web_contents/web_contents_impl.h
Project Member

Comment 26 by bugdroid1@chromium.org, Sep 13 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a445def0e5b261cd9e5097dbdfa61597981af76d

commit a445def0e5b261cd9e5097dbdfa61597981af76d
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Wed Sep 13 23:51:37 2017

Delegate TouchHandleDrawable creation to SelectionPopupController

RenderWidgetHostViewAndroid was getting Android context object
 through ContentViewCore to create drawables for touch handle.
This CL delegates the createion to SelectionPopupController
so that it can avoid accessing CVC directly.

BUG= 626765 

Change-Id: I36f29b64b8746e900fac8bbf1cae7f392b17064e
Reviewed-on: https://chromium-review.googlesource.com/654280
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501806}
[modify] https://crrev.com/a445def0e5b261cd9e5097dbdfa61597981af76d/content/browser/android/content_view_core.cc
[modify] https://crrev.com/a445def0e5b261cd9e5097dbdfa61597981af76d/content/browser/android/content_view_core.h
[modify] https://crrev.com/a445def0e5b261cd9e5097dbdfa61597981af76d/content/browser/android/render_widget_host_connector.cc
[modify] https://crrev.com/a445def0e5b261cd9e5097dbdfa61597981af76d/content/browser/android/render_widget_host_connector.h
[modify] https://crrev.com/a445def0e5b261cd9e5097dbdfa61597981af76d/content/browser/android/selection_popup_controller.cc
[modify] https://crrev.com/a445def0e5b261cd9e5097dbdfa61597981af76d/content/browser/android/selection_popup_controller.h
[modify] https://crrev.com/a445def0e5b261cd9e5097dbdfa61597981af76d/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/a445def0e5b261cd9e5097dbdfa61597981af76d/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
[modify] https://crrev.com/a445def0e5b261cd9e5097dbdfa61597981af76d/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java
Project Member

Comment 27 by bugdroid1@chromium.org, Mar 7 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/00e4698b772d9c266b88b69ea7f0824afe194f2e

commit 00e4698b772d9c266b88b69ea7f0824afe194f2e
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Wed Mar 07 07:54:36 2018

Android: WindowAndroid returns mouse wheel scroll factor

Use WindowAndroid context to return a suitable mouse wheel
scroll factor to the renderer asking for mouse wheel minimum
granularity. Moving the logic to WindowAndroid saves
content layer from having to keep the value in RenderCoordinates.

Bug:  626765 
Change-Id: I98c057b428aaf10266c72e4be8cce5fd39019e83
Reviewed-on: https://chromium-review.googlesource.com/950426
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541377}
[modify] https://crrev.com/00e4698b772d9c266b88b69ea7f0824afe194f2e/content/browser/android/content_view_core.cc
[modify] https://crrev.com/00e4698b772d9c266b88b69ea7f0824afe194f2e/content/browser/android/content_view_core.h
[modify] https://crrev.com/00e4698b772d9c266b88b69ea7f0824afe194f2e/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/00e4698b772d9c266b88b69ea7f0824afe194f2e/content/public/android/java/src/org/chromium/content/browser/ContentViewCoreImpl.java
[modify] https://crrev.com/00e4698b772d9c266b88b69ea7f0824afe194f2e/content/public/android/java/src/org/chromium/content/browser/RenderCoordinates.java
[modify] https://crrev.com/00e4698b772d9c266b88b69ea7f0824afe194f2e/ui/android/event_forwarder.cc
[modify] https://crrev.com/00e4698b772d9c266b88b69ea7f0824afe194f2e/ui/android/event_forwarder.h
[modify] https://crrev.com/00e4698b772d9c266b88b69ea7f0824afe194f2e/ui/android/java/src/org/chromium/ui/base/EventForwarder.java
[modify] https://crrev.com/00e4698b772d9c266b88b69ea7f0824afe194f2e/ui/android/java/src/org/chromium/ui/base/WindowAndroid.java
[modify] https://crrev.com/00e4698b772d9c266b88b69ea7f0824afe194f2e/ui/android/window_android.cc
[modify] https://crrev.com/00e4698b772d9c266b88b69ea7f0824afe194f2e/ui/android/window_android.h
Project Member

Comment 28 by bugdroid1@chromium.org, Mar 8 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9762fe19b8e133406b2b4fd10abc2d7fc9d70695

commit 9762fe19b8e133406b2b4fd10abc2d7fc9d70695
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Thu Mar 08 01:12:33 2018

Android: Route frame metadata update to GestureListenerManager

Made GestureListenerManager a bit more responsible and take over
update using the frame metadata from renderer.

GestureListenerManager was already handling scroll/scale limit
changed through frame metadata update, which I believe gives
justification to the refactoring.

This also helps reduce the layering-violating reference to
ContentViewCore from RenderWidgetHostViewAndroid.

Bug:  626765 
Change-Id: Iaa63e2ccd2e3e3b87c8c06b140ff93687f6601a1
Reviewed-on: https://chromium-review.googlesource.com/950526
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541672}
[modify] https://crrev.com/9762fe19b8e133406b2b4fd10abc2d7fc9d70695/content/browser/android/content_view_core.cc
[modify] https://crrev.com/9762fe19b8e133406b2b4fd10abc2d7fc9d70695/content/browser/android/content_view_core.h
[modify] https://crrev.com/9762fe19b8e133406b2b4fd10abc2d7fc9d70695/content/browser/android/gesture_listener_manager.cc
[modify] https://crrev.com/9762fe19b8e133406b2b4fd10abc2d7fc9d70695/content/browser/android/gesture_listener_manager.h
[modify] https://crrev.com/9762fe19b8e133406b2b4fd10abc2d7fc9d70695/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/9762fe19b8e133406b2b4fd10abc2d7fc9d70695/content/public/android/java/src/org/chromium/content/browser/ContentViewCoreImpl.java
[modify] https://crrev.com/9762fe19b8e133406b2b4fd10abc2d7fc9d70695/content/public/android/java/src/org/chromium/content/browser/GestureListenerManagerImpl.java
Project Member

Comment 29 by bugdroid1@chromium.org, Mar 14 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f8f4b9c6e52810997430302c941bc2a00866249d

commit f8f4b9c6e52810997430302c941bc2a00866249d
Author: Jinsuk Kim <jinsukkim@chromium.org>
Date: Wed Mar 14 04:05:45 2018

Android: Remove references to ContentViewCore in RWHVA

Now we got only a handful of references to ContentViewCore object
left in RenderWidgetHostViewAndroid.

All but 3 are all null checks trying to see if content_view_core_
is set to null or not, which is equivalent to checking if the native
view (ViewAndroid) of ContentViewCore and its own native view form
a view tree or not. This can be replaced simply by a boolean flag.

The only 3 are significant (i.e. invoking methods of CVC)

ContentViewCore::GetViewAndroid() - RWHVA may not have an access
    to the parent native view. It should be passed to RWHVA instead.

ContentViewCore::RequestDisallowInterceptTouchEvent() - It goes up
    to Java container view to invoke
    android.view.View.requestDisallowInterceptTouchEvent.
    One more move from CVC to ViewAndroidDelegate...

ContentViewCore::GetWindowAndroid() : same WindowAndroid (or null
    if not attached to view tree) can be obtained via
    ViewAndroid::GetWindowAndroid().

Bug:  626765 
Change-Id: I80088c6a9ca2ca0047184ac77f84f08f64ff0acb
Reviewed-on: https://chromium-review.googlesource.com/958646
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542999}
[modify] https://crrev.com/f8f4b9c6e52810997430302c941bc2a00866249d/content/browser/android/content_view_core.cc
[modify] https://crrev.com/f8f4b9c6e52810997430302c941bc2a00866249d/content/browser/android/content_view_core.h
[modify] https://crrev.com/f8f4b9c6e52810997430302c941bc2a00866249d/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/f8f4b9c6e52810997430302c941bc2a00866249d/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/f8f4b9c6e52810997430302c941bc2a00866249d/content/browser/web_contents/web_contents_view_android.cc
[modify] https://crrev.com/f8f4b9c6e52810997430302c941bc2a00866249d/content/public/android/java/src/org/chromium/content/browser/ContentViewCoreImpl.java
[modify] https://crrev.com/f8f4b9c6e52810997430302c941bc2a00866249d/ui/android/java/src/org/chromium/ui/base/ViewAndroidDelegate.java
[modify] https://crrev.com/f8f4b9c6e52810997430302c941bc2a00866249d/ui/android/view_android.cc
[modify] https://crrev.com/f8f4b9c6e52810997430302c941bc2a00866249d/ui/android/view_android.h

Comment 30 by jinsuk...@chromium.org, Mar 14 2018

Status: Fixed (was: Assigned)
Completed!
►

Sign in to add a comment