Cleanup of Layout Tests code |
|||||||
Issue descriptionAs I proceed with adding OOPIF support for Layout Tests, I am encountering cases where code can be cleaned-up - I thought it will be useful to open a bug to track this work. Examples of anticipated work: - Remove unnecessary public API surface from componente/test_runner - Cleaner separation of view-vs-frame-related code - Remove unused code
,
Mar 15 2016
commit 264b0e5be5267ddd0cbf4ea6bc62df961e492697 Author: lukasza <lukasza@chromium.org> Date: Fri Mar 11 09:19:08 2016 -0800 Using "override" to explicitly annotate overrides of virtual functions. This CL improves compliance with Google C++ Style Guide: https://google.github.io/styleguide/cppguide.html: Explicitly annotate overrides of virtual functions or virtual destructors with an override or (less frequently) final specifier. Older (pre-C++11) code will use the virtual keyword as an inferior alternative annotation. For clarity, use exactly one of override, final, or virtual when declaring an override. Rationale: A function or destructor marked override or final that is not an override of a base class virtual function will not compile, and this helps catch common errors. The specifiers serve as documentation; if no specifier is present, the reader has to check all ancestors of the class in question to determine if the function or destructor is virtual or not. BUG= Review URL: https://codereview.chromium.org/1783003004 Cr-Commit-Position: refs/heads/master@{#380658} components/test_runner/web_frame_test_proxy.h | 145 ++++++++++++++++++++++++++++++++------------------------------- components/test_runner/web_test_proxy.h | 67 +++++++++++++++-------------- 2 files changed, 108 insertions(+), 104 deletions(-)
,
Mar 15 2016
,
Mar 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/27bbd7475ee6baef2c72ab03205abde2fa8b95f7 commit 27bbd7475ee6baef2c72ab03205abde2fa8b95f7 Author: lukasza <lukasza@chromium.org> Date: Thu Mar 17 21:12:52 2016 Make test_runner::AppBannerClient an internal detail of components/test_runner. WebTestInterfaces::GetAppBannerClient can be removed, because TestRunner::ResolveBeforeInstallPromptPromise can interact with AppBannerClient directly, rather than going through WebTestDelegate. This also means that components/test_runner/app_banner_client.h header no longer needs to be included outside of components/test_runner. BUG= 595089 Review URL: https://codereview.chromium.org/1807643002 Cr-Commit-Position: refs/heads/master@{#381797} [modify] https://crrev.com/27bbd7475ee6baef2c72ab03205abde2fa8b95f7/components/test_runner/app_banner_client.h [modify] https://crrev.com/27bbd7475ee6baef2c72ab03205abde2fa8b95f7/components/test_runner/test_runner.cc [modify] https://crrev.com/27bbd7475ee6baef2c72ab03205abde2fa8b95f7/components/test_runner/web_test_delegate.h [modify] https://crrev.com/27bbd7475ee6baef2c72ab03205abde2fa8b95f7/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/27bbd7475ee6baef2c72ab03205abde2fa8b95f7/content/shell/renderer/layout_test/blink_test_runner.h [modify] https://crrev.com/27bbd7475ee6baef2c72ab03205abde2fa8b95f7/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
,
Mar 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6a113ae1eed99940208bef750221d2f9ff058c9b commit 6a113ae1eed99940208bef750221d2f9ff058c9b Author: lukasza <lukasza@chromium.org> Date: Thu Mar 17 22:41:20 2016 Make MockScreenOrientationClient an internal detail of components/test_runner. Ownership of MockScreenOrientationClient can be moved to TestRunner (which holds ownership of other things that can be influenced by javascript bindings). Furthermore, TestRunner doesn't need to take a detour via WebTestDelegate (aka BlinkTestRunner) to interact with MockScreenOrientationClient. This means that we can remove three screen-orientation-related methods from WebTestDelegate. This in turn means that components/test_runner/mock_screen_orientation_client.h header no longer needs to be included outside of components/test_runner. BUG= 595089 Review URL: https://codereview.chromium.org/1807733002 Cr-Commit-Position: refs/heads/master@{#381814} [modify] https://crrev.com/6a113ae1eed99940208bef750221d2f9ff058c9b/components/test_runner/test_runner.cc [modify] https://crrev.com/6a113ae1eed99940208bef750221d2f9ff058c9b/components/test_runner/test_runner.h [modify] https://crrev.com/6a113ae1eed99940208bef750221d2f9ff058c9b/components/test_runner/web_test_delegate.h [modify] https://crrev.com/6a113ae1eed99940208bef750221d2f9ff058c9b/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/6a113ae1eed99940208bef750221d2f9ff058c9b/components/test_runner/web_test_proxy.h [modify] https://crrev.com/6a113ae1eed99940208bef750221d2f9ff058c9b/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/6a113ae1eed99940208bef750221d2f9ff058c9b/content/shell/renderer/layout_test/blink_test_runner.h
,
Mar 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ef264fc5601239984a67e3a39580b6a7855142b8 commit ef264fc5601239984a67e3a39580b6a7855142b8 Author: lukasza <lukasza@chromium.org> Date: Thu Mar 17 22:49:19 2016 Make test_runner::LayoutDumpFlags an internal detail of components/test_runner. To simplify public API surface of components/test_runner we can hide LayoutDumpFlags behind WebTestRunner interface. For example - instead of returning the whole LayoutDumpFlags struct, we can only expose IsRecursiveLayoutDumpRequested method - this is consistent with how other similar methods are exposed via WebTestRunner (i.e. we already have ShouldDumpAsAudio and/or ShouldGeneratePixelResults). After doing this components/test_runner/layout_dump_flags.h and components/test_runner/layout_dump.h headers no longer needs to be included outside of components/test_runner. BUG= 595089 Review URL: https://codereview.chromium.org/1805243002 Cr-Commit-Position: refs/heads/master@{#381819} [modify] https://crrev.com/ef264fc5601239984a67e3a39580b6a7855142b8/components/test_runner/test_runner.cc [modify] https://crrev.com/ef264fc5601239984a67e3a39580b6a7855142b8/components/test_runner/test_runner.h [modify] https://crrev.com/ef264fc5601239984a67e3a39580b6a7855142b8/components/test_runner/web_test_runner.h [modify] https://crrev.com/ef264fc5601239984a67e3a39580b6a7855142b8/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/ef264fc5601239984a67e3a39580b6a7855142b8/content/shell/renderer/layout_test/layout_test_render_frame_observer.cc
,
Mar 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3cbe3f8b30b3eda644b1fef73f88185375101731 commit 3cbe3f8b30b3eda644b1fef73f88185375101731 Author: lukasza <lukasza@chromium.org> Date: Thu Mar 17 23:08:05 2016 Make test_runner::WebTask an internal detail of components/test_runner. We can expose test_runner::WebTask as blink::WebTaskRunner::Task. Doing this means that components/test_runner/web_task.h header no longer needs to be included outside of components/test_runner. BUG= 595089 Review URL: https://codereview.chromium.org/1805753002 Cr-Commit-Position: refs/heads/master@{#381826} [modify] https://crrev.com/3cbe3f8b30b3eda644b1fef73f88185375101731/components/test_runner/mock_web_speech_recognizer.cc [modify] https://crrev.com/3cbe3f8b30b3eda644b1fef73f88185375101731/components/test_runner/web_task.h [modify] https://crrev.com/3cbe3f8b30b3eda644b1fef73f88185375101731/components/test_runner/web_test_delegate.h [modify] https://crrev.com/3cbe3f8b30b3eda644b1fef73f88185375101731/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/3cbe3f8b30b3eda644b1fef73f88185375101731/content/shell/renderer/layout_test/blink_test_runner.h
,
Mar 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/11e459b088ab4736f10776a3831d0b9ddf249517 commit 11e459b088ab4736f10776a3831d0b9ddf249517 Author: lukasza <lukasza@chromium.org> Date: Wed Mar 30 19:07:59 2016 Extract WebFrameClient implementation out of WebTestProxyBase. Overview ======== This CL extracts WebFrameClient-specific parts of WebTestProxyBase into a new WebFrameTestClient class. This CL is part of a sequence of CLs that make progress toward: 1) Splitting WebTestProxyBase into separate WebFrameClient / WebWidgetClient / WebViewClient focused classes 2) Removing WebTestProxyBase from public API of components/test_runner (and having WebTestProxy and WebFrameTestProxy consume instead abstract WebFrameClient / WebWidgetClient / WebViewClient interfaces). Details ======= Most of the change is mechanical move of code from WebTestProxyBase into the new WebFrameTestClient (slightly tweaking method signatures as needed to make them fit WebFrameClient interface - note camelCasing and extra parameters here and there). Additionally some helper methods (i.e. URLDescription and WebNavigationPolicyToString) are now needed by both WebTestProxyBase and by WebFrameTestClient. Consequently, to facilitate code reuse, these helper methods were moved to test_common.cc. Finally, in order to decouple WebFrameTestClient from view-specific WebTestProxyBase the CL does the following: - It moves counting currently active color choosers from WebTestProxyBase into TestRunner (and adjusts MockColorChooser accordingly). - It moves ownership of MockWebUserMediaClient from WebTestProxyBase into TestRunner. This is quite natural, as MockWebUserMediaClient depends only on WebTestDelegate::PostTask and WebTestDelegate::AddMediaStreamAudioSourceAndTrack (which are not view-specific). - Replacing WebFrameTestProxy::set_base_proxy with WebFrameTestProxy::set_test_client requires refactoring how higher layers provide the necessary object - in particular after the CL the WebFrame[Test]Client cannot be extracted from WebTestProxy/RenderView (because the whole point of this CL is to remove the coupling between view-specific WebTestProxy and frame-focused WebFrameTestProxy). This part of the refactoring is accomplished by adding a frame-proxy creation callback that can be provided to EnableWebTestProxyCreation (similarily how it already was accepting equivalent view-proxy creation callback). This refactoring also necessitates extracting a WebFrameTestProxyBase (to have a concrete/non-templatized type that content/shell/.../layout_tests layer can take a dependency on). BUG= 595089 Review URL: https://codereview.chromium.org/1821923003 Cr-Commit-Position: refs/heads/master@{#384044} [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/BUILD.gn [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/mock_color_chooser.cc [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/mock_color_chooser.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/test_common.cc [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/test_common.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/test_runner.cc [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/test_runner.gyp [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/test_runner.h [add] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/web_frame_test_client.cc [add] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/web_frame_test_client.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/web_frame_test_proxy.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/web_test_interfaces.cc [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/web_test_interfaces.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/web_test_proxy.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/content/public/test/layouttest_support.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/content/shell/renderer/layout_test/layout_test_content_renderer_client.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/content/test/layouttest_support.cc
,
Mar 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/11e459b088ab4736f10776a3831d0b9ddf249517 commit 11e459b088ab4736f10776a3831d0b9ddf249517 Author: lukasza <lukasza@chromium.org> Date: Wed Mar 30 19:07:59 2016 Extract WebFrameClient implementation out of WebTestProxyBase. Overview ======== This CL extracts WebFrameClient-specific parts of WebTestProxyBase into a new WebFrameTestClient class. This CL is part of a sequence of CLs that make progress toward: 1) Splitting WebTestProxyBase into separate WebFrameClient / WebWidgetClient / WebViewClient focused classes 2) Removing WebTestProxyBase from public API of components/test_runner (and having WebTestProxy and WebFrameTestProxy consume instead abstract WebFrameClient / WebWidgetClient / WebViewClient interfaces). Details ======= Most of the change is mechanical move of code from WebTestProxyBase into the new WebFrameTestClient (slightly tweaking method signatures as needed to make them fit WebFrameClient interface - note camelCasing and extra parameters here and there). Additionally some helper methods (i.e. URLDescription and WebNavigationPolicyToString) are now needed by both WebTestProxyBase and by WebFrameTestClient. Consequently, to facilitate code reuse, these helper methods were moved to test_common.cc. Finally, in order to decouple WebFrameTestClient from view-specific WebTestProxyBase the CL does the following: - It moves counting currently active color choosers from WebTestProxyBase into TestRunner (and adjusts MockColorChooser accordingly). - It moves ownership of MockWebUserMediaClient from WebTestProxyBase into TestRunner. This is quite natural, as MockWebUserMediaClient depends only on WebTestDelegate::PostTask and WebTestDelegate::AddMediaStreamAudioSourceAndTrack (which are not view-specific). - Replacing WebFrameTestProxy::set_base_proxy with WebFrameTestProxy::set_test_client requires refactoring how higher layers provide the necessary object - in particular after the CL the WebFrame[Test]Client cannot be extracted from WebTestProxy/RenderView (because the whole point of this CL is to remove the coupling between view-specific WebTestProxy and frame-focused WebFrameTestProxy). This part of the refactoring is accomplished by adding a frame-proxy creation callback that can be provided to EnableWebTestProxyCreation (similarily how it already was accepting equivalent view-proxy creation callback). This refactoring also necessitates extracting a WebFrameTestProxyBase (to have a concrete/non-templatized type that content/shell/.../layout_tests layer can take a dependency on). BUG= 595089 Review URL: https://codereview.chromium.org/1821923003 Cr-Commit-Position: refs/heads/master@{#384044} [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/BUILD.gn [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/mock_color_chooser.cc [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/mock_color_chooser.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/test_common.cc [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/test_common.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/test_runner.cc [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/test_runner.gyp [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/test_runner.h [add] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/web_frame_test_client.cc [add] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/web_frame_test_client.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/web_frame_test_proxy.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/web_test_interfaces.cc [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/web_test_interfaces.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/components/test_runner/web_test_proxy.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/content/public/test/layouttest_support.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/content/shell/renderer/layout_test/layout_test_content_renderer_client.h [modify] https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517/content/test/layouttest_support.cc
,
Mar 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/305f1739ea0dc08182da9d81193b859518f31bc7 commit 305f1739ea0dc08182da9d81193b859518f31bc7 Author: lukasza <lukasza@chromium.org> Date: Wed Mar 30 20:55:42 2016 Replicate to all renderers the accepted languages set by a layout test. Moving |accept_languages_| from WebTestProxyBase into LayoutTestRuntimeFlags helps to 1) replicate the value requested by a test into all renderers 2) prepare WebTestProxyBase to help with extraction of WebViewTestClient class. BUG= 587175 , 595089 Review URL: https://codereview.chromium.org/1829253002 Cr-Commit-Position: refs/heads/master@{#384089} [modify] https://crrev.com/305f1739ea0dc08182da9d81193b859518f31bc7/components/test_runner/layout_test_runtime_flags.cc [modify] https://crrev.com/305f1739ea0dc08182da9d81193b859518f31bc7/components/test_runner/layout_test_runtime_flags.h [modify] https://crrev.com/305f1739ea0dc08182da9d81193b859518f31bc7/components/test_runner/test_runner.cc [modify] https://crrev.com/305f1739ea0dc08182da9d81193b859518f31bc7/components/test_runner/test_runner.h [modify] https://crrev.com/305f1739ea0dc08182da9d81193b859518f31bc7/components/test_runner/tracked_dictionary.cc [modify] https://crrev.com/305f1739ea0dc08182da9d81193b859518f31bc7/components/test_runner/tracked_dictionary.h [modify] https://crrev.com/305f1739ea0dc08182da9d81193b859518f31bc7/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/305f1739ea0dc08182da9d81193b859518f31bc7/components/test_runner/web_test_proxy.h
,
Mar 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bc3216c1723d16c05d88e26e72727a53bbc0c50b commit bc3216c1723d16c05d88e26e72727a53bbc0c50b Author: lukasza <lukasza@chromium.org> Date: Wed Mar 30 22:07:46 2016 Moving pixel-capturing code from web_test_proxy_base.* into pixel_dump.* This CL: - Moves pixel-capturing code from web_test_proxy_base.* into newly created pixel_dump.cc and pixel_dump.h. This helps clean-up WebTestProxyBase in preparation for extracting view/frame/widget-specific code to separate classes. - Moves |drag_image_| fields from WebTestProxyBase into TestRunner. This also helps clean-up WebTestProxyBase in preparation for extracting view/frame/widget-specific code to separate classes. - Moves |dump_selection_rect| and |dump_drag_image| from TestRunner into LayoutTestRuntimeFlags. This makes sure these flags are replicated across renderers as needed. This also means that accessors for these flags no longer need to be exposed by TestRunner. - Simplifies how WebTestDelegate communicates devices scaling factor (used when positioning a pop-up for pixel dump). This helps avoid adding a WebTestDelegate dependency to pixel_dump.h/.cc. BUG= 587175 , 595089 Review URL: https://codereview.chromium.org/1835673002 Cr-Commit-Position: refs/heads/master@{#384109} [modify] https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b/components/test_runner/BUILD.gn [modify] https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b/components/test_runner/layout_dump.h [modify] https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b/components/test_runner/layout_test_runtime_flags.cc [modify] https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b/components/test_runner/layout_test_runtime_flags.h [add] https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b/components/test_runner/pixel_dump.cc [add] https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b/components/test_runner/pixel_dump.h [modify] https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b/components/test_runner/test_runner.cc [modify] https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b/components/test_runner/test_runner.gyp [modify] https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b/components/test_runner/test_runner.h [modify] https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b/components/test_runner/web_test_delegate.h [modify] https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b/components/test_runner/web_test_proxy.h [modify] https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b/components/test_runner/web_test_runner.h [modify] https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b/content/shell/renderer/layout_test/blink_test_runner.h
,
Mar 31 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fd193b305672063bb1620edf134a82fc6edf5e1d commit fd193b305672063bb1620edf134a82fc6edf5e1d Author: lukasza <lukasza@chromium.org> Date: Thu Mar 31 14:05:56 2016 Revert of Moving pixel-capturing code from web_test_proxy_base.* into pixel_dump.* (patchset #4 id:60001 of https://codereview.chromium.org/1835673002/ ) Reason for revert: Prevents clean reverting of an earlier CL (https://codereview.chromium.org/1821923003/) tht cause use after free related to color chooser. Original issue's description: > Moving pixel-capturing code from web_test_proxy_base.* into pixel_dump.* > > This CL: > > - Moves pixel-capturing code from web_test_proxy_base.* into newly > created pixel_dump.cc and pixel_dump.h. This helps clean-up > WebTestProxyBase in preparation for extracting > view/frame/widget-specific code to separate classes. > > - Moves |drag_image_| fields from WebTestProxyBase into TestRunner. > This also helps clean-up WebTestProxyBase in preparation for > extracting view/frame/widget-specific code to separate classes. > > - Moves |dump_selection_rect| and |dump_drag_image| from TestRunner > into LayoutTestRuntimeFlags. This makes sure these flags are > replicated across renderers as needed. This also means that accessors > for these flags no longer need to be exposed by TestRunner. > > - Simplifies how WebTestDelegate communicates devices scaling factor > (used when positioning a pop-up for pixel dump). This helps > avoid adding a WebTestDelegate dependency to pixel_dump.h/.cc. > > BUG= 587175 , 595089 > > Committed: https://crrev.com/bc3216c1723d16c05d88e26e72727a53bbc0c50b > Cr-Commit-Position: refs/heads/master@{#384109} TBR=jochen@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 587175 , 595089 Review URL: https://codereview.chromium.org/1842353002 Cr-Commit-Position: refs/heads/master@{#384265} [modify] https://crrev.com/fd193b305672063bb1620edf134a82fc6edf5e1d/components/test_runner/BUILD.gn [modify] https://crrev.com/fd193b305672063bb1620edf134a82fc6edf5e1d/components/test_runner/layout_dump.h [modify] https://crrev.com/fd193b305672063bb1620edf134a82fc6edf5e1d/components/test_runner/layout_test_runtime_flags.cc [modify] https://crrev.com/fd193b305672063bb1620edf134a82fc6edf5e1d/components/test_runner/layout_test_runtime_flags.h [delete] https://crrev.com/e4c212bb226b1b456fb0ac10178204da221987f4/components/test_runner/pixel_dump.cc [delete] https://crrev.com/e4c212bb226b1b456fb0ac10178204da221987f4/components/test_runner/pixel_dump.h [modify] https://crrev.com/fd193b305672063bb1620edf134a82fc6edf5e1d/components/test_runner/test_runner.cc [modify] https://crrev.com/fd193b305672063bb1620edf134a82fc6edf5e1d/components/test_runner/test_runner.gyp [modify] https://crrev.com/fd193b305672063bb1620edf134a82fc6edf5e1d/components/test_runner/test_runner.h [modify] https://crrev.com/fd193b305672063bb1620edf134a82fc6edf5e1d/components/test_runner/web_test_delegate.h [modify] https://crrev.com/fd193b305672063bb1620edf134a82fc6edf5e1d/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/fd193b305672063bb1620edf134a82fc6edf5e1d/components/test_runner/web_test_proxy.h [modify] https://crrev.com/fd193b305672063bb1620edf134a82fc6edf5e1d/components/test_runner/web_test_runner.h [modify] https://crrev.com/fd193b305672063bb1620edf134a82fc6edf5e1d/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/fd193b305672063bb1620edf134a82fc6edf5e1d/content/shell/renderer/layout_test/blink_test_runner.h
,
Mar 31 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1f3397f96d32c86a816c76d3f99801cf32491c04 commit 1f3397f96d32c86a816c76d3f99801cf32491c04 Author: lukasza <lukasza@chromium.org> Date: Thu Mar 31 14:14:19 2016 Revert of Replicate to all renderers the accepted languages set by a layout test. (patchset #5 id:80001 of https://codereview.chromium.org/1829253002/ ) Reason for revert: Prevents clean reverting of an earlier CL (https://codereview.chromium.org/1821923003/) that caused use after free related to color chooser. Original issue's description: > Replicate to all renderers the accepted languages set by a layout test. > > Moving |accept_languages_| from WebTestProxyBase into LayoutTestRuntimeFlags > helps to > 1) replicate the value requested by a test into all renderers > 2) prepare WebTestProxyBase to help with extraction of WebViewTestClient class. > > BUG= 587175 , 595089 > > Committed: https://crrev.com/305f1739ea0dc08182da9d81193b859518f31bc7 > Cr-Commit-Position: refs/heads/master@{#384089} TBR=jochen@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 587175 , 595089 Review URL: https://codereview.chromium.org/1851483002 Cr-Commit-Position: refs/heads/master@{#384267} [modify] https://crrev.com/1f3397f96d32c86a816c76d3f99801cf32491c04/components/test_runner/layout_test_runtime_flags.cc [modify] https://crrev.com/1f3397f96d32c86a816c76d3f99801cf32491c04/components/test_runner/layout_test_runtime_flags.h [modify] https://crrev.com/1f3397f96d32c86a816c76d3f99801cf32491c04/components/test_runner/test_runner.cc [modify] https://crrev.com/1f3397f96d32c86a816c76d3f99801cf32491c04/components/test_runner/test_runner.h [modify] https://crrev.com/1f3397f96d32c86a816c76d3f99801cf32491c04/components/test_runner/tracked_dictionary.cc [modify] https://crrev.com/1f3397f96d32c86a816c76d3f99801cf32491c04/components/test_runner/tracked_dictionary.h [modify] https://crrev.com/1f3397f96d32c86a816c76d3f99801cf32491c04/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/1f3397f96d32c86a816c76d3f99801cf32491c04/components/test_runner/web_test_proxy.h
,
Mar 31 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/69e85e2e09b39a7c5366a8c704a984622b1e7846 commit 69e85e2e09b39a7c5366a8c704a984622b1e7846 Author: jochen <jochen@chromium.org> Date: Thu Mar 31 14:19:57 2016 Revert of Extract WebFrameClient implementation out of WebTestProxyBase. (patchset #5 id:80001 of https://codereview.chromium.org/1821923003/ ) Reason for revert: use after free in color chooser Original issue's description: > Extract WebFrameClient implementation out of WebTestProxyBase. > > Overview > ======== > > This CL extracts WebFrameClient-specific parts of WebTestProxyBase into > a new WebFrameTestClient class. > > This CL is part of a sequence of CLs that make progress toward: > > 1) Splitting WebTestProxyBase into separate WebFrameClient / > WebWidgetClient / WebViewClient focused classes > > 2) Removing WebTestProxyBase from public API of components/test_runner > (and having WebTestProxy and WebFrameTestProxy consume instead > abstract WebFrameClient / WebWidgetClient / WebViewClient interfaces). > > Details > ======= > > Most of the change is mechanical move of code from WebTestProxyBase > into the new WebFrameTestClient (slightly tweaking method signatures > as needed to make them fit WebFrameClient interface - note camelCasing > and extra parameters here and there). > > Additionally some helper methods (i.e. URLDescription and > WebNavigationPolicyToString) are now needed by both WebTestProxyBase and > by WebFrameTestClient. Consequently, to facilitate code reuse, these > helper methods were moved to test_common.cc. > > Finally, in order to decouple WebFrameTestClient from view-specific > WebTestProxyBase the CL does the following: > > - It moves counting currently active color choosers from > WebTestProxyBase into TestRunner (and adjusts MockColorChooser > accordingly). > > - It moves ownership of MockWebUserMediaClient from > WebTestProxyBase into TestRunner. This is quite natural, as > MockWebUserMediaClient depends only on WebTestDelegate::PostTask and > WebTestDelegate::AddMediaStreamAudioSourceAndTrack (which are not > view-specific). > > - Replacing WebFrameTestProxy::set_base_proxy with > WebFrameTestProxy::set_test_client requires refactoring how higher > layers provide the necessary object - in particular after the CL the > WebFrame[Test]Client cannot be extracted from WebTestProxy/RenderView > (because the whole point of this CL is to remove the coupling between > view-specific WebTestProxy and frame-focused WebFrameTestProxy). This > part of the refactoring is accomplished by adding a frame-proxy > creation callback that can be provided to EnableWebTestProxyCreation > (similarily how it already was accepting equivalent view-proxy > creation callback). This refactoring also necessitates extracting a > WebFrameTestProxyBase (to have a concrete/non-templatized type that > content/shell/.../layout_tests layer can take a dependency on). > > BUG= 595089 > > Committed: https://crrev.com/11e459b088ab4736f10776a3831d0b9ddf249517 > Cr-Commit-Position: refs/heads/master@{#384044} TBR=kochi@chromium.org,lukasza@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 595089 Review URL: https://codereview.chromium.org/1841833005 Cr-Commit-Position: refs/heads/master@{#384269} [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/components/test_runner/BUILD.gn [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/components/test_runner/mock_color_chooser.cc [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/components/test_runner/mock_color_chooser.h [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/components/test_runner/test_common.cc [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/components/test_runner/test_common.h [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/components/test_runner/test_runner.cc [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/components/test_runner/test_runner.gyp [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/components/test_runner/test_runner.h [delete] https://crrev.com/e76b563f8771eece6ef162753c7909a934eb3a07/components/test_runner/web_frame_test_client.cc [delete] https://crrev.com/e76b563f8771eece6ef162753c7909a934eb3a07/components/test_runner/web_frame_test_client.h [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/components/test_runner/web_frame_test_proxy.h [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/components/test_runner/web_test_interfaces.cc [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/components/test_runner/web_test_interfaces.h [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/components/test_runner/web_test_proxy.h [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/content/public/test/layouttest_support.h [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/content/shell/renderer/layout_test/layout_test_content_renderer_client.h [modify] https://crrev.com/69e85e2e09b39a7c5366a8c704a984622b1e7846/content/test/layouttest_support.cc
,
Apr 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fd1246076d86b2b9245bb808e7d9a25867256b96 commit fd1246076d86b2b9245bb808e7d9a25867256b96 Author: lukasza <lukasza@chromium.org> Date: Fri Apr 01 16:53:30 2016 Extract WebFrameClient implementation out of WebTestProxyBase. Overview ======== This CL extracts WebFrameClient-specific parts of WebTestProxyBase into a new WebFrameTestClient class. This CL is part of a sequence of CLs that make progress toward: 1) Splitting WebTestProxyBase into separate WebFrameClient / WebWidgetClient / WebViewClient focused classes 2) Removing WebTestProxyBase from public API of components/test_runner (and having WebTestProxy and WebFrameTestProxy consume instead abstract WebFrameClient / WebWidgetClient / WebViewClient interfaces). Details ======= Most of the change is mechanical move of code from WebTestProxyBase into the new WebFrameTestClient (slightly tweaking method signatures as needed to make them fit WebFrameClient interface - note camelCasing and extra parameters here and there). Additionally some helper methods (i.e. URLDescription and WebNavigationPolicyToString) are now needed by both WebTestProxyBase and by WebFrameTestClient. Consequently, to facilitate code reuse, these helper methods were moved to test_common.cc. Finally, in order to decouple WebFrameTestClient from view-specific WebTestProxyBase the CL does the following: - It moves counting currently active color choosers from WebTestProxyBase into TestRunner (and adjusts MockColorChooser accordingly). - It moves ownership of MockWebUserMediaClient from WebTestProxyBase into TestRunner. This is quite natural, as MockWebUserMediaClient depends only on WebTestDelegate::PostTask and WebTestDelegate::AddMediaStreamAudioSourceAndTrack (which are not view-specific). - Replacing WebFrameTestProxy::set_base_proxy with WebFrameTestProxy::set_test_client requires refactoring how higher layers provide the necessary object - in particular after the CL the WebFrame[Test]Client cannot be extracted from WebTestProxy/RenderView (because the whole point of this CL is to remove the coupling between view-specific WebTestProxy and frame-focused WebFrameTestProxy). This part of the refactoring is accomplished by adding a frame-proxy creation callback that can be provided to EnableWebTestProxyCreation (similarily how it already was accepting equivalent view-proxy creation callback). This refactoring also necessitates extracting a WebFrameTestProxyBase (to have a concrete/non-templatized type that content/shell/.../layout_tests layer can take a dependency on). BUG= 595089 Review URL: https://codereview.chromium.org/1847893002 Cr-Commit-Position: refs/heads/master@{#384609} [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/components/test_runner/BUILD.gn [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/components/test_runner/mock_color_chooser.cc [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/components/test_runner/mock_color_chooser.h [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/components/test_runner/test_common.cc [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/components/test_runner/test_common.h [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/components/test_runner/test_runner.cc [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/components/test_runner/test_runner.gyp [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/components/test_runner/test_runner.h [add] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/components/test_runner/web_frame_test_client.cc [add] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/components/test_runner/web_frame_test_client.h [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/components/test_runner/web_frame_test_proxy.h [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/components/test_runner/web_test_interfaces.cc [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/components/test_runner/web_test_interfaces.h [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/components/test_runner/web_test_proxy.h [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/content/public/test/layouttest_support.h [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/content/shell/renderer/layout_test/layout_test_content_renderer_client.h [modify] https://crrev.com/fd1246076d86b2b9245bb808e7d9a25867256b96/content/test/layouttest_support.cc
,
Apr 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a0b624a27f13cab7b1e0dcfcafdc71570dc04a0b commit a0b624a27f13cab7b1e0dcfcafdc71570dc04a0b Author: lukasza <lukasza@chromium.org> Date: Mon Apr 04 15:00:49 2016 Replicate to all renderers the accepted languages set by a layout test. Moving |accept_languages_| from WebTestProxyBase into LayoutTestRuntimeFlags helps to 1) replicate the value requested by a test into all renderers 2) prepare WebTestProxyBase to help with extraction of WebViewTestClient class. BUG= 587175 , 595089 TBR=jochen@chromium.org Review URL: https://codereview.chromium.org/1847033004 Cr-Commit-Position: refs/heads/master@{#384908} [modify] https://crrev.com/a0b624a27f13cab7b1e0dcfcafdc71570dc04a0b/components/test_runner/layout_test_runtime_flags.cc [modify] https://crrev.com/a0b624a27f13cab7b1e0dcfcafdc71570dc04a0b/components/test_runner/layout_test_runtime_flags.h [modify] https://crrev.com/a0b624a27f13cab7b1e0dcfcafdc71570dc04a0b/components/test_runner/test_runner.cc [modify] https://crrev.com/a0b624a27f13cab7b1e0dcfcafdc71570dc04a0b/components/test_runner/test_runner.h [modify] https://crrev.com/a0b624a27f13cab7b1e0dcfcafdc71570dc04a0b/components/test_runner/tracked_dictionary.cc [modify] https://crrev.com/a0b624a27f13cab7b1e0dcfcafdc71570dc04a0b/components/test_runner/tracked_dictionary.h [modify] https://crrev.com/a0b624a27f13cab7b1e0dcfcafdc71570dc04a0b/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/a0b624a27f13cab7b1e0dcfcafdc71570dc04a0b/components/test_runner/web_test_proxy.h
,
Apr 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1b546c1600f9daea517c16b29cddffe515936822 commit 1b546c1600f9daea517c16b29cddffe515936822 Author: lukasza <lukasza@chromium.org> Date: Mon Apr 04 16:19:20 2016 Moving pixel-capturing code from web_test_proxy_base.* into pixel_dump.* This CL: - Moves pixel-capturing code from web_test_proxy_base.* into newly created pixel_dump.cc and pixel_dump.h. This helps clean-up WebTestProxyBase in preparation for extracting view/frame/widget-specific code to separate classes. - Moves |drag_image_| fields from WebTestProxyBase into TestRunner. This also helps clean-up WebTestProxyBase in preparation for extracting view/frame/widget-specific code to separate classes. - Moves |dump_selection_rect| and |dump_drag_image| from TestRunner into LayoutTestRuntimeFlags. This makes sure these flags are replicated across renderers as needed. This also means that accessors for these flags no longer need to be exposed by TestRunner. - Simplifies how WebTestDelegate communicates devices scaling factor (used when positioning a pop-up for pixel dump). This helps avoid adding a WebTestDelegate dependency to pixel_dump.h/.cc. BUG= 587175 , 595089 TBR=jochen@chromium.org Review URL: https://codereview.chromium.org/1844233004 Cr-Commit-Position: refs/heads/master@{#384921} [modify] https://crrev.com/1b546c1600f9daea517c16b29cddffe515936822/components/test_runner/BUILD.gn [modify] https://crrev.com/1b546c1600f9daea517c16b29cddffe515936822/components/test_runner/layout_dump.h [modify] https://crrev.com/1b546c1600f9daea517c16b29cddffe515936822/components/test_runner/layout_test_runtime_flags.cc [modify] https://crrev.com/1b546c1600f9daea517c16b29cddffe515936822/components/test_runner/layout_test_runtime_flags.h [add] https://crrev.com/1b546c1600f9daea517c16b29cddffe515936822/components/test_runner/pixel_dump.cc [add] https://crrev.com/1b546c1600f9daea517c16b29cddffe515936822/components/test_runner/pixel_dump.h [modify] https://crrev.com/1b546c1600f9daea517c16b29cddffe515936822/components/test_runner/test_runner.cc [modify] https://crrev.com/1b546c1600f9daea517c16b29cddffe515936822/components/test_runner/test_runner.gyp [modify] https://crrev.com/1b546c1600f9daea517c16b29cddffe515936822/components/test_runner/test_runner.h [modify] https://crrev.com/1b546c1600f9daea517c16b29cddffe515936822/components/test_runner/web_test_delegate.h [modify] https://crrev.com/1b546c1600f9daea517c16b29cddffe515936822/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/1b546c1600f9daea517c16b29cddffe515936822/components/test_runner/web_test_proxy.h [modify] https://crrev.com/1b546c1600f9daea517c16b29cddffe515936822/components/test_runner/web_test_runner.h [modify] https://crrev.com/1b546c1600f9daea517c16b29cddffe515936822/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/1b546c1600f9daea517c16b29cddffe515936822/content/shell/renderer/layout_test/blink_test_runner.h
,
Apr 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/01da260209729cc19fd30b0d234ec3ecaf3e0766 commit 01da260209729cc19fd30b0d234ec3ecaf3e0766 Author: lukasza <lukasza@chromium.org> Date: Tue Apr 05 14:51:26 2016 Extract WebViewClient implementation out of WebTestProxyBase. Overview ======== This CL extracts WebViewClient-specific parts of WebTestProxyBase into a new WebViewTestClient class. This CL is part of a sequence of CLs that make progress toward: 1) Splitting WebTestProxyBase into separate WebFrameClient / WebWidgetClient / WebViewClient focused classes 2) Removing WebTestProxyBase from public API of components/test_runner (and having WebTestProxy and WebFrameTestProxy consume instead abstract WebFrameClient / WebWidgetClient / WebViewClient interfaces). Details ======= Most of the change is mechanical move of code from WebTestProxyBase into the new WebViewTestClient (slightly tweaking method signatures as needed to make them fit WebViewClient interface - note camelCasing in particular). The new WebViewTestClient needs access to 1) WebWidget (for various widget-y things like animations) 2) WebView (this is just a way to help get RenderView pointer in BlinkTestRunner::SetFocus method, while avoiding component/test_runner -> content dependency). This CL refactors slightly how WebViewTestClient gets both the WebWidget and WebView pointers: 1) moving initialization to happen right after creation of these objects by RenderViewImpl::Initialize - when ContentRenderClient::RenderViewCreated is called 2) to initialize without depending on passing WebTestProxyBase via BlinkTestRunner::proxy_ field. 3) to pass WebWidget and WebView separately (rather than depend [inside the removed WebTestProxyBase::GetWebView] on the fact that a particular upcast from WebWidget to WebView is safe). WebViewTestClient needs a pointer back to WebTestProxyBase (to be able to get WebView and WebWidget the client corresponds to). This means that a separate WebViewTestClient is needed per WebTestProxyBase (this is different from WebFrameTestClient which depended only on global TestInterfaces, TestRunner and mocks). To accomodate this, WebTestInterface no longer owns WebViewTestClient and/or WebFrameTestClient, but instead passes ownership to WebTestProxyBase and/or WebFrameTestProxyBase. This CL also moves tracking of some test state (i.e. animation_scheduled_, speech_recognizer_ mock, etc.) from WebTestProxyBase into TestRunner (which tracks most of the other test state). This means that WebTestProxyBase::Reset method is no longer needed. BUG= 595089 Review URL: https://codereview.chromium.org/1840823002 Cr-Commit-Position: refs/heads/master@{#385173} [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/components/test_runner/BUILD.gn [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/components/test_runner/event_sender.cc [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/components/test_runner/test_runner.cc [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/components/test_runner/test_runner.gyp [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/components/test_runner/test_runner.h [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/components/test_runner/web_frame_test_client.cc [add] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/components/test_runner/web_frame_test_proxy.cc [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/components/test_runner/web_frame_test_proxy.h [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/components/test_runner/web_test_delegate.h [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/components/test_runner/web_test_interfaces.cc [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/components/test_runner/web_test_interfaces.h [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/components/test_runner/web_test_proxy.h [add] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/components/test_runner/web_view_test_client.cc [add] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/components/test_runner/web_view_test_client.h [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/content/public/test/layouttest_support.h [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/content/shell/renderer/layout_test/blink_test_runner.h [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc [modify] https://crrev.com/01da260209729cc19fd30b0d234ec3ecaf3e0766/content/test/layouttest_support.cc
,
Apr 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5c2279cfe031d3fe92687e7e83f13feeafb407c0 commit 5c2279cfe031d3fe92687e7e83f13feeafb407c0 Author: lukasza <lukasza@chromium.org> Date: Tue Apr 05 16:44:47 2016 Moving remaining mocks from WebTestProxyBase into TestRunner. This CL moves SpellCheckClient and MockCredentialManagerClient from WebTestProxyBase into TestRunner. This keeps the mocks closer to the actual usage and helps further limit the amount of code left inside WebTestProxyBase (most of the code has been moved out into WebFrameTestClient and WebViewTestClient in the few previous CLs). This CL also makes WebTestRunner responsible for installing mocks it manages into a particular WebView (i.e. this CL moves this code out of LayoutTestContentRendererClient::RenderViewCreated and into WebTestRunner::InitializeWebViewWithMocks). Note that the removed WebTestProxyBase::PostSpellCheckEvent method didn't disappear altogether, but was instead inlined into SpellCheckClient::FinishLastTextCheck. BUG= 595089 Review URL: https://codereview.chromium.org/1839853002 Cr-Commit-Position: refs/heads/master@{#385194} [modify] https://crrev.com/5c2279cfe031d3fe92687e7e83f13feeafb407c0/components/test_runner/spell_check_client.cc [modify] https://crrev.com/5c2279cfe031d3fe92687e7e83f13feeafb407c0/components/test_runner/spell_check_client.h [modify] https://crrev.com/5c2279cfe031d3fe92687e7e83f13feeafb407c0/components/test_runner/test_runner.cc [modify] https://crrev.com/5c2279cfe031d3fe92687e7e83f13feeafb407c0/components/test_runner/test_runner.h [modify] https://crrev.com/5c2279cfe031d3fe92687e7e83f13feeafb407c0/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/5c2279cfe031d3fe92687e7e83f13feeafb407c0/components/test_runner/web_test_proxy.h [modify] https://crrev.com/5c2279cfe031d3fe92687e7e83f13feeafb407c0/components/test_runner/web_test_runner.h [modify] https://crrev.com/5c2279cfe031d3fe92687e7e83f13feeafb407c0/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
,
Apr 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c935882393f5644d8e06c2fbeaeab65f9527ffcb commit c935882393f5644d8e06c2fbeaeab65f9527ffcb Author: lukasza <lukasza@chromium.org> Date: Thu Apr 07 14:43:46 2016 Replacing most of web_task.h with base::Closure + base::WeakPtrFactory. WebMethodTask was used to wrap and invoke a method pointer. This can be replaced by wrapping the method in a base::Closure instead. When WebTaskList was used as a field of an object, it would make sure that after the object's destruction, no associated callbacks / WebTasks would run. The same functionality can be accomplished via base::WeakPtrFactory which has the same effect of suppressing base::Callbacks after the WeakPtrFactory dies. There were also a few implementations of test_runner::WebTask other than test_runner::WebMethodTask - these other implementations could also be simplified by building on top of base::Callback. BUG= 595089 Review URL: https://codereview.chromium.org/1852603002 Cr-Commit-Position: refs/heads/master@{#385752} [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/event_sender.cc [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/event_sender.h [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_color_chooser.cc [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_color_chooser.h [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_web_media_stream_center.cc [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_web_media_stream_center.h [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_web_midi_accessor.cc [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_web_midi_accessor.h [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_web_speech_recognizer.cc [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_web_speech_recognizer.h [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_web_user_media_client.cc [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_web_user_media_client.h [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_webrtc_data_channel_handler.cc [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_webrtc_data_channel_handler.h [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_webrtc_dtmf_sender_handler.cc [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_webrtc_dtmf_sender_handler.h [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_webrtc_peer_connection_handler.cc [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/mock_webrtc_peer_connection_handler.h [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/spell_check_client.cc [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/spell_check_client.h [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/test_runner.cc [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/test_runner.h [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/web_task.cc [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/web_task.h [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/web_test_interfaces.cc [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/web_view_test_client.cc [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/components/test_runner/web_view_test_client.h [modify] https://crrev.com/c935882393f5644d8e06c2fbeaeab65f9527ffcb/third_party/WebKit/LayoutTests/fast/css/counters/counter-reparent-table-children-crash.html
,
Apr 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/95416be1445ef1c72a8f907b36c48d8c209c8442 commit 95416be1445ef1c72a8f907b36c48d8c209c8442 Author: lukasza <lukasza@chromium.org> Date: Thu Apr 14 15:06:33 2016 Track focused view in TestRunner instead of in BlinkTestRunner. Problem description =================== Layout tests attempt to "globally" track currently focused view - this is done in order to "defocus" previously focused view at the right time. Before this CL, the tracking was done in an instance field of a BlinkTestRunner - this is wrong, as BlinkTestRunner is not "global" (there can be multiple BlinkTestRunner objects associated with a single TestInterfaces / TestRunner pair, because BlinkTestRunner objects are created for each view). The wrongness described above was leading to test failures in https://crrev.com/1885993002 which starts to use correct BlinkTestRunner from WebViewTestClient and WebTestProxyBase. The test failures were in: - scrollbars/custom-scrollbar-inactive-pseudo.html - http/tests/notifications/click-window-focus-document.html Fix description =============== This CL moves tracking of the previously focused view from BlinkTestRunner::focused_view_ into TestRunner::previously_focused_view_. BlinkTestRunner::OnSetTestConfiguration has to go through WebTestRunner/TestRunner to correctly track view focused when starting a test. Alternative fix would be to make BlinkTestRunner::focused_view_ field static. This works (as seen in patchset 1 at https://crrev.com/1885993002/#ps1), but the fix in the current CL more correctly assigns responsibilities: - TestRunner tracks global test state - BlinkTestRunner tracks test state associated with a specific view / window (i.e. whether it is a |is_main_window_|). OTOH, I recognize that unfortunately BlinkTestRunner is currently still responsible for other globally-applicable functionality (i.e. PrintMessage). BUG= 595089 Review URL: https://codereview.chromium.org/1886013002 Cr-Commit-Position: refs/heads/master@{#387316} [modify] https://crrev.com/95416be1445ef1c72a8f907b36c48d8c209c8442/components/test_runner/test_runner.cc [modify] https://crrev.com/95416be1445ef1c72a8f907b36c48d8c209c8442/components/test_runner/test_runner.h [modify] https://crrev.com/95416be1445ef1c72a8f907b36c48d8c209c8442/components/test_runner/web_test_runner.h [modify] https://crrev.com/95416be1445ef1c72a8f907b36c48d8c209c8442/components/test_runner/web_view_test_client.cc [modify] https://crrev.com/95416be1445ef1c72a8f907b36c48d8c209c8442/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/95416be1445ef1c72a8f907b36c48d8c209c8442/content/shell/renderer/layout_test/blink_test_runner.h
,
Apr 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eebc689061ed11598010112c5e47cce7edfa80ac commit eebc689061ed11598010112c5e47cce7edfa80ac Author: lukasza <lukasza@chromium.org> Date: Thu Apr 14 16:36:07 2016 Use correct WebTestDelegate from WebViewTestClient and WebTestProxyBase. Before this CL: - WebViewTestClient would use a WebTestDelegate from the (effectively global) TestInterfaces object. - WebTestProxyBase would use a WebTestDelegate from the global LayoutTestRenderProcessObserver singleton. This WebTestDelegate (aka BlinkTestRunner) might or might not be associated with the specific view related to the given WebView / RenderView / WebViewTestClient / WebTestProxyBase. This in turn could lead to UaF (no repro at ToT - this would be trigerred after some other OOPIF-related changes that have not landed yet). This CL modifies WebTestProxyCreated function in layout_test_content_renderer_client.cc to make sure the right BlinkTestRunner is passed to the WebTestProxyBase. Other changes make sure that WebViewTestClient uses BlinkTestRunner from the associated WebTestProxyBase (rather than using one handed over from WebTestInterfaces). BUG= 595089 Review URL: https://codereview.chromium.org/1885993002 Cr-Commit-Position: refs/heads/master@{#387336} [modify] https://crrev.com/eebc689061ed11598010112c5e47cce7edfa80ac/components/test_runner/web_test_interfaces.cc [modify] https://crrev.com/eebc689061ed11598010112c5e47cce7edfa80ac/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/eebc689061ed11598010112c5e47cce7edfa80ac/components/test_runner/web_test_proxy.h [modify] https://crrev.com/eebc689061ed11598010112c5e47cce7edfa80ac/components/test_runner/web_view_test_client.cc [modify] https://crrev.com/eebc689061ed11598010112c5e47cce7edfa80ac/components/test_runner/web_view_test_client.h [modify] https://crrev.com/eebc689061ed11598010112c5e47cce7edfa80ac/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
,
Apr 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f41da4bd6ff9f8dd8a8d6e3e1416d539c8377b5a commit f41da4bd6ff9f8dd8a8d6e3e1416d539c8377b5a Author: lukasza <lukasza@chromium.org> Date: Thu Apr 14 22:33:20 2016 Move LayoutAndPaintAsyncThen to a separate compilation unit. Moving LayoutAndPaintAsyncThen to a separate compilation unit is desirable, because: 1. It helps remove code from WebTestProxyBase (WebTestProxy should delegate everything to WebViewTestClient and/or [future] WebWidgetTestClient). 2. It helps break TestRunner -> WebTestProxyBase dependency and remove TestRunner::proxy_ field. BUG= 595089 Review URL: https://codereview.chromium.org/1889673002 Cr-Commit-Position: refs/heads/master@{#387452} [modify] https://crrev.com/f41da4bd6ff9f8dd8a8d6e3e1416d539c8377b5a/components/test_runner/BUILD.gn [add] https://crrev.com/f41da4bd6ff9f8dd8a8d6e3e1416d539c8377b5a/components/test_runner/layout_and_paint_async_then.cc [add] https://crrev.com/f41da4bd6ff9f8dd8a8d6e3e1416d539c8377b5a/components/test_runner/layout_and_paint_async_then.h [modify] https://crrev.com/f41da4bd6ff9f8dd8a8d6e3e1416d539c8377b5a/components/test_runner/test_interfaces.cc [modify] https://crrev.com/f41da4bd6ff9f8dd8a8d6e3e1416d539c8377b5a/components/test_runner/test_runner.cc [modify] https://crrev.com/f41da4bd6ff9f8dd8a8d6e3e1416d539c8377b5a/components/test_runner/test_runner.gyp [modify] https://crrev.com/f41da4bd6ff9f8dd8a8d6e3e1416d539c8377b5a/components/test_runner/test_runner.h [modify] https://crrev.com/f41da4bd6ff9f8dd8a8d6e3e1416d539c8377b5a/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/f41da4bd6ff9f8dd8a8d6e3e1416d539c8377b5a/components/test_runner/web_test_proxy.h [modify] https://crrev.com/f41da4bd6ff9f8dd8a8d6e3e1416d539c8377b5a/content/shell/renderer/layout_test/blink_test_runner.cc
,
Apr 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7ab191b81769178cbbe438736b0144783ca40b49 commit 7ab191b81769178cbbe438736b0144783ca40b49 Author: lukasza <lukasza@chromium.org> Date: Thu Apr 14 22:43:25 2016 Move DumpBackForwardLists out of WebTestProxyBase. Iterating over all known test windows can be moved out of web_test_proxy_base.cc and into blink_test_runner.cc. This helps minimize the amount of code in WebTestProxyBase (which should hopefully be just a passive holder of WebView, WebViewTestClient, etc. - all logic should be implemented inside WebViewTestClient and [future] WebWidgetTestClient). BUG= 595089 Review URL: https://codereview.chromium.org/1886103002 Cr-Commit-Position: refs/heads/master@{#387465} [modify] https://crrev.com/7ab191b81769178cbbe438736b0144783ca40b49/components/test_runner/web_test_delegate.h [modify] https://crrev.com/7ab191b81769178cbbe438736b0144783ca40b49/components/test_runner/web_test_interfaces.cc [modify] https://crrev.com/7ab191b81769178cbbe438736b0144783ca40b49/components/test_runner/web_test_interfaces.h [modify] https://crrev.com/7ab191b81769178cbbe438736b0144783ca40b49/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/7ab191b81769178cbbe438736b0144783ca40b49/components/test_runner/web_test_proxy.h [modify] https://crrev.com/7ab191b81769178cbbe438736b0144783ca40b49/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/7ab191b81769178cbbe438736b0144783ca40b49/content/shell/renderer/layout_test/blink_test_runner.h
,
Apr 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8e550a0c01885ed449fd7ea4540f2d6f18762d16 commit 8e550a0c01885ed449fd7ea4540f2d6f18762d16 Author: lukasza <lukasza@chromium.org> Date: Mon Apr 18 18:34:17 2016 Explicitly initialize secondary renderers for layout tests. Secondary renderers used to be implicitly initized for layout tests in BlinkTestRunner::Navigate method, relying on comparisons with LayoutTestRenderProcessObserver::GetInstance()->main_test_runner(). Such initialization is brittle: 1. it relies on correct singling out of a specific BlinkTestRunner and designating it as a main_test_runner (right now this is the instance that happens to be created first in the given renderer process) 2. it reinitializes the test after each navigation, which may clobber previous test state changes which is especially worrisome if another BlinkTestRunner is present in the same process (with OOPIFs a BlinkTestRunner for secondary and main test window can coexist in the same renderer process). This CL starts to explicitly initialize all secondary renderers via ShellViewMsg_SetupSecondaryRenderer message, tracking them on the browser-side in BrowserTestController::HandleNewRenderFrameHost. This helps avoid the brittleness described above. Additionally explicit initialization helps to guarantee that a secondary renderer is initialized before propagating to such renderer changes of LayoutTestRuntimeFlags (planned in https://crrev.com/1878863002). Cleaning up BlinkTestRunner::Navigate method (and fixing up the target BlinkTestRunner used by LayoytTestRenderFrameObserver) enables to remove one occurence of a global field holding (an essentially non-global) BlinkTestRunner - we can now remove LayoutTestRenderProcessObserver::main_test_runner_ field (and then also remove LayoutTestRenderProcessObserver::SetMainWindow which can be simply inlined into LayoutTestContentRendererClient). BUG= 595089 Review URL: https://codereview.chromium.org/1890223002 Cr-Commit-Position: refs/heads/master@{#387963} [modify] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/content_shell.gypi [modify] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/BUILD.gn [modify] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/browser/layout_test/blink_test_controller.cc [modify] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/browser/layout_test/blink_test_controller.h [modify] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/browser/layout_test/layout_test_devtools_frontend.cc [modify] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/browser/layout_test/layout_test_devtools_frontend.h [delete] https://crrev.com/4a7a3d08a5fc3115096a78acc7e228650bc84e0f/content/shell/browser/layout_test/notify_done_forwarder.cc [delete] https://crrev.com/4a7a3d08a5fc3115096a78acc7e228650bc84e0f/content/shell/browser/layout_test/notify_done_forwarder.h [add] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/browser/layout_test/secondary_test_window_observer.cc [add] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/browser/layout_test/secondary_test_window_observer.h [modify] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/browser/shell.cc [modify] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/common/shell_messages.h [modify] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/renderer/layout_test/blink_test_runner.h [modify] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc [modify] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/renderer/layout_test/layout_test_render_frame_observer.cc [modify] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/renderer/layout_test/layout_test_render_frame_observer.h [modify] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/renderer/layout_test/layout_test_render_thread_observer.cc [modify] https://crrev.com/8e550a0c01885ed449fd7ea4540f2d6f18762d16/content/shell/renderer/layout_test/layout_test_render_thread_observer.h
,
Apr 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/335bb76db170d76ba46fa003a8838255a5d46187 commit 335bb76db170d76ba46fa003a8838255a5d46187 Author: lukasza <lukasza@chromium.org> Date: Fri Apr 22 16:44:03 2016 Use correct WebView from EventSender. EventSender provides javascript bindings for test functions that tests can use to inject input events (i.e. mouse events) into WebView. Before this CL EventSender would act on the WebView associated with the main test window, rather than acting on the WebView associated with the frame owning the javascript bindings. This could lead to UaF - i.e. when EventSender::PointerDown tries to access an already destroyed WebView (no good repro at ToT - repro would happen after other OOPIF refactorings when running in --site-per-process mode). Changes in the current CL: - EventSender's lifetime is now owned by WebViewTestProxy (rather than having EventSender owned by the global TestInterfaces object). - EventSender now uses WebView and WebTestDelegate from the correct WebViewTestProxy (rather than ones associted with the main test window). - TestInterfaces object no longer has a pointer to an EventSender object (because there is no longer a central/global EventSender object). This means having to move code that calls EventSender::Install and EventSender::set_send_wheel_gestures away from TestInterfaces. Additional changes: - Some methods of EventSender can be made private - EventSender does not need to inherit from base::SupportsWeakPtr<...> (because EventSender already has a weak_factory_ field). BUG= 595089 Review URL: https://codereview.chromium.org/1897363003 Cr-Commit-Position: refs/heads/master@{#389139} [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/components/test_runner/event_sender.cc [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/components/test_runner/event_sender.h [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/components/test_runner/test_interfaces.cc [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/components/test_runner/test_interfaces.h [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/components/test_runner/web_frame_test_client.cc [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/components/test_runner/web_frame_test_client.h [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/components/test_runner/web_frame_test_proxy.h [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/components/test_runner/web_test_interfaces.cc [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/components/test_runner/web_test_interfaces.h [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/components/test_runner/web_test_proxy.h [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/components/test_runner/web_view_test_client.cc [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/components/test_runner/web_view_test_client.h [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc [modify] https://crrev.com/335bb76db170d76ba46fa003a8838255a5d46187/content/shell/renderer/layout_test/layout_test_render_thread_observer.cc
,
Apr 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8b6d5f3c021f312bb13f4a4cf2c04bd8f1c5187f commit 8b6d5f3c021f312bb13f4a4cf2c04bd8f1c5187f Author: lukasza <lukasza@chromium.org> Date: Fri Apr 22 16:56:31 2016 Use correct WebView from TextInputController. Before this CL TextInputController would act on the WebView associated with the main test window, rather than acting on the WebView associated with the frame owning the TextInputController's javascript bindings. This could potentially lead to UaF (there is no known repro though at the moment). Changes in the current CL: - TextInputController's lifetime is now owned by WebViewTestProxy (rather than having TextInputController owned by the global TestInterfaces object). - TextInputController now uses WebView from the correct WebViewTestProxy (rather than one associted with the main test window). - TestInterfaces object no longer has a pointer to an TextInputController object (because there is no longer a central/global TextInputController object). This means having to move code that calls TextInputController::Install away from TestInterfaces. Additional changes: - TextInputController does not need to inherit from base::SupportsWeakPtr<...> (because TextInputController already has a weak_factory_ field). BUG= 595089 Review URL: https://codereview.chromium.org/1896413003 Cr-Commit-Position: refs/heads/master@{#389142} [modify] https://crrev.com/8b6d5f3c021f312bb13f4a4cf2c04bd8f1c5187f/components/test_runner/test_interfaces.cc [modify] https://crrev.com/8b6d5f3c021f312bb13f4a4cf2c04bd8f1c5187f/components/test_runner/test_interfaces.h [modify] https://crrev.com/8b6d5f3c021f312bb13f4a4cf2c04bd8f1c5187f/components/test_runner/text_input_controller.cc [modify] https://crrev.com/8b6d5f3c021f312bb13f4a4cf2c04bd8f1c5187f/components/test_runner/text_input_controller.h [modify] https://crrev.com/8b6d5f3c021f312bb13f4a4cf2c04bd8f1c5187f/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/8b6d5f3c021f312bb13f4a4cf2c04bd8f1c5187f/components/test_runner/web_test_proxy.h
,
Apr 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8ee983a2a52420c843fc4ed0de1ee3c107ef00b8 commit 8ee983a2a52420c843fc4ed0de1ee3c107ef00b8 Author: lukasza <lukasza@chromium.org> Date: Fri Apr 22 17:47:44 2016 Use correct WebView from AccessibilityController. Before this CL AccessibilityController would act on the WebView associated with the main test window, rather than acting on the WebView associated with the frame owning the AccessibilityController's javascript bindings. This could lead to UaF in AccessibilityController::NotificationReceived where it was trying to use web_view_->mainFrame on an already destroyed view (no good repro at ToT - repro would happen after other OOPIF refactorings when running in --site-per-process mode). Changes in the current CL: - AccessibilityController's lifetime is now owned by WebViewTestProxy (rather than having AccessibilityController owned by the global TestInterfaces object). - AccessibilityController now uses WebView from the correct WebViewTestProxy (rather than one associted with the main test window). - TestInterfaces object no longer has a pointer to an AccessibilityController object (because there is no longer a central/global AccessibilityController object). This means having to move code that calls AccessibilityController::Install and AccessibilityController::Reset away from TestInterfaces. Additional changes: - AccessibilityController does not need to inherit from base::SupportsWeakPtr<...> (because AccessibilityController already has a weak_factory_ field). BUG= 595089 Review URL: https://codereview.chromium.org/1903043002 Cr-Commit-Position: refs/heads/master@{#389156} [modify] https://crrev.com/8ee983a2a52420c843fc4ed0de1ee3c107ef00b8/components/test_runner/accessibility_controller.cc [modify] https://crrev.com/8ee983a2a52420c843fc4ed0de1ee3c107ef00b8/components/test_runner/accessibility_controller.h [modify] https://crrev.com/8ee983a2a52420c843fc4ed0de1ee3c107ef00b8/components/test_runner/test_interfaces.cc [modify] https://crrev.com/8ee983a2a52420c843fc4ed0de1ee3c107ef00b8/components/test_runner/test_interfaces.h [modify] https://crrev.com/8ee983a2a52420c843fc4ed0de1ee3c107ef00b8/components/test_runner/web_frame_test_client.cc [modify] https://crrev.com/8ee983a2a52420c843fc4ed0de1ee3c107ef00b8/components/test_runner/web_frame_test_client.h [modify] https://crrev.com/8ee983a2a52420c843fc4ed0de1ee3c107ef00b8/components/test_runner/web_test_interfaces.cc [modify] https://crrev.com/8ee983a2a52420c843fc4ed0de1ee3c107ef00b8/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/8ee983a2a52420c843fc4ed0de1ee3c107ef00b8/components/test_runner/web_test_proxy.h
,
Apr 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/556d013d1b49c18b396f2c5c55b07447b5f30172 commit 556d013d1b49c18b396f2c5c55b07447b5f30172 Author: lukasza <lukasza@chromium.org> Date: Fri Apr 22 19:10:26 2016 Remove no longer needed BlinkTestRunner::proxy_ field. BUG= 595089 Review URL: https://codereview.chromium.org/1911153002 Cr-Commit-Position: refs/heads/master@{#389200} [modify] https://crrev.com/556d013d1b49c18b396f2c5c55b07447b5f30172/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/556d013d1b49c18b396f2c5c55b07447b5f30172/content/shell/renderer/layout_test/blink_test_runner.h [modify] https://crrev.com/556d013d1b49c18b396f2c5c55b07447b5f30172/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
,
Apr 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5cf0f0ca0e0478113e2df9814266e6291ccea925 commit 5cf0f0ca0e0478113e2df9814266e6291ccea925 Author: lukasza <lukasza@chromium.org> Date: Tue Apr 26 02:27:43 2016 Wait until the test page is fully loaded before dumping the contents. Context - how timing of text/pixel dumps works. =============================================== Text dump or pixel dump are taken 1) immediately when testRunner.notifyDone is called or 2) when test page finishes loading (i.e. if TestRunner::topLoadingFrame was not-null when testRunner.notifyDone was called and we didn't take path (1) above - see how TestRunner::CompleteNotifyDone calls TestFinished only if !topLoadingFrame()). Context - OOPIFs support requires changes to topLoadingFrame tracking ===================================================================== Some planned refactorings change how TestRunner::topLoadingFrame works (to make sure there is only 1 top loading frame across all renderer processes). This can affect the timing of text/pixel dumps. Changes in this CL ================== So - to avoid timing problems described above, this CL makes sure the tests are explicitly timing their text/pixel dumps. BUG= 595089 Review URL: https://codereview.chromium.org/1907413003 Cr-Commit-Position: refs/heads/master@{#389670} [modify] https://crrev.com/5cf0f0ca0e0478113e2df9814266e6291ccea925/third_party/WebKit/LayoutTests/http/tests/navigation/resources/notify-done.html [modify] https://crrev.com/5cf0f0ca0e0478113e2df9814266e6291ccea925/third_party/WebKit/LayoutTests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade.html
,
Apr 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b2ad050469118ea18e7c4c12112180d3fb8ab725 commit b2ad050469118ea18e7c4c12112180d3fb8ab725 Author: lukasza <lukasza@chromium.org> Date: Wed Apr 27 15:51:42 2016 Use correct WebView from TestRunner methods called via testRunner bindings. TestRunner provides javascript bindings for test functions that tests can use for various functionality. Some of these test functions operate on global state - for example: *) testRunner.dumpAsText (test flag affecting test behavior) *) testRunner.setAllowDisplayOfInsecureContent (test flag affecting product behavior) *) testRunner.setTextSubpixelPositioning (directly interacts with product). Some of these test functions operate on a specific view - for example: *) testRunner.capturePixelsAsyncThen *) testRunner.setPageVisibility View-specific functions should operate on a view associated with the view of the frame that the javascript is calling from (rather then on the main window's view). To accomplish this the CL splits view-specific functionality of TestRunner into a separete TestRunnerForSpecificView class (with one instance per WebTestProxyBase). This CL is important to enable setting TestRunner::web_view_ only to the actual main test window (rather than to the first WebView/RenderView that happens to be created in a given renderer process) - see the planned https://crrev.com/1896623002. Without the current CL javascript calls from a DevTools window would cease to work (they would encounter a null TestRunner::web_view_ and failsafe, rather than using the DevTools view from which javascript calls them). Repro test case would be running layout tests under inspector/ directory (with or without --site-per-process flag). BUG= 595089 Review-Url: https://codereview.chromium.org/1908183002 Cr-Commit-Position: refs/heads/master@{#390088} [modify] https://crrev.com/b2ad050469118ea18e7c4c12112180d3fb8ab725/components/test_runner/test_interfaces.cc [modify] https://crrev.com/b2ad050469118ea18e7c4c12112180d3fb8ab725/components/test_runner/test_runner.cc [modify] https://crrev.com/b2ad050469118ea18e7c4c12112180d3fb8ab725/components/test_runner/test_runner.h [modify] https://crrev.com/b2ad050469118ea18e7c4c12112180d3fb8ab725/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/b2ad050469118ea18e7c4c12112180d3fb8ab725/components/test_runner/web_test_proxy.h [modify] https://crrev.com/b2ad050469118ea18e7c4c12112180d3fb8ab725/components/test_runner/web_view_test_client.cc [modify] https://crrev.com/b2ad050469118ea18e7c4c12112180d3fb8ab725/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
,
Apr 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8973c521e42739f6c2d1e1b3c8fa7c6e90753014 commit 8973c521e42739f6c2d1e1b3c8fa7c6e90753014 Author: lukasza <lukasza@chromium.org> Date: Wed Apr 27 16:32:28 2016 Make sure TestRunner::topLoadingFrame is from the main test window. Before this CL, TestRunner::web_view_ would always be set to the first WebView/RenderView created in the given renderer process. This has been interfering with TestRunner::topLoadingFrame tracking - in OOPIF modes this might be tracking a frame not belonging to the main window. BUG= 595089 Review-Url: https://codereview.chromium.org/1896623002 Cr-Commit-Position: refs/heads/master@{#390097} [modify] https://crrev.com/8973c521e42739f6c2d1e1b3c8fa7c6e90753014/components/test_runner/test_interfaces.cc [modify] https://crrev.com/8973c521e42739f6c2d1e1b3c8fa7c6e90753014/components/test_runner/test_interfaces.h [modify] https://crrev.com/8973c521e42739f6c2d1e1b3c8fa7c6e90753014/components/test_runner/test_runner.cc [modify] https://crrev.com/8973c521e42739f6c2d1e1b3c8fa7c6e90753014/components/test_runner/test_runner.h [modify] https://crrev.com/8973c521e42739f6c2d1e1b3c8fa7c6e90753014/components/test_runner/web_test_interfaces.cc [modify] https://crrev.com/8973c521e42739f6c2d1e1b3c8fa7c6e90753014/components/test_runner/web_test_interfaces.h [modify] https://crrev.com/8973c521e42739f6c2d1e1b3c8fa7c6e90753014/content/shell/renderer/layout_test/blink_test_runner.cc [modify] https://crrev.com/8973c521e42739f6c2d1e1b3c8fa7c6e90753014/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc [modify] https://crrev.com/8973c521e42739f6c2d1e1b3c8fa7c6e90753014/content/shell/renderer/layout_test/layout_test_render_thread_observer.cc [modify] https://crrev.com/8973c521e42739f6c2d1e1b3c8fa7c6e90753014/content/shell/renderer/layout_test/layout_test_render_thread_observer.h
,
Apr 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/10cd87645bc101283ca7f741977c68d9a1ac9644 commit 10cd87645bc101283ca7f741977c68d9a1ac9644 Author: lukasza <lukasza@chromium.org> Date: Wed Apr 27 20:03:02 2016 Split TestRunner::setTopLoadingFrame into separate "set" and "clear" methods. BUG= 595089 Review-Url: https://codereview.chromium.org/1893783002 Cr-Commit-Position: refs/heads/master@{#390160} [modify] https://crrev.com/10cd87645bc101283ca7f741977c68d9a1ac9644/components/test_runner/test_runner.cc [modify] https://crrev.com/10cd87645bc101283ca7f741977c68d9a1ac9644/components/test_runner/test_runner.h [modify] https://crrev.com/10cd87645bc101283ca7f741977c68d9a1ac9644/components/test_runner/web_frame_test_client.cc
,
Apr 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cb2ee5f8033a278716249a0ccc095ddd3f7a675f commit cb2ee5f8033a278716249a0ccc095ddd3f7a675f Author: lukasza <lukasza@chromium.org> Date: Thu Apr 28 15:37:40 2016 One and only one top-loading-frame across all renderers. This CL prevents an OOPIF from being tracked as the top-loading-frame, when another renderer already tracks the main frame as the top-loading-frame. Without this CL, BlinkTestRunner::TestFinished would be called for an OOPIF (and we would just get lucky and fail to deliver notification to the browser because of swapped-out filtering of ShellViewHostMsg_TestFinishedInSecondaryRenderer). BUG= 595089 Review-Url: https://codereview.chromium.org/1908233002 Cr-Commit-Position: refs/heads/master@{#390387} [modify] https://crrev.com/cb2ee5f8033a278716249a0ccc095ddd3f7a675f/components/test_runner/layout_test_runtime_flags.cc [modify] https://crrev.com/cb2ee5f8033a278716249a0ccc095ddd3f7a675f/components/test_runner/layout_test_runtime_flags.h [modify] https://crrev.com/cb2ee5f8033a278716249a0ccc095ddd3f7a675f/components/test_runner/test_runner.cc
,
May 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c360bb3480742cb303d9693cfc12b72a3d701693 commit c360bb3480742cb303d9693cfc12b72a3d701693 Author: lukasza <lukasza@chromium.org> Date: Tue May 03 16:20:31 2016 Moving TestRunnerForSpecificView into a separate compilation unit. BUG= 595089 Review-Url: https://codereview.chromium.org/1931833003 Cr-Commit-Position: refs/heads/master@{#391254} [modify] https://crrev.com/c360bb3480742cb303d9693cfc12b72a3d701693/components/test_runner/BUILD.gn [modify] https://crrev.com/c360bb3480742cb303d9693cfc12b72a3d701693/components/test_runner/test_common.cc [modify] https://crrev.com/c360bb3480742cb303d9693cfc12b72a3d701693/components/test_runner/test_common.h [modify] https://crrev.com/c360bb3480742cb303d9693cfc12b72a3d701693/components/test_runner/test_runner.cc [modify] https://crrev.com/c360bb3480742cb303d9693cfc12b72a3d701693/components/test_runner/test_runner.gyp [modify] https://crrev.com/c360bb3480742cb303d9693cfc12b72a3d701693/components/test_runner/test_runner.h [add] https://crrev.com/c360bb3480742cb303d9693cfc12b72a3d701693/components/test_runner/test_runner_for_specific_view.cc [add] https://crrev.com/c360bb3480742cb303d9693cfc12b72a3d701693/components/test_runner/test_runner_for_specific_view.h [modify] https://crrev.com/c360bb3480742cb303d9693cfc12b72a3d701693/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/c360bb3480742cb303d9693cfc12b72a3d701693/components/test_runner/web_view_test_client.cc
,
May 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/df92022f3fc9037b1a584f2badc4ef308b705dfe commit df92022f3fc9037b1a584f2badc4ef308b705dfe Author: lukasza <lukasza@chromium.org> Date: Tue May 03 16:42:31 2016 Extract WebWidgetTestClient out of WebTestProxyBase and WebViewTestClient. This CL is part of a sequence of CLs that make progress toward: splitting WebTestProxyBase into separate WebFrameClient / WebWidgetClient / WebViewClient focused classes. This CL is similar to https://crrev.com/1840823002 (which extracted WebViewTestClient out of WebTestProxyBase) and https://crrev.com/1847893002 (which extracted WebFrameTestClient out of WebTestProxyBase). This CL in turn extracts WebWidgetTestClient out of WebTestProxyBase (and also out of WebViewTestClient accidentally included quite a few methods which were overriding WebWidgetClient, rather than WebViewClient). BUG= 595089 Review-Url: https://codereview.chromium.org/1935593004 Cr-Commit-Position: refs/heads/master@{#391264} [modify] https://crrev.com/df92022f3fc9037b1a584f2badc4ef308b705dfe/components/test_runner/BUILD.gn [modify] https://crrev.com/df92022f3fc9037b1a584f2badc4ef308b705dfe/components/test_runner/test_runner.cc [modify] https://crrev.com/df92022f3fc9037b1a584f2badc4ef308b705dfe/components/test_runner/test_runner.gyp [modify] https://crrev.com/df92022f3fc9037b1a584f2badc4ef308b705dfe/components/test_runner/test_runner.h [modify] https://crrev.com/df92022f3fc9037b1a584f2badc4ef308b705dfe/components/test_runner/web_test_interfaces.cc [modify] https://crrev.com/df92022f3fc9037b1a584f2badc4ef308b705dfe/components/test_runner/web_test_interfaces.h [modify] https://crrev.com/df92022f3fc9037b1a584f2badc4ef308b705dfe/components/test_runner/web_test_proxy.cc [modify] https://crrev.com/df92022f3fc9037b1a584f2badc4ef308b705dfe/components/test_runner/web_test_proxy.h [modify] https://crrev.com/df92022f3fc9037b1a584f2badc4ef308b705dfe/components/test_runner/web_view_test_client.cc [modify] https://crrev.com/df92022f3fc9037b1a584f2badc4ef308b705dfe/components/test_runner/web_view_test_client.h [add] https://crrev.com/df92022f3fc9037b1a584f2badc4ef308b705dfe/components/test_runner/web_widget_test_client.cc [add] https://crrev.com/df92022f3fc9037b1a584f2badc4ef308b705dfe/components/test_runner/web_widget_test_client.h [modify] https://crrev.com/df92022f3fc9037b1a584f2badc4ef308b705dfe/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc [modify] https://crrev.com/df92022f3fc9037b1a584f2badc4ef308b705dfe/third_party/WebKit/public/web/WebWidgetClient.h
,
May 18 2016
,
May 18 2016
Deprecating component:Blink>LayoutTests, to use label Test=Layout instead. Merging these to component:Blink for the Blink rotation to pick up and re-triage as appropriate.
,
May 24 2016
,
Jun 8 2016
,
Oct 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bdc126c7556aa6499acefbd0b3b4945cb46d08c3 commit bdc126c7556aa6499acefbd0b3b4945cb46d08c3 Author: lukasza <lukasza@chromium.org> Date: Tue Oct 04 20:20:51 2016 Do not store TestRunner* in WebViewTestClient and/or WebFrameTestClient. Instead of storing borrowed pointer to TestRunner, WebFrameTestClient and WebViewTestClient can both obtain the same pointer via another (also borrowed) pointer they already have - one to WebViewTestProxyBase. Less fields to worry about (e.g. wrt lifetime management) is good. In the long-term we also should remove |delegate_| field from WebFrameTestClient (responsible for UaF in https://crbug.com/606594 ). BUG= 595089 Review-Url: https://codereview.chromium.org/2386683002 Cr-Commit-Position: refs/heads/master@{#422901} [modify] https://crrev.com/bdc126c7556aa6499acefbd0b3b4945cb46d08c3/components/test_runner/web_frame_test_client.cc [modify] https://crrev.com/bdc126c7556aa6499acefbd0b3b4945cb46d08c3/components/test_runner/web_frame_test_client.h [modify] https://crrev.com/bdc126c7556aa6499acefbd0b3b4945cb46d08c3/components/test_runner/web_test_interfaces.cc [modify] https://crrev.com/bdc126c7556aa6499acefbd0b3b4945cb46d08c3/components/test_runner/web_view_test_client.cc [modify] https://crrev.com/bdc126c7556aa6499acefbd0b3b4945cb46d08c3/components/test_runner/web_view_test_client.h
,
Mar 30 2017
lukasza@, what is left to do on this bug? Are the remaining tasks specific enough that they could be split up into separate bugs?
,
Apr 11 2017
Let's just mark this as fixed and (if needed) open smaller bugs if any remaining work pops up. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by lukasza@chromium.org
, Mar 15 2016