Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(848)

Side by Side Diff: base/observer_list_unittest.cc

Issue 2669673002: Revert of Allow ObserverListThreadSafe to be used from sequenced tasks. (Closed)
Patch Set: Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « base/observer_list_threadsafe.h ('k') | components/metrics/net/network_metrics_provider.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "base/observer_list.h" 5 #include "base/observer_list.h"
6 #include "base/observer_list_threadsafe.h" 6 #include "base/observer_list_threadsafe.h"
7 7
8 #include <utility>
9 #include <vector> 8 #include <vector>
10 9
11 #include "base/bind.h"
12 #include "base/compiler_specific.h" 10 #include "base/compiler_specific.h"
13 #include "base/location.h" 11 #include "base/location.h"
14 #include "base/memory/weak_ptr.h" 12 #include "base/memory/weak_ptr.h"
15 #include "base/run_loop.h" 13 #include "base/run_loop.h"
16 #include "base/sequenced_task_runner.h"
17 #include "base/single_thread_task_runner.h" 14 #include "base/single_thread_task_runner.h"
18 #include "base/task_scheduler/post_task.h"
19 #include "base/test/scoped_task_scheduler.h"
20 #include "base/threading/platform_thread.h" 15 #include "base/threading/platform_thread.h"
21 #include "testing/gtest/include/gtest/gtest.h" 16 #include "testing/gtest/include/gtest/gtest.h"
22 17
23 namespace base { 18 namespace base {
24 namespace { 19 namespace {
25 20
26 class Foo { 21 class Foo {
27 public: 22 public:
28 virtual void Observe(int x) = 0; 23 virtual void Observe(int x) = 0;
29 virtual ~Foo() {} 24 virtual ~Foo() {}
(...skipping 33 matching lines...) Expand 10 before | Expand all | Expand 10 after
63 } 58 }
64 59
65 void SetDoomed(Foo* doomed) { doomed_ = doomed; } 60 void SetDoomed(Foo* doomed) { doomed_ = doomed; }
66 61
67 private: 62 private:
68 ObserverList<Foo>* list_; 63 ObserverList<Foo>* list_;
69 Foo* doomed_; 64 Foo* doomed_;
70 bool remove_self_; 65 bool remove_self_;
71 }; 66 };
72 67
68 class ThreadSafeDisrupter : public Foo {
69 public:
70 ThreadSafeDisrupter(ObserverListThreadSafe<Foo>* list, Foo* doomed)
71 : list_(list),
72 doomed_(doomed) {
73 }
74 ~ThreadSafeDisrupter() override {}
75 void Observe(int x) override { list_->RemoveObserver(doomed_); }
76
77 private:
78 ObserverListThreadSafe<Foo>* list_;
79 Foo* doomed_;
80 };
81
73 template <typename ObserverListType> 82 template <typename ObserverListType>
74 class AddInObserve : public Foo { 83 class AddInObserve : public Foo {
75 public: 84 public:
76 explicit AddInObserve(ObserverListType* observer_list) 85 explicit AddInObserve(ObserverListType* observer_list)
77 : observer_list(observer_list), to_add_() {} 86 : observer_list(observer_list), to_add_() {}
78 87
79 void SetToAdd(Foo* to_add) { to_add_ = to_add; } 88 void SetToAdd(Foo* to_add) { to_add_ = to_add; }
80 89
81 void Observe(int x) override { 90 void Observe(int x) override {
82 if (to_add_) { 91 if (to_add_) {
(...skipping 177 matching lines...) Expand 10 before | Expand all | Expand 10 after
260 269
261 TEST(ObserverListThreadSafeTest, BasicTest) { 270 TEST(ObserverListThreadSafeTest, BasicTest) {
262 MessageLoop loop; 271 MessageLoop loop;
263 272
264 scoped_refptr<ObserverListThreadSafe<Foo> > observer_list( 273 scoped_refptr<ObserverListThreadSafe<Foo> > observer_list(
265 new ObserverListThreadSafe<Foo>); 274 new ObserverListThreadSafe<Foo>);
266 Adder a(1); 275 Adder a(1);
267 Adder b(-1); 276 Adder b(-1);
268 Adder c(1); 277 Adder c(1);
269 Adder d(-1); 278 Adder d(-1);
279 ThreadSafeDisrupter evil(observer_list.get(), &c);
270 280
271 observer_list->AddObserver(&a); 281 observer_list->AddObserver(&a);
272 observer_list->AddObserver(&b); 282 observer_list->AddObserver(&b);
273 283
274 observer_list->Notify(FROM_HERE, &Foo::Observe, 10); 284 observer_list->Notify(FROM_HERE, &Foo::Observe, 10);
275 RunLoop().RunUntilIdle(); 285 RunLoop().RunUntilIdle();
276 286
287 observer_list->AddObserver(&evil);
277 observer_list->AddObserver(&c); 288 observer_list->AddObserver(&c);
278 observer_list->AddObserver(&d); 289 observer_list->AddObserver(&d);
279 290
280 observer_list->Notify(FROM_HERE, &Foo::Observe, 10); 291 observer_list->Notify(FROM_HERE, &Foo::Observe, 10);
281 observer_list->RemoveObserver(&c);
282 RunLoop().RunUntilIdle(); 292 RunLoop().RunUntilIdle();
283 293
284 EXPECT_EQ(20, a.total); 294 EXPECT_EQ(20, a.total);
285 EXPECT_EQ(-20, b.total); 295 EXPECT_EQ(-20, b.total);
286 EXPECT_EQ(0, c.total); 296 EXPECT_EQ(0, c.total);
287 EXPECT_EQ(-10, d.total); 297 EXPECT_EQ(-10, d.total);
288 } 298 }
289 299
290 TEST(ObserverListThreadSafeTest, RemoveObserver) { 300 TEST(ObserverListThreadSafeTest, RemoveObserver) {
291 MessageLoop loop; 301 MessageLoop loop;
(...skipping 20 matching lines...) Expand all
312 // Should also do nothing. 322 // Should also do nothing.
313 observer_list->RemoveObserver(&b); 323 observer_list->RemoveObserver(&b);
314 324
315 observer_list->Notify(FROM_HERE, &Foo::Observe, 10); 325 observer_list->Notify(FROM_HERE, &Foo::Observe, 10);
316 RunLoop().RunUntilIdle(); 326 RunLoop().RunUntilIdle();
317 327
318 EXPECT_EQ(10, a.total); 328 EXPECT_EQ(10, a.total);
319 EXPECT_EQ(0, b.total); 329 EXPECT_EQ(0, b.total);
320 } 330 }
321 331
322 TEST(ObserverListThreadSafeTest, WithoutSequence) { 332 TEST(ObserverListThreadSafeTest, WithoutMessageLoop) {
323 scoped_refptr<ObserverListThreadSafe<Foo> > observer_list( 333 scoped_refptr<ObserverListThreadSafe<Foo> > observer_list(
324 new ObserverListThreadSafe<Foo>); 334 new ObserverListThreadSafe<Foo>);
325 335
326 Adder a(1), b(1), c(1); 336 Adder a(1), b(1), c(1);
327 337
328 // No sequence, so these should not be added. 338 // No MessageLoop, so these should not be added.
329 observer_list->AddObserver(&a); 339 observer_list->AddObserver(&a);
330 observer_list->AddObserver(&b); 340 observer_list->AddObserver(&b);
331 341
332 { 342 {
333 // Add c when there's a sequence. 343 // Add c when there's a loop.
334 MessageLoop loop; 344 MessageLoop loop;
335 observer_list->AddObserver(&c); 345 observer_list->AddObserver(&c);
336 346
337 observer_list->Notify(FROM_HERE, &Foo::Observe, 10); 347 observer_list->Notify(FROM_HERE, &Foo::Observe, 10);
338 RunLoop().RunUntilIdle(); 348 RunLoop().RunUntilIdle();
339 349
340 EXPECT_EQ(0, a.total); 350 EXPECT_EQ(0, a.total);
341 EXPECT_EQ(0, b.total); 351 EXPECT_EQ(0, b.total);
342 EXPECT_EQ(10, c.total); 352 EXPECT_EQ(10, c.total);
343 353
344 // Now add a when there's a sequence. 354 // Now add a when there's a loop.
345 observer_list->AddObserver(&a); 355 observer_list->AddObserver(&a);
346 356
347 // Remove c when there's a sequence. 357 // Remove c when there's a loop.
348 observer_list->RemoveObserver(&c); 358 observer_list->RemoveObserver(&c);
349 359
350 // Notify again. 360 // Notify again.
351 observer_list->Notify(FROM_HERE, &Foo::Observe, 20); 361 observer_list->Notify(FROM_HERE, &Foo::Observe, 20);
352 RunLoop().RunUntilIdle(); 362 RunLoop().RunUntilIdle();
353 363
354 EXPECT_EQ(20, a.total); 364 EXPECT_EQ(20, a.total);
355 EXPECT_EQ(0, b.total); 365 EXPECT_EQ(0, b.total);
356 EXPECT_EQ(10, c.total); 366 EXPECT_EQ(10, c.total);
357 } 367 }
358 368
359 // Removing should always succeed with or without a sequence. 369 // Removing should always succeed with or without a loop.
360 observer_list->RemoveObserver(&a); 370 observer_list->RemoveObserver(&a);
361 371
362 // Notifying should not fail but should also be a no-op. 372 // Notifying should not fail but should also be a no-op.
363 MessageLoop loop; 373 MessageLoop loop;
364 observer_list->AddObserver(&b); 374 observer_list->AddObserver(&b);
365 observer_list->Notify(FROM_HERE, &Foo::Observe, 30); 375 observer_list->Notify(FROM_HERE, &Foo::Observe, 30);
366 RunLoop().RunUntilIdle(); 376 RunLoop().RunUntilIdle();
367 377
368 EXPECT_EQ(20, a.total); 378 EXPECT_EQ(20, a.total);
369 EXPECT_EQ(30, b.total); 379 EXPECT_EQ(30, b.total);
(...skipping 104 matching lines...) Expand 10 before | Expand all | Expand 10 after
474 scoped_refptr<ObserverListThreadSafe<Foo> > observer_list( 484 scoped_refptr<ObserverListThreadSafe<Foo> > observer_list(
475 new ObserverListThreadSafe<Foo>); 485 new ObserverListThreadSafe<Foo>);
476 486
477 Adder a(1); 487 Adder a(1);
478 observer_list->AddObserver(&a); 488 observer_list->AddObserver(&a);
479 delete loop; 489 delete loop;
480 // Test passes if we don't crash here. 490 // Test passes if we don't crash here.
481 observer_list->Notify(FROM_HERE, &Foo::Observe, 1); 491 observer_list->Notify(FROM_HERE, &Foo::Observe, 1);
482 } 492 }
483 493
484 namespace {
485
486 class SequenceVerificationObserver : public Foo {
487 public:
488 explicit SequenceVerificationObserver(
489 scoped_refptr<SequencedTaskRunner> task_runner)
490 : task_runner_(std::move(task_runner)) {}
491 ~SequenceVerificationObserver() override = default;
492
493 void Observe(int x) override {
494 called_on_valid_sequence_ = task_runner_->RunsTasksOnCurrentThread();
495 }
496
497 bool called_on_valid_sequence() const { return called_on_valid_sequence_; }
498
499 private:
500 const scoped_refptr<SequencedTaskRunner> task_runner_;
501 bool called_on_valid_sequence_ = false;
502
503 DISALLOW_COPY_AND_ASSIGN(SequenceVerificationObserver);
504 };
505
506 } // namespace
507
508 // Verify that observers are notified on the correct sequence.
509 TEST(ObserverListThreadSafeTest, NotificationOnValidSequence) {
510 test::ScopedTaskScheduler scoped_task_scheduler;
511
512 auto task_runner_1 = CreateSequencedTaskRunnerWithTraits(TaskTraits());
513 auto task_runner_2 = CreateSequencedTaskRunnerWithTraits(TaskTraits());
514
515 auto observer_list = make_scoped_refptr(new ObserverListThreadSafe<Foo>());
516
517 SequenceVerificationObserver observer_1(task_runner_1);
518 SequenceVerificationObserver observer_2(task_runner_2);
519
520 task_runner_1->PostTask(
521 FROM_HERE, Bind(&ObserverListThreadSafe<Foo>::AddObserver, observer_list,
522 Unretained(&observer_1)));
523 task_runner_2->PostTask(
524 FROM_HERE, Bind(&ObserverListThreadSafe<Foo>::AddObserver, observer_list,
525 Unretained(&observer_2)));
526
527 RunLoop().RunUntilIdle();
528
529 observer_list->Notify(FROM_HERE, &Foo::Observe, 1);
530
531 RunLoop().RunUntilIdle();
532
533 EXPECT_TRUE(observer_1.called_on_valid_sequence());
534 EXPECT_TRUE(observer_2.called_on_valid_sequence());
535 }
536
537 // Verify that when an observer is added to a NOTIFY_ALL ObserverListThreadSafe
538 // from a notification, it is itself notified.
539 TEST(ObserverListThreadSafeTest, AddObserverFromNotificationNotifyAll) {
540 MessageLoop message_loop;
541 auto observer_list = make_scoped_refptr(new ObserverListThreadSafe<Foo>());
542
543 Adder observer_added_from_notification(1);
544
545 AddInObserve<ObserverListThreadSafe<Foo>> initial_observer(
546 observer_list.get());
547 initial_observer.SetToAdd(&observer_added_from_notification);
548 observer_list->AddObserver(&initial_observer);
549
550 observer_list->Notify(FROM_HERE, &Foo::Observe, 1);
551
552 RunLoop().RunUntilIdle();
553
554 EXPECT_EQ(1, observer_added_from_notification.GetValue());
555 }
556
557 TEST(ObserverListTest, Existing) { 494 TEST(ObserverListTest, Existing) {
558 ObserverList<Foo> observer_list(ObserverList<Foo>::NOTIFY_EXISTING_ONLY); 495 ObserverList<Foo> observer_list(ObserverList<Foo>::NOTIFY_EXISTING_ONLY);
559 Adder a(1); 496 Adder a(1);
560 AddInObserve<ObserverList<Foo> > b(&observer_list); 497 AddInObserve<ObserverList<Foo> > b(&observer_list);
561 Adder c(1); 498 Adder c(1);
562 b.SetToAdd(&c); 499 b.SetToAdd(&c);
563 500
564 observer_list.AddObserver(&a); 501 observer_list.AddObserver(&a);
565 observer_list.AddObserver(&b); 502 observer_list.AddObserver(&b);
566 503
(...skipping 422 matching lines...) Expand 10 before | Expand all | Expand 10 after
989 // However, the first Observe() call will add a second observer: at this 926 // However, the first Observe() call will add a second observer: at this
990 // point, it != observer_list.end() should be true, and Observe() should be 927 // point, it != observer_list.end() should be true, and Observe() should be
991 // called on the newly added observer on the next iteration of the loop. 928 // called on the newly added observer on the next iteration of the loop.
992 observer.Observe(10); 929 observer.Observe(10);
993 } 930 }
994 931
995 EXPECT_EQ(-10, b.total); 932 EXPECT_EQ(-10, b.total);
996 } 933 }
997 934
998 } // namespace base 935 } // namespace base
OLDNEW
« no previous file with comments | « base/observer_list_threadsafe.h ('k') | components/metrics/net/network_metrics_provider.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698