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

Issue 771800 link

Starred by 2 users
Status: WontFix
Owner:
█ shanmug...@samsung.com
Last visit > 30 days ago
Closed: May 2018
Cc:
editing-dev@chromium.org
style-bugs@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Introduce PageTestBase

Project Member Reported by xiaoche...@chromium.org, Oct 4 2017 
The two gtest base classes share a lot of common logic.

Both RenderingTest (in core/layout) and EditingTestBase (in core/editing/testing) allow setting up an empty page of size 800*600, setting up body html, etc.

It is better to unify them somehow. For example, EditingTestBase should inherit from RenderingTest to share the page setup logic.

Comment 1 by yosin@chromium.org, Oct 5 2017

Components: Blink>CSS Blink>DOM
Summary: Introduce PageTestBase (was: Unify RenderingTest and EditingTestBase)
core/css, core/dom, core/exported, core/frame and etc. also has

  void SetUp() override {
    dummy_page_holder_ = DummyPageHolder::Create(IntSize(800, 600));
  }

and GetDocument(), thus it is better has base class which creates DummyPageHolder
in core/testing/

Comment 2 by shend@chromium.org, Oct 5 2017

Labels: Objective Update-Quarterly

Comment 3 by shanmug...@samsung.com, Oct 9 2017

Owner: shanmug...@samsung.com
Status: Assigned (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 13 2017 

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

commit 07513e0abea6e130fc627a17da630df5f296e8e0
Author: Shanmuga Pandi M <shanmuga.m@samsung.com>
Date: Fri Oct 13 08:49:06 2017

Introduce PageTestBase for test common logic

Currently gtest base classes share lot of common logic.
So better to introduce new base class for common logic codes.

Bug:  771800 
Change-Id: I103ea1bd6eb466aa43380fb9d5d1c10ee458e79d
Reviewed-on: https://chromium-review.googlesource.com/716276
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Shanmuga Pandi <shanmuga.m@samsung.com>
Cr-Commit-Position: refs/heads/master@{#508648}
[modify] https://crrev.com/07513e0abea6e130fc627a17da630df5f296e8e0/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/07513e0abea6e130fc627a17da630df5f296e8e0/third_party/WebKit/Source/core/editing/GranularityStrategyTest.cpp
[modify] https://crrev.com/07513e0abea6e130fc627a17da630df5f296e8e0/third_party/WebKit/Source/core/editing/testing/EditingTestBase.cpp
[modify] https://crrev.com/07513e0abea6e130fc627a17da630df5f296e8e0/third_party/WebKit/Source/core/editing/testing/EditingTestBase.h
[add] https://crrev.com/07513e0abea6e130fc627a17da630df5f296e8e0/third_party/WebKit/Source/core/testing/PageTestBase.cpp
[add] https://crrev.com/07513e0abea6e130fc627a17da630df5f296e8e0/third_party/WebKit/Source/core/testing/PageTestBase.h

Comment 8 by shanmug...@samsung.com, Oct 27 2017

Status: Started (was: Assigned)
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 17 2017 

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

commit 85776e460ef7c3d2ea054bd0c1adeb6e86b92df3
Author: Shanmuga Pandi M <shanmuga.m@samsung.com>
Date: Fri Nov 17 07:21:05 2017

Unify test classes to use PageTestBase

Bug:  771800 
Change-Id: Ica4742d1b849e2c60bb195b538fd6bca895a6339
Reviewed-on: https://chromium-review.googlesource.com/770928
Commit-Queue: Shanmuga Pandi <shanmuga.m@samsung.com>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517334}
[modify] https://crrev.com/85776e460ef7c3d2ea054bd0c1adeb6e86b92df3/third_party/WebKit/Source/core/dom/DocumentStatisticsCollectorTest.cpp
[modify] https://crrev.com/85776e460ef7c3d2ea054bd0c1adeb6e86b92df3/third_party/WebKit/Source/core/dom/DocumentTest.cpp
[modify] https://crrev.com/85776e460ef7c3d2ea054bd0c1adeb6e86b92df3/third_party/WebKit/Source/core/html/canvas/CanvasFontCacheTest.cpp
[modify] https://crrev.com/85776e460ef7c3d2ea054bd0c1adeb6e86b92df3/third_party/WebKit/Source/core/html/media/AutoplayUmaHelperTest.cpp
[modify] https://crrev.com/85776e460ef7c3d2ea054bd0c1adeb6e86b92df3/third_party/WebKit/Source/core/input/EventHandlerTest.cpp
[modify] https://crrev.com/85776e460ef7c3d2ea054bd0c1adeb6e86b92df3/third_party/WebKit/Source/core/testing/PageTestBase.cpp
[modify] https://crrev.com/85776e460ef7c3d2ea054bd0c1adeb6e86b92df3/third_party/WebKit/Source/core/testing/PageTestBase.h
[modify] https://crrev.com/85776e460ef7c3d2ea054bd0c1adeb6e86b92df3/third_party/WebKit/Source/modules/canvas/canvas2d/CanvasRenderingContext2DAPITest.cpp
[modify] https://crrev.com/85776e460ef7c3d2ea054bd0c1adeb6e86b92df3/third_party/WebKit/Source/modules/document_metadata/CopylessPasteExtractorTest.cpp

Comment 12 by xiaoche...@chromium.org, Nov 17 2017 

An idea: we can also unify some setter functions into PageTestBase

An example is EditingTestBase::SetBodyContent and RenderingTest::SetBodyInnerHTML, which can can be unified

We also have some other setters of, for example, innerHTML of documentElement, innerHTML of STYLE (in HEAD), ...

And possibly some getters, like GetElementById
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 27 2017 

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

commit e08748f1f4efc66b026e50b8b195656100c6a7a4
Author: Shanmuga Pandi M <shanmuga.m@samsung.com>
Date: Mon Nov 27 07:35:24 2017

Move more helper functions to PageTestBase

This patch moves/adds below functions to PageTestBase.
* GetStyleEngine()
* EditingTestBase::SetBodyContent
* RenderingTest::SetBodyInnerHTML
* GetElementById
* UpdateAllLifecyclePhases

Bug:  771800 
Change-Id: I9084bc75f45bcaee72d9bfeb5d9d26188cd58bc8
Reviewed-on: https://chromium-review.googlesource.com/784660
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Shanmuga Pandi <shanmuga.m@samsung.com>
Cr-Commit-Position: refs/heads/master@{#519235}
[modify] https://crrev.com/e08748f1f4efc66b026e50b8b195656100c6a7a4/third_party/WebKit/Source/core/css/ActiveStyleSheetsTest.cpp
[modify] https://crrev.com/e08748f1f4efc66b026e50b8b195656100c6a7a4/third_party/WebKit/Source/core/css/AffectedByPseudoTest.cpp
[modify] https://crrev.com/e08748f1f4efc66b026e50b8b195656100c6a7a4/third_party/WebKit/Source/core/editing/testing/EditingTestBase.cpp
[modify] https://crrev.com/e08748f1f4efc66b026e50b8b195656100c6a7a4/third_party/WebKit/Source/core/editing/testing/EditingTestBase.h
[modify] https://crrev.com/e08748f1f4efc66b026e50b8b195656100c6a7a4/third_party/WebKit/Source/core/layout/LayoutTestHelper.h
[modify] https://crrev.com/e08748f1f4efc66b026e50b8b195656100c6a7a4/third_party/WebKit/Source/core/testing/PageTestBase.cpp
[modify] https://crrev.com/e08748f1f4efc66b026e50b8b195656100c6a7a4/third_party/WebKit/Source/core/testing/PageTestBase.h
Project Member

Comment 14 by bugdroid1@chromium.org, Nov 29 2017 

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

commit b380fae11c46a727ab0e4b76f68bd7a83bd3b1da
Author: Shanmuga Pandi M <shanmuga.m@samsung.com>
Date: Wed Nov 29 05:01:45 2017

Unify test classes to use PageTestBase

Bug:  771800 
Change-Id: Ib6dc1341e87f8c7901e7c09b610ca8a72b99396c
Reviewed-on: https://chromium-review.googlesource.com/790060
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Shanmuga Pandi <shanmuga.m@samsung.com>
Cr-Commit-Position: refs/heads/master@{#520009}
[modify] https://crrev.com/b380fae11c46a727ab0e4b76f68bd7a83bd3b1da/third_party/WebKit/Source/core/animation/AnimationInputHelpersTest.cpp
[modify] https://crrev.com/b380fae11c46a727ab0e4b76f68bd7a83bd3b1da/third_party/WebKit/Source/core/animation/EffectStackTest.cpp
[modify] https://crrev.com/b380fae11c46a727ab0e4b76f68bd7a83bd3b1da/third_party/WebKit/Source/core/dom/ElementVisibilityObserverTest.cpp
[modify] https://crrev.com/b380fae11c46a727ab0e4b76f68bd7a83bd3b1da/third_party/WebKit/Source/core/dom/events/EventPathTest.cpp
[modify] https://crrev.com/b380fae11c46a727ab0e4b76f68bd7a83bd3b1da/third_party/WebKit/Source/core/html/custom/CustomElementRegistryTest.cpp
[modify] https://crrev.com/b380fae11c46a727ab0e4b76f68bd7a83bd3b1da/third_party/WebKit/Source/core/html/custom/CustomElementUpgradeSorterTest.cpp
[modify] https://crrev.com/b380fae11c46a727ab0e4b76f68bd7a83bd3b1da/third_party/WebKit/Source/core/html/forms/ExternalPopupMenuTest.cpp
Project Member

Comment 15 by bugdroid1@chromium.org, Dec 5 2017 

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

commit 8da418842b28baaebff1d8e4c588c7853f3b1570
Author: Shanmuga Pandi M <shanmuga.m@samsung.com>
Date: Tue Dec 05 07:57:37 2017

Unify test classes to use PageTestBase

Bug:  771800 
Change-Id: I7d5eb8b4049aaa3536723fd377a54b76929837a5
Reviewed-on: https://chromium-review.googlesource.com/800090
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521635}
[modify] https://crrev.com/8da418842b28baaebff1d8e4c588c7853f3b1570/third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp
[modify] https://crrev.com/8da418842b28baaebff1d8e4c588c7853f3b1570/third_party/WebKit/Source/core/animation/DocumentTimelineTest.cpp
[modify] https://crrev.com/8da418842b28baaebff1d8e4c588c7853f3b1570/third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMapTest.cpp
[modify] https://crrev.com/8da418842b28baaebff1d8e4c588c7853f3b1570/third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreatorTest.cpp
[modify] https://crrev.com/8da418842b28baaebff1d8e4c588c7853f3b1570/third_party/WebKit/Source/core/loader/resource/CSSStyleSheetResourceTest.cpp
[modify] https://crrev.com/8da418842b28baaebff1d8e4c588c7853f3b1570/third_party/WebKit/Source/core/testing/PageTestBase.cpp
[modify] https://crrev.com/8da418842b28baaebff1d8e4c588c7853f3b1570/third_party/WebKit/Source/core/testing/PageTestBase.h
[modify] https://crrev.com/8da418842b28baaebff1d8e4c588c7853f3b1570/third_party/WebKit/Source/core/workers/DedicatedWorkerTest.cpp

Comment 16 by nainar@chromium.org, Dec 6 2017

Labels: -Update-Quarterly
Project Member

Comment 17 by bugdroid1@chromium.org, Dec 12 2017 

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

commit e389ece656fb71c9727e400a2fcd4ac3c494cc24
Author: Shanmuga Pandi M <shanmuga.m@samsung.com>
Date: Tue Dec 12 11:38:13 2017

Unify test classes to use PageTestBase

Bug:  771800 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I710238913e0400c1eabb24351116ab445c79739a
Reviewed-on: https://chromium-review.googlesource.com/821812
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Shanmuga Pandi <shanmuga.m@samsung.com>
Cr-Commit-Position: refs/heads/master@{#523405}
[modify] https://crrev.com/e389ece656fb71c9727e400a2fcd4ac3c494cc24/third_party/WebKit/Source/core/dom/FlatTreeTraversalTest.cpp
[modify] https://crrev.com/e389ece656fb71c9727e400a2fcd4ac3c494cc24/third_party/WebKit/Source/core/dom/ng/flat_tree_traversal_ng_test.cc
[modify] https://crrev.com/e389ece656fb71c9727e400a2fcd4ac3c494cc24/third_party/WebKit/Source/core/frame/FrameTest.cpp
[modify] https://crrev.com/e389ece656fb71c9727e400a2fcd4ac3c494cc24/third_party/WebKit/Source/core/html/HTMLEmbedElementTest.cpp
[modify] https://crrev.com/e389ece656fb71c9727e400a2fcd4ac3c494cc24/third_party/WebKit/Source/core/html/parser/HTMLDocumentParserTest.cpp
[modify] https://crrev.com/e389ece656fb71c9727e400a2fcd4ac3c494cc24/third_party/WebKit/Source/core/page/FocusControllerTest.cpp
[modify] https://crrev.com/e389ece656fb71c9727e400a2fcd4ac3c494cc24/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp
[modify] https://crrev.com/e389ece656fb71c9727e400a2fcd4ac3c494cc24/third_party/WebKit/Source/core/testing/PageTestBase.cpp
[modify] https://crrev.com/e389ece656fb71c9727e400a2fcd4ac3c494cc24/third_party/WebKit/Source/core/testing/PageTestBase.h
Project Member

Comment 18 by bugdroid1@chromium.org, Dec 20 2017 

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

commit c172a5aa1e3a8d640f216f6663bc323048d89779
Author: Shanmuga Pandi M <shanmuga.m@samsung.com>
Date: Wed Dec 20 11:10:06 2017

Unify test classes to use PageTestBase

Bug:  771800 
Change-Id: Id28d95a3afac31ee33711f99857f7af0b9d5e1d7
Reviewed-on: https://chromium-review.googlesource.com/836328
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Shanmuga Pandi <shanmuga.m@samsung.com>
Cr-Commit-Position: refs/heads/master@{#525305}
[modify] https://crrev.com/c172a5aa1e3a8d640f216f6663bc323048d89779/third_party/WebKit/Source/modules/animationworklet/AnimationWorkletGlobalScopeTest.cpp
[modify] https://crrev.com/c172a5aa1e3a8d640f216f6663bc323048d89779/third_party/WebKit/Source/modules/animationworklet/AnimationWorkletThreadTest.cpp
[modify] https://crrev.com/c172a5aa1e3a8d640f216f6663bc323048d89779/third_party/WebKit/Source/modules/fetch/BlobBytesConsumerTest.cpp
[modify] https://crrev.com/c172a5aa1e3a8d640f216f6663bc323048d89779/third_party/WebKit/Source/modules/fetch/BytesConsumerForDataConsumerHandleTest.cpp
[modify] https://crrev.com/c172a5aa1e3a8d640f216f6663bc323048d89779/third_party/WebKit/Source/modules/webaudio/AudioContextTest.cpp
[modify] https://crrev.com/c172a5aa1e3a8d640f216f6663bc323048d89779/third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScopeTest.cpp
[modify] https://crrev.com/c172a5aa1e3a8d640f216f6663bc323048d89779/third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp
Project Member

Comment 19 by bugdroid1@chromium.org, Dec 21 2017 

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

commit b772cf50ccb213054260916742725765ef1e61d6
Author: Shanmuga Pandi M <shanmuga.m@samsung.com>
Date: Thu Dec 21 14:11:20 2017

Unify test classes to use PageTestBase

Bug:  771800 
Change-Id: I4788313edb4afe8392e2c1173dcbda91ea26291a
Reviewed-on: https://chromium-review.googlesource.com/826204
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Shanmuga Pandi <shanmuga.m@samsung.com>
Cr-Commit-Position: refs/heads/master@{#525687}
[modify] https://crrev.com/b772cf50ccb213054260916742725765ef1e61d6/third_party/WebKit/Source/core/html/HTMLImageElementTest.cpp
[modify] https://crrev.com/b772cf50ccb213054260916742725765ef1e61d6/third_party/WebKit/Source/core/html/HTMLLinkElementTest.cpp
[modify] https://crrev.com/b772cf50ccb213054260916742725765ef1e61d6/third_party/WebKit/Source/core/html/forms/HTMLFormControlElementTest.cpp
[modify] https://crrev.com/b772cf50ccb213054260916742725765ef1e61d6/third_party/WebKit/Source/core/html/forms/HTMLInputElementTest.cpp
[modify] https://crrev.com/b772cf50ccb213054260916742725765ef1e61d6/third_party/WebKit/Source/core/html/forms/HTMLSelectElementTest.cpp
[modify] https://crrev.com/b772cf50ccb213054260916742725765ef1e61d6/third_party/WebKit/Source/core/html/media/HTMLMediaElementEventListenersTest.cpp
[modify] https://crrev.com/b772cf50ccb213054260916742725765ef1e61d6/third_party/WebKit/Source/core/html/parser/HTMLPreloadScannerTest.cpp
[modify] https://crrev.com/b772cf50ccb213054260916742725765ef1e61d6/third_party/WebKit/Source/core/html/parser/HTMLResourcePreloaderTest.cpp
Project Member

Comment 23 by bugdroid1@chromium.org, Feb 8 2018 

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

commit 4cbd9a5962a0b48d52c9b7cf3fa69c44dba9a357
Author: Shanmuga Pandi M <shanmuga.m@samsung.com>
Date: Thu Feb 08 03:16:06 2018

Unify test classes to use PageTestBase

Bug:  771800 
Change-Id: Ic3c8f24438ff4831fc9d7f1133f210a943485327
Reviewed-on: https://chromium-review.googlesource.com/904382
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535285}
[modify] https://crrev.com/4cbd9a5962a0b48d52c9b7cf3fa69c44dba9a357/third_party/WebKit/Source/core/animation/KeyframeEffectTest.cpp
[modify] https://crrev.com/4cbd9a5962a0b48d52c9b7cf3fa69c44dba9a357/third_party/WebKit/Source/core/css/MediaValuesInitialViewportTest.cpp
[modify] https://crrev.com/4cbd9a5962a0b48d52c9b7cf3fa69c44dba9a357/third_party/WebKit/Source/core/dom/LiveNodeListRegistryTest.cpp
[modify] https://crrev.com/4cbd9a5962a0b48d52c9b7cf3fa69c44dba9a357/third_party/WebKit/Source/core/fetch/BytesConsumerTest.cpp
[modify] https://crrev.com/4cbd9a5962a0b48d52c9b7cf3fa69c44dba9a357/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoaderTest.cpp
[modify] https://crrev.com/4cbd9a5962a0b48d52c9b7cf3fa69c44dba9a357/third_party/WebKit/Source/core/workers/MainThreadWorkletTest.cpp

Comment 25 by dcheng@chromium.org, May 3 2018

Status: WontFix (was: Started)
New tests should not use PageTestBase. Internally, it uses DummyPageHolder. This is problematic because it closely, but doesn't quite match, many expectations that the blink core <-> blink public API bridge make. This makes it hard to clean up code around frames.

I'm actively working on converting tests that use this over to use FrameTestHelpers::WebViewHelper: see issue 734748.
►

Sign in to add a comment