Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
242783003:
cc: Add testing stubs for proxy test (Closed)
|
Created:
6 years, 8 months ago by simonhong Modified:
6 years, 7 months ago CC:
chromium-reviews, cc-bugs_chromium.org, hyojun.im_lge.com Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
Descriptioncc: Add testing stubs for proxy test
This is initial patch for proxy unittests.
New testing stubs are added to LayerTreeTest.
R=brianderson@chromium.org, danakj@chomium.org
BUG=356832
TEST=*ProxyTest*
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272051
Patch Set 1 #Patch Set 2 : Use LayerTreeTest harness #
Total comments: 8
Patch Set 3 : Integrate ThreadProxyTestAPI into LayerTreeTest #Patch Set 4 : Rebased #Patch Set 5 : Rebased #
Total comments: 14
Patch Set 6 : Comment addressed #
Total comments: 2
Patch Set 7 : Move impl() & main() accessors into ThreadProxyTest #Patch Set 8 : Rebased #Patch Set 9 : Makes proxy test more general for single & threaded proxy #
Total comments: 1
Patch Set 10 : Add more tests #Patch Set 11 : Use NON_EXPORTED_BASE for non-exported base class #
Messages
Total messages: 36 (0 generated)
Dear brain, I referenced test pattern used by LayerTreeTest. Please take a look.
This looks a lot more like the LayerTreeTest harness than I was expecting. Is there a reason to not just use the LayerTreeTest harness for these tests? You could make another cc/trees/layer_tree_host_unittest_proxy.cc file and group all the tests that way?
On 2014/04/22 15:03:49, danakj wrote: > This looks a lot more like the LayerTreeTest harness than I was expecting. Is > there a reason to not just use the LayerTreeTest harness for these tests? You > could make another cc/trees/layer_tree_host_unittest_proxy.cc file and group all > the tests that way? Yep, testing pattern is very similar with LayerTreeTest. The reason why I created new one is there are many fake subclasses such as LayerTreHostImiplForTesting, LayerTreehostForTesting in layer_tree_test.cc For proxy test, these classes are not used. So, I created new one for proxy test. As you suggest, we can reuse LayerTreeTest by subclassing LayerTreeTest to use another fake subclasses of LayerTreeHost, ThreadProxy or SingleThreadProxy. How about refactoring layer_tree_test.[cc|h] to reuse LayerTreeTest with other test? (ex, separating LayerTreeTest into another files)
On Tue, Apr 22, 2014 at 8:24 PM, <simonhong@chromium.org> wrote: > On 2014/04/22 15:03:49, danakj wrote: > >> This looks a lot more like the LayerTreeTest harness than I was >> expecting. Is >> there a reason to not just use the LayerTreeTest harness for these tests? >> You >> could make another cc/trees/layer_tree_host_unittest_proxy.cc file and >> group >> > all > >> the tests that way? >> > > Yep, testing pattern is very similar with LayerTreeTest. > The reason why I created new one is there are many fake subclasses such as > LayerTreHostImiplForTesting, LayerTreehostForTesting in layer_tree_test.cc > For proxy test, these classes are not used. So, I created new one for proxy > test. > Why does it matter if you're not using all the of the fake capabilities for the test. No one test is making use of all the hooks. > As you suggest, we can reuse LayerTreeTest by subclassing LayerTreeTest to > use > another > fake subclasses of LayerTreeHost, ThreadProxy or SingleThreadProxy. > How about refactoring layer_tree_test.[cc|h] to reuse LayerTreeTest with > other > test? > (ex, separating LayerTreeTest into another files) > What will this accomplish? Can you just add hooks to LayerTreeTest that you need, and ignore ones you don't? > > > https://codereview.chromium.org/242783003/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2014/04/23 14:29:52, danakj wrote: > On Tue, Apr 22, 2014 at 8:24 PM, <mailto:simonhong@chromium.org> wrote: > > > On 2014/04/22 15:03:49, danakj wrote: > > > >> This looks a lot more like the LayerTreeTest harness than I was > >> expecting. Is > >> there a reason to not just use the LayerTreeTest harness for these tests? > >> You > >> could make another cc/trees/layer_tree_host_unittest_proxy.cc file and > >> group > >> > > all > > > >> the tests that way? > >> > > > > Yep, testing pattern is very similar with LayerTreeTest. > > The reason why I created new one is there are many fake subclasses such as > > LayerTreHostImiplForTesting, LayerTreehostForTesting in layer_tree_test.cc > > For proxy test, these classes are not used. So, I created new one for proxy > > test. > > > > Why does it matter if you're not using all the of the fake capabilities for > the test. No one test is making use of all the hooks. > > > > As you suggest, we can reuse LayerTreeTest by subclassing LayerTreeTest to > > use > > another > > fake subclasses of LayerTreeHost, ThreadProxy or SingleThreadProxy. > > How about refactoring layer_tree_test.[cc|h] to reuse LayerTreeTest with > > other > > test? > > (ex, separating LayerTreeTest into another files) > > > > What will this accomplish? Can you just add hooks to LayerTreeTest that you > need, and ignore ones you don't? To test proxy behavior, we not only need new hooks but also need new fake ThreadProxy to inject these new hooks into ThreadProxy. To do this, new fake LayerTreeHost is needed because ThreadProxy is tightly coupled with LTH. > > > > > > > > https://codereview.chromium.org/242783003/ > > > > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org.
On Wed, Apr 23, 2014 at 5:08 PM, <simonhong@chromium.org> wrote: > On 2014/04/23 14:29:52, danakj wrote: > > On Tue, Apr 22, 2014 at 8:24 PM, <mailto:simonhong@chromium.org> wrote: >> > > > On 2014/04/22 15:03:49, danakj wrote: >> > >> >> This looks a lot more like the LayerTreeTest harness than I was >> >> expecting. Is >> >> there a reason to not just use the LayerTreeTest harness for these >> tests? >> >> You >> >> could make another cc/trees/layer_tree_host_unittest_proxy.cc file and >> >> group >> >> >> > all >> > >> >> the tests that way? >> >> >> > >> > Yep, testing pattern is very similar with LayerTreeTest. >> > The reason why I created new one is there are many fake subclasses such >> as >> > LayerTreHostImiplForTesting, LayerTreehostForTesting in >> layer_tree_test.cc >> > For proxy test, these classes are not used. So, I created new one for >> proxy >> > test. >> > >> > > Why does it matter if you're not using all the of the fake capabilities >> for >> the test. No one test is making use of all the hooks. >> > > > > As you suggest, we can reuse LayerTreeTest by subclassing LayerTreeTest >> to >> > use >> > another >> > fake subclasses of LayerTreeHost, ThreadProxy or SingleThreadProxy. >> > How about refactoring layer_tree_test.[cc|h] to reuse LayerTreeTest with >> > other >> > test? >> > (ex, separating LayerTreeTest into another files) >> > >> > > What will this accomplish? Can you just add hooks to LayerTreeTest that >> you >> need, and ignore ones you don't? >> > > To test proxy behavior, we not only need new hooks but also need new fake > ThreadProxy > to inject these new hooks into ThreadProxy. > To do this, new fake LayerTreeHost is needed because ThreadProxy is tightly > coupled > with LTH. > Sorry for being dense just wanna make sure we're doing what makes sense. You're talking about the LayerTreeHostForProxyTest class, right? I don't think I see anything that sets it apart from the LayerTreeTest version, other than it being more simple at the moment. Can you point out what I'm missing, or maybe give a concrete example of something you'd be adding to it that couldn't be done with LayerTreeTest? > > > > > >> > >> > https://codereview.chromium.org/242783003/ >> > >> > > To unsubscribe from this group and stop receiving emails from it, send an >> > email > >> to mailto:chromium-reviews+unsubscribe@chromium.org. >> > > > https://codereview.chromium.org/242783003/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2014/04/23 21:17:24, danakj wrote: > On Wed, Apr 23, 2014 at 5:08 PM, <mailto:simonhong@chromium.org> wrote: > > > On 2014/04/23 14:29:52, danakj wrote: > > > > On Tue, Apr 22, 2014 at 8:24 PM, <mailto:simonhong@chromium.org> wrote: > >> > > > > > On 2014/04/22 15:03:49, danakj wrote: > >> > > >> >> This looks a lot more like the LayerTreeTest harness than I was > >> >> expecting. Is > >> >> there a reason to not just use the LayerTreeTest harness for these > >> tests? > >> >> You > >> >> could make another cc/trees/layer_tree_host_unittest_proxy.cc file and > >> >> group > >> >> > >> > all > >> > > >> >> the tests that way? > >> >> > >> > > >> > Yep, testing pattern is very similar with LayerTreeTest. > >> > The reason why I created new one is there are many fake subclasses such > >> as > >> > LayerTreHostImiplForTesting, LayerTreehostForTesting in > >> layer_tree_test.cc > >> > For proxy test, these classes are not used. So, I created new one for > >> proxy > >> > test. > >> > > >> > > > > Why does it matter if you're not using all the of the fake capabilities > >> for > >> the test. No one test is making use of all the hooks. > >> > > > > > > > As you suggest, we can reuse LayerTreeTest by subclassing LayerTreeTest > >> to > >> > use > >> > another > >> > fake subclasses of LayerTreeHost, ThreadProxy or SingleThreadProxy. > >> > How about refactoring layer_tree_test.[cc|h] to reuse LayerTreeTest with > >> > other > >> > test? > >> > (ex, separating LayerTreeTest into another files) > >> > > >> > > > > What will this accomplish? Can you just add hooks to LayerTreeTest that > >> you > >> need, and ignore ones you don't? > >> > > > > To test proxy behavior, we not only need new hooks but also need new fake > > ThreadProxy > > to inject these new hooks into ThreadProxy. > > To do this, new fake LayerTreeHost is needed because ThreadProxy is tightly > > coupled > > with LTH. > > > > Sorry for being dense just wanna make sure we're doing what makes sense. No problem! > > You're talking about the LayerTreeHostForProxyTest class, right? I don't > think I see anything that sets it apart from the LayerTreeTest version, > other than it being more simple at the moment. Can you point out what I'm > missing, or maybe give a concrete example of something you'd be adding to > it that couldn't be done with LayerTreeTest? The different part is DoBeginTest() which is initialize fake stubs like LayerTreeHost. LayerTreeTest initialize its fake LayerTreeHost, LayerTreeHostForProxyTest also initialize new fake LayerTreeHost. Ah.. I think just modifying LayerTreeHostForTesting in layer_tree_test.cc also can do. > > > > > > > > > > > > >> > > >> > https://codereview.chromium.org/242783003/ > >> > > >> > > > > To unsubscribe from this group and stop receiving emails from it, send an > >> > > email > > > >> to mailto:chromium-reviews+unsubscribe@chromium.org. > >> > > > > > > https://codereview.chromium.org/242783003/ > > > > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org.
On Wed, Apr 23, 2014 at 5:39 PM, <simonhong@chromium.org> wrote: > On 2014/04/23 21:17:24, danakj wrote: > > On Wed, Apr 23, 2014 at 5:08 PM, <mailto:simonhong@chromium.org> wrote: >> > > > On 2014/04/23 14:29:52, danakj wrote: >> > >> > On Tue, Apr 22, 2014 at 8:24 PM, <mailto:simonhong@chromium.org> >> wrote: >> >> >> > >> > > On 2014/04/22 15:03:49, danakj wrote: >> >> > >> >> >> This looks a lot more like the LayerTreeTest harness than I was >> >> >> expecting. Is >> >> >> there a reason to not just use the LayerTreeTest harness for these >> >> tests? >> >> >> You >> >> >> could make another cc/trees/layer_tree_host_unittest_proxy.cc file >> and >> >> >> group >> >> >> >> >> > all >> >> > >> >> >> the tests that way? >> >> >> >> >> > >> >> > Yep, testing pattern is very similar with LayerTreeTest. >> >> > The reason why I created new one is there are many fake subclasses >> such >> >> as >> >> > LayerTreHostImiplForTesting, LayerTreehostForTesting in >> >> layer_tree_test.cc >> >> > For proxy test, these classes are not used. So, I created new one for >> >> proxy >> >> > test. >> >> > >> >> >> > >> > Why does it matter if you're not using all the of the fake capabilities >> >> for >> >> the test. No one test is making use of all the hooks. >> >> >> > >> > >> > > As you suggest, we can reuse LayerTreeTest by subclassing >> LayerTreeTest >> >> to >> >> > use >> >> > another >> >> > fake subclasses of LayerTreeHost, ThreadProxy or SingleThreadProxy. >> >> > How about refactoring layer_tree_test.[cc|h] to reuse LayerTreeTest >> with >> >> > other >> >> > test? >> >> > (ex, separating LayerTreeTest into another files) >> >> > >> >> >> > >> > What will this accomplish? Can you just add hooks to LayerTreeTest that >> >> you >> >> need, and ignore ones you don't? >> >> >> > >> > To test proxy behavior, we not only need new hooks but also need new >> fake >> > ThreadProxy >> > to inject these new hooks into ThreadProxy. >> > To do this, new fake LayerTreeHost is needed because ThreadProxy is >> tightly >> > coupled >> > with LTH. >> > >> > > Sorry for being dense just wanna make sure we're doing what makes sense. >> > No problem! > > > You're talking about the LayerTreeHostForProxyTest class, right? I don't >> think I see anything that sets it apart from the LayerTreeTest version, >> other than it being more simple at the moment. Can you point out what I'm >> missing, or maybe give a concrete example of something you'd be adding to >> it that couldn't be done with LayerTreeTest? >> > > The different part is DoBeginTest() which is initialize fake stubs like > LayerTreeHost. > LayerTreeTest initialize its fake LayerTreeHost, LayerTreeHostForProxyTest > also > initialize > new fake LayerTreeHost. > Ah.. > I think just modifying LayerTreeHostForTesting in layer_tree_test.cc also > can > do. OK, if that's possible it's probably going to be much more pleasant and avoid maintaining 2 test harnesses over large cc refactors. > > > > > >> > >> > >> > > >> >> > >> >> > https://codereview.chromium.org/242783003/ >> >> > >> >> >> > >> > To unsubscribe from this group and stop receiving emails from it, send >> an >> >> >> > email >> > >> >> to mailto:chromium-reviews+unsubscribe@chromium.org. >> >> >> > >> > >> > https://codereview.chromium.org/242783003/ >> > >> > > To unsubscribe from this group and stop receiving emails from it, send an >> > email > >> to mailto:chromium-reviews+unsubscribe@chromium.org. >> > > > > https://codereview.chromium.org/242783003/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
CL is updated to use LayerTreeTest. Please check again.
LGTM me on the approach, thanks. I'll let brian review the rest. https://codereview.chromium.org/242783003/diff/20001/cc/trees/layer_tree_host.h File cc/trees/layer_tree_host.h (right): https://codereview.chromium.org/242783003/diff/20001/cc/trees/layer_tree_host... cc/trees/layer_tree_host.h:312: // virtual for test. drop the comment. https://codereview.chromium.org/242783003/diff/20001/cc/trees/layer_tree_host... File cc/trees/layer_tree_host_unittest_proxy.cc (right): https://codereview.chromium.org/242783003/diff/20001/cc/trees/layer_tree_host... cc/trees/layer_tree_host_unittest_proxy.cc:13: class ThreadProxyTestAPI { You could also make accessors for these on the LayerTreeTest class, and just make them protected on ThreadProxy and expose them to LayerTreeTest via your subclass. Then you don't need friends. https://codereview.chromium.org/242783003/diff/20001/cc/trees/layer_tree_host... cc/trees/layer_tree_host_unittest_proxy.cc:18: ThreadProxy::MainThreadOnly& main() { const&? https://codereview.chromium.org/242783003/diff/20001/cc/trees/layer_tree_host... cc/trees/layer_tree_host_unittest_proxy.cc:22: ThreadProxy::CompositorThreadOnly& impl() { const&
Done. Thanks for review! https://codereview.chromium.org/242783003/diff/20001/cc/trees/layer_tree_host.h File cc/trees/layer_tree_host.h (right): https://codereview.chromium.org/242783003/diff/20001/cc/trees/layer_tree_host... cc/trees/layer_tree_host.h:312: // virtual for test. On 2014/04/24 16:03:15, danakj wrote: > drop the comment. Done. https://codereview.chromium.org/242783003/diff/20001/cc/trees/layer_tree_host... File cc/trees/layer_tree_host_unittest_proxy.cc (right): https://codereview.chromium.org/242783003/diff/20001/cc/trees/layer_tree_host... cc/trees/layer_tree_host_unittest_proxy.cc:13: class ThreadProxyTestAPI { On 2014/04/24 16:03:15, danakj wrote: > You could also make accessors for these on the LayerTreeTest class, and just > make them protected on ThreadProxy and expose them to LayerTreeTest via your > subclass. Then you don't need friends. Done. https://codereview.chromium.org/242783003/diff/20001/cc/trees/layer_tree_host... cc/trees/layer_tree_host_unittest_proxy.cc:18: ThreadProxy::MainThreadOnly& main() { On 2014/04/24 16:03:15, danakj wrote: > const&? Done. https://codereview.chromium.org/242783003/diff/20001/cc/trees/layer_tree_host... cc/trees/layer_tree_host_unittest_proxy.cc:22: ThreadProxy::CompositorThreadOnly& impl() { On 2014/04/24 16:03:15, danakj wrote: > const& Done.
Dear brian, Please check again. Many thanks!
Kindly ping to brian :)
I was originally imagining a more stand alone proxy test harness, but re-using the existing LTH harness seems like it's going to work really well. I really like the direction this patch is going. Apologies for the slow review. https://codereview.chromium.org/242783003/diff/70001/cc/test/layer_tree_test.cc File cc/test/layer_tree_test.cc (right): https://codereview.chromium.org/242783003/diff/70001/cc/test/layer_tree_test.... cc/test/layer_tree_test.cc:708: DCHECK(proxy() && proxy()->HasImplThread()); Please split DCHECK into two separate DCHECKs. https://codereview.chromium.org/242783003/diff/70001/cc/test/layer_tree_test.... cc/test/layer_tree_test.cc:714: DCHECK(proxy() && proxy()->HasImplThread()); Please split DCHECK into two separate DCHECKs. https://codereview.chromium.org/242783003/diff/70001/cc/trees/layer_tree_host.h File cc/trees/layer_tree_host.h (right): https://codereview.chromium.org/242783003/diff/70001/cc/trees/layer_tree_host... cc/trees/layer_tree_host.h:307: virtual void InitializeThreaded( Instead of overriding this method, could we re-use the LayerTreeHostWithProxy pattern in layer_tree_host_unittest.cc? https://codereview.chromium.org/242783003/diff/70001/cc/trees/layer_tree_host... cc/trees/layer_tree_host.h:315: void InitializeProxy(scoped_ptr<Proxy> proxy); This could stay private. There's an existing LayerTreeHost::InitializeForTesting. https://codereview.chromium.org/242783003/diff/70001/cc/trees/layer_tree_host... File cc/trees/layer_tree_host_unittest_proxy.cc (right): https://codereview.chromium.org/242783003/diff/70001/cc/trees/layer_tree_host... cc/trees/layer_tree_host_unittest_proxy.cc:23: bool impl_side_painting = true; Note: We will care about impl-side-painting since it affects how scheduling works and what proxy logic is used/valid. https://codereview.chromium.org/242783003/diff/70001/cc/trees/thread_proxy.h File cc/trees/thread_proxy.h (left): https://codereview.chromium.org/242783003/diff/70001/cc/trees/thread_proxy.h#... cc/trees/thread_proxy.h:248: const MainThreadOrBlockedMainThread& blocked_main() const; Please expose MainThreadOrBlockedMainThread as the MainThreadOnly and CompositorThreadOnly are. https://codereview.chromium.org/242783003/diff/70001/cc/trees/thread_proxy.h File cc/trees/thread_proxy.h (right): https://codereview.chromium.org/242783003/diff/70001/cc/trees/thread_proxy.h#... cc/trees/thread_proxy.h:226: MainThreadOnly& main(); Does this need to be protected too? It looks like only the const version is used by this patch.
All done. Please take another look. Thanks for review! https://codereview.chromium.org/242783003/diff/70001/cc/test/layer_tree_test.cc File cc/test/layer_tree_test.cc (right): https://codereview.chromium.org/242783003/diff/70001/cc/test/layer_tree_test.... cc/test/layer_tree_test.cc:708: DCHECK(proxy() && proxy()->HasImplThread()); On 2014/05/14 21:12:07, brianderson wrote: > Please split DCHECK into two separate DCHECKs. Done. https://codereview.chromium.org/242783003/diff/70001/cc/test/layer_tree_test.... cc/test/layer_tree_test.cc:714: DCHECK(proxy() && proxy()->HasImplThread()); On 2014/05/14 21:12:07, brianderson wrote: > Please split DCHECK into two separate DCHECKs. Done. https://codereview.chromium.org/242783003/diff/70001/cc/trees/layer_tree_host.h File cc/trees/layer_tree_host.h (right): https://codereview.chromium.org/242783003/diff/70001/cc/trees/layer_tree_host... cc/trees/layer_tree_host.h:307: virtual void InitializeThreaded( On 2014/05/14 21:12:07, brianderson wrote: > Instead of overriding this method, could we re-use the LayerTreeHostWithProxy > pattern in layer_tree_host_unittest.cc? Done. https://codereview.chromium.org/242783003/diff/70001/cc/trees/layer_tree_host... cc/trees/layer_tree_host.h:315: void InitializeProxy(scoped_ptr<Proxy> proxy); On 2014/05/14 21:12:07, brianderson wrote: > This could stay private. There's an existing > LayerTreeHost::InitializeForTesting. Ah, I didn't find it. Thanks. Restored. https://codereview.chromium.org/242783003/diff/70001/cc/trees/layer_tree_host... File cc/trees/layer_tree_host_unittest_proxy.cc (right): https://codereview.chromium.org/242783003/diff/70001/cc/trees/layer_tree_host... cc/trees/layer_tree_host_unittest_proxy.cc:23: bool impl_side_painting = true; On 2014/05/14 21:12:07, brianderson wrote: > Note: We will care about impl-side-painting since it affects how scheduling > works and what proxy logic is used/valid. Yep. changed to it configurable. https://codereview.chromium.org/242783003/diff/70001/cc/trees/thread_proxy.h File cc/trees/thread_proxy.h (left): https://codereview.chromium.org/242783003/diff/70001/cc/trees/thread_proxy.h#... cc/trees/thread_proxy.h:248: const MainThreadOrBlockedMainThread& blocked_main() const; On 2014/05/14 21:12:07, brianderson wrote: > Please expose MainThreadOrBlockedMainThread as the MainThreadOnly and > CompositorThreadOnly are. Done. https://codereview.chromium.org/242783003/diff/70001/cc/trees/thread_proxy.h File cc/trees/thread_proxy.h (right): https://codereview.chromium.org/242783003/diff/70001/cc/trees/thread_proxy.h#... cc/trees/thread_proxy.h:226: MainThreadOnly& main(); On 2014/05/14 21:12:07, brianderson wrote: > Does this need to be protected too? It looks like only the const version is used > by this patch. Done.
https://codereview.chromium.org/242783003/diff/90002/cc/test/layer_tree_test.h File cc/test/layer_tree_test.h (right): https://codereview.chromium.org/242783003/diff/90002/cc/test/layer_tree_test.... cc/test/layer_tree_test.h:183: const ThreadProxy::CompositorThreadOnly& ThreadProxyImplOnly() const; Instead of exposing these to all LayerTreeTests, can you add to the ThreadProxyTest class instead? lgtm if you move this.
Done. Thanks! https://codereview.chromium.org/242783003/diff/90002/cc/test/layer_tree_test.h File cc/test/layer_tree_test.h (right): https://codereview.chromium.org/242783003/diff/90002/cc/test/layer_tree_test.... cc/test/layer_tree_test.h:183: const ThreadProxy::CompositorThreadOnly& ThreadProxyImplOnly() const; On 2014/05/16 01:24:49, brianderson wrote: > Instead of exposing these to all LayerTreeTests, can you add to the > ThreadProxyTest class instead? > > lgtm if you move this. Done.
The CQ bit was checked by simonhong@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/simonhong@chromium.org/242783003/120001
The CQ bit was unchecked by simonhong@chromium.org
The CQ bit was checked by simonhong@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/simonhong@chromium.org/242783003/140001
The CQ bit was unchecked by simonhong@chromium.org
Dear brian, I couldn't submit previous patch set you lgtm'd because test_hooks_ of ThreadProxyForTest is not used. So, I added some more basic test for proxy related with ScheduledClient. Please check again.
lgtm https://codereview.chromium.org/242783003/diff/160001/cc/test/layer_tree_test.h File cc/test/layer_tree_test.h (right): https://codereview.chromium.org/242783003/diff/160001/cc/test/layer_tree_test... cc/test/layer_tree_test.h:81: // Hooks for SchedulerClient. I would say this makes more sense in the ThreadProxyLayerTreeTest today, but since the SingleThreadProxy will also have these soon, we might as well just put them here.
The CQ bit was checked by simonhong@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/simonhong@chromium.org/242783003/200001
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are real, and report flakes to chrome-troopers@google.com. The failing builders are: win_chromium_compile_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_compile_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
The CQ bit was checked by simonhong@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/simonhong@chromium.org/242783003/220001
The CQ bit was unchecked by simonhong@chromium.org
The CQ bit was checked by simonhong@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/simonhong@chromium.org/242783003/230001
Message was sent while issue was closed.
Change committed as 272051 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
