Issue 2693 - Update adblockplus dependency

test/Notification.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 30 matching lines @@
     }
 
     void AddNotification(const std::string& notification)
     {
       jsEngine->Evaluate("(function()"
       "{"
         "require('notification').Notification.addNotification(" + notification + ");"
       "})();");
     }
 
-    struct PeekNotificationResult
-    {
-      bool isCallbackCalled;

[Felix Dahlke, 2015/06/25 18:35:49]

Can't we derive this by checking if the notification pointer is pointing
somewhere? Then we could return a NotificationPtr instead of
PeekNotificationResult.

[sergei, 2015/06/26 14:21:17]

Yes we can. I just wanted to be sure that it's really not called rather than we
cannot construct `AdblockPlus::Notification` object.

Fixed to use `NotificationPtr`.

-      NotificationPtr notification;
-    };
-    PeekNotificationResult PeekNotification(const std::string& url = std::string())

[Felix Dahlke, 2015/06/25 18:35:49]

Nit: "peek" sounds a bit weird to me here, why not just "show"?

We've got a very similar function in the JS tests BTW:

  function showNotifications(url)
  {
    let shownNotifications = [];
    function showListener(notification)
    {
      shownNotifications.push(notification);
      Notification.markAsShown(notification.id);
    }
    Notification.addShowListener(showListener);
    Notification.showNext(url);
    Notification.removeShowListener(showListener);
    return shownNotifications;
  }

We're working with an array of notifications instead of a single one there, to
ensure the show handler is not being called multiple times. But we don't have to
do that here as well IMO.

[sergei, 2015/06/26 14:21:17]

Because I don't mark the notification as shown, so we can say that it stays in
some internal queue, there is even a test for that case
`TEST_F(NotificationTest, MarkAsShown)`.
 
I also thought about multiple times calls but decided to check merely that it's
called with the correct notification object.

[Felix Dahlke, 2015/06/30 19:45:31]

Yeah right, makes sense then.
Yeah, surely sufficient, we don't need to repeat the core tests here.

-    {
-      PeekNotificationResult retValue = {false};
-      filterEngine->SetNotificationAvailableCallback([&retValue](const NotificationPtr& notification)

[Felix Dahlke, 2015/06/25 18:35:49]

Still C++03 here :)

[sergei, 2015/06/26 14:21:17]

Done.

-      {
-        retValue.isCallbackCalled = true;
-        retValue.notification = notification;
-      });
+    NotificationPtr PeekNotification(const std::string& url = std::string())
+    {
+      NotificationPtr retValue;
+      filterEngine->SetShowNotificationCallback(std::tr1::bind(
+        &NotificationTest::NotificationAvailableCallback,
+        std::tr1::placeholders::_1, std::tr1::ref(retValue)));
       filterEngine->ShowNextNotification(url);
-      filterEngine->RemoveNotificationAvailableCallback();
+      filterEngine->RemoveShowNotificationCallback();
       return retValue;
+    }
+
+    static void NotificationAvailableCallback(const NotificationPtr& src, NotificationPtr& dst)
+    {
+      EXPECT_TRUE(src);
+      dst = src;
     }
   };
 
   class MockWebRequest : public WebRequest
   {
   public:
     std::string responseText;
     explicit MockWebRequest(const std::string& notification)
       : responseText(notification)
     {
@@ skipping 11 matching lines @@
       serverResponse.responseText = responseText;
       return serverResponse;
     }
   };
 
 #ifdef NotificationMockWebRequestTest_ENABLED
   class NotificationMockWebRequestTest : public BaseJsTest
   {
   protected:
     FilterEnginePtr filterEngine;
+    bool isNotificationCallbackCalled;
     void SetUp()
     {
       BaseJsTest::SetUp();
+      isNotificationCallbackCalled = false;
       jsEngine->SetFileSystem(
         std::tr1::shared_ptr<LazyFileSystem>(new LazyFileSystem()));
       const char* responseJsonText = "{"
         "\"notifications\": [{"
           "\"id\": \"some id\","
           "\"type\": \"information\","
           "\"message\": {"
              "\"en-US\": \"message\""
           "},"
           "\"title\": \"Title\""
         "}]"
         "}";
       jsEngine->SetWebRequest(std::tr1::shared_ptr<MockWebRequest>(
         new MockWebRequest(responseJsonText)));
       jsEngine->SetLogSystem(LogSystemPtr(new DefaultLogSystem()));
       filterEngine.reset(new FilterEngine(jsEngine));
+      filterEngine->SetShowNotificationCallback(
+        std::bind(&NotificationMockWebRequestTest::OnNotification,
+        this, std::tr1::placeholders::_1));
+    }
+
+    void OnNotification(const NotificationPtr& notification)
+    {
+      isNotificationCallbackCalled = true;
+      ASSERT_TRUE(notification);
+      EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_INFORMATION, notification->GetType());
+      EXPECT_EQ("Title", notification->GetTitle());
+      EXPECT_EQ("message", notification->GetMessageString());
+      notification->MarkAsShown();
     }
   };
 #endif
 }
 
 TEST_F(NotificationTest, NoNotifications)
 {
-  EXPECT_FALSE(PeekNotification().isCallbackCalled);
+  EXPECT_FALSE(PeekNotification());
 }
 
 #ifdef NotificationMockWebRequestTest_ENABLED
 TEST_F(NotificationMockWebRequestTest, SingleNotification)
 {
-  bool isNotificationCallbackCalled = false;
-  filterEngine->SetNotificationAvailableCallback([&isNotificationCallbackCalled](const NotificationPtr& notification)
-  {
-    isNotificationCallbackCalled = true;
-    ASSERT_TRUE(notification);
-    EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_INFORMATION, notification->GetType());
-    EXPECT_EQ("Title", notification->GetTitle());
-    EXPECT_EQ("message", notification->GetMessageString());
-    notification->MarkAsShown();
-  });
   AdblockPlus::Sleep(5000/*msec*/); // it's a hack
   EXPECT_TRUE(isNotificationCallbackCalled);
 }
 #endif
 
 TEST_F(NotificationTest, AddNotification)
 {
   AddNotification("{"
       "type: 'critical',"
       "title: 'testTitle',"
       "message: 'testMessage',"
     "}");
-  PeekNotificationResult peekNotificationResult = PeekNotification();
-  ASSERT_TRUE(peekNotificationResult.notification);
-  EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_CRITICAL, peekNotificationResult.notification->GetType());
-  EXPECT_EQ("testTitle", peekNotificationResult.notification->GetTitle());
-  EXPECT_EQ("testMessage", peekNotificationResult.notification->GetMessageString());
+  NotificationPtr notification = PeekNotification();
+  ASSERT_TRUE(notification);
+  EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_CRITICAL, notification->GetType());
+  EXPECT_EQ("testTitle", notification->GetTitle());
+  EXPECT_EQ("testMessage", notification->GetMessageString());
 }
 
 TEST_F(NotificationTest, FilterByUrl)
 {
   AddNotification("{ id: 'no-filter', type: 'critical' }");
   AddNotification("{ id: 'www.com', type: 'information',"
     "urlFilters:['||www.com$document']"
   "}");
   AddNotification("{ id: 'www.de', type: 'question',"
     "urlFilters:['||www.de$document']"
   "}");
 
-  PeekNotificationResult peekNotificationResult = PeekNotification();
-  ASSERT_TRUE(peekNotificationResult.notification);
-  EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_CRITICAL, peekNotificationResult.notification->GetType());
-
-  peekNotificationResult = PeekNotification("http://www.de");
-  ASSERT_TRUE(peekNotificationResult.notification);
-  EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_QUESTION, peekNotificationResult.notification->GetType());
-
-  peekNotificationResult = PeekNotification("http://www.com");
-  ASSERT_TRUE(peekNotificationResult.notification);
-  EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_INFORMATION, peekNotificationResult.notification->GetType());
+  NotificationPtr notification = PeekNotification();
+  ASSERT_TRUE(notification);
+  EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_CRITICAL, notification->GetType());
+
+  notification = PeekNotification("http://www.de");
+  ASSERT_TRUE(notification);
+  EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_QUESTION, notification->GetType());
+
+  notification = PeekNotification("http://www.com");
+  ASSERT_TRUE(notification);
+  EXPECT_EQ(NotificationType::NOTIFICATION_TYPE_INFORMATION, notification->GetType());
 }
 
 TEST_F(NotificationTest, MarkAsShown)
 {
   AddNotification("{ id: 'id', type: 'question' }");
-  EXPECT_TRUE(PeekNotification().notification);
-  PeekNotificationResult peekNotificationResult = PeekNotification();
-  ASSERT_TRUE(peekNotificationResult.notification);
-  peekNotificationResult.notification->MarkAsShown();
-  EXPECT_FALSE(PeekNotification().isCallbackCalled);
+  EXPECT_TRUE(PeekNotification());
+  NotificationPtr notification = PeekNotification();
+  ASSERT_TRUE(notification);
+  notification->MarkAsShown();
+  EXPECT_FALSE(PeekNotification());
 }
 
 TEST_F(NotificationTest, NoLinks)
 {
   AddNotification("{ id: 'id'}");
-  PeekNotificationResult peekNotificationResult = PeekNotification();
-  ASSERT_TRUE(peekNotificationResult.notification);
-  EXPECT_EQ(0, peekNotificationResult.notification->GetLinks().size());
+  NotificationPtr notification = PeekNotification();
+  ASSERT_TRUE(notification);
+  EXPECT_EQ(0, notification->GetLinks().size());
 }
 
 TEST_F(NotificationTest, Links)
 {
   AddNotification("{ id: 'id', links: ['link1', 'link2'] }");
-  PeekNotificationResult peekNotificationResult = PeekNotification();
-  ASSERT_TRUE(peekNotificationResult.notification);
-  std::vector<std::string> notificationLinks = peekNotificationResult.notification->GetLinks();
+  NotificationPtr notification = PeekNotification();
+  ASSERT_TRUE(notification);
+  std::vector<std::string> notificationLinks = notification->GetLinks();
   ASSERT_EQ(2, notificationLinks.size());
   EXPECT_EQ("link1", notificationLinks[0]);
   EXPECT_EQ("link2", notificationLinks[1]);
-}
+}