Fix semantics of ExtensionWebRequestEventRouter::EventListeners.

chrome/browser/extensions/api/web_request/web_request_api_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <map>
 #include <memory>
 #include <queue>
@@ skipping 73 matching lines @@
 using helpers::MergeOnBeforeRequestResponses;
 using helpers::RequestCookieModification;
 using helpers::ResponseCookieModification;
 using helpers::ResponseHeader;
 using helpers::ResponseHeaders;
 using helpers::StringToCharList;
 
 namespace extensions {
 
 namespace {
+
 static void EventHandledOnIOThread(
     void* profile,
     const std::string& extension_id,
     const std::string& event_name,
     const std::string& sub_event_name,
     uint64_t request_id,
     ExtensionWebRequestEventRouter::EventResponse* response) {
   ExtensionWebRequestEventRouter::GetInstance()->OnEventHandled(
       profile, extension_id, event_name, sub_event_name, request_id,
       response);
@@ skipping 244 matching lines @@
     base::RunLoop().Run();
 
     EXPECT_TRUE(!request2->is_pending());
     EXPECT_EQ(net::URLRequestStatus::SUCCESS, request2->status().status());
     EXPECT_EQ(0, request2->status().error());
     EXPECT_EQ(redirect_url, request2->url());
     EXPECT_EQ(2U, request2->url_chain().size());
     EXPECT_EQ(0U, ipc_sender_.GetNumTasks());
   }
 
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+  ExtensionWebRequestEventRouter::EventListener::Identifier id1(
       &profile_, extension1_id, kEventName + "/1", 0, 0);
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+  ExtensionWebRequestEventRouter::EventListener::Identifier id2(
       &profile_, extension2_id, kEventName + "/2", 0, 0);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id1);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id2);
 }
 
 // Test that a request is canceled if this is requested by any extension
 // regardless whether it is the extension with the highest precedence.
 TEST_F(ExtensionWebRequestTest, BlockingEventPrecedenceCancel) {
   std::string extension1_id("1");
   std::string extension2_id("2");
   ExtensionWebRequestEventRouter::RequestFilter filter;
   const std::string kEventName(web_request::OnBeforeRequest::kEventName);
   base::WeakPtrFactory<TestIPCSender> ipc_sender_factory(&ipc_sender_);
@@ skipping 40 matching lines @@
 
   base::RunLoop().Run();
 
   EXPECT_TRUE(!request->is_pending());
   EXPECT_EQ(net::URLRequestStatus::FAILED, request->status().status());
   EXPECT_EQ(net::ERR_BLOCKED_BY_CLIENT, request->status().error());
   EXPECT_EQ(request_url, request->url());
   EXPECT_EQ(1U, request->url_chain().size());
   EXPECT_EQ(0U, ipc_sender_.GetNumTasks());
 
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+  ExtensionWebRequestEventRouter::EventListener::Identifier id1(
       &profile_, extension1_id, kEventName + "/1", 0, 0);
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+  ExtensionWebRequestEventRouter::EventListener::Identifier id2(
       &profile_, extension2_id, kEventName + "/2", 0, 0);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id1);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id2);
 }
 
 TEST_F(ExtensionWebRequestTest, SimulateChancelWhileBlocked) {
   // We subscribe to OnBeforeRequest and OnErrorOccurred.
   // While the OnBeforeRequest handler is blocked, we cancel the request.
   // We verify that the response of the blocked OnBeforeRequest handler
   // is ignored.
 
   std::string extension_id("1");
   ExtensionWebRequestEventRouter::RequestFilter filter;
@@ skipping 40 matching lines @@
   request->Cancel();
   run_loop.Run();
 
   EXPECT_TRUE(!request->is_pending());
   EXPECT_EQ(net::URLRequestStatus::CANCELED, request->status().status());
   EXPECT_EQ(net::ERR_ABORTED, request->status().error());
   EXPECT_EQ(request_url, request->url());
   EXPECT_EQ(1U, request->url_chain().size());
   EXPECT_EQ(0U, ipc_sender_.GetNumTasks());
 
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+  ExtensionWebRequestEventRouter::EventListener::Identifier id1(
       &profile_, extension_id, kEventName + "/1", 0, 0);
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+  ExtensionWebRequestEventRouter::EventListener::Identifier id2(
       &profile_, extension_id, kEventName2 + "/1", 0, 0);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id1);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id2);
 }
 
 namespace {
 
 // Create the numerical representation of |values|, strings passed as
 // extraInfoSpec by the event handler. Returns true on success, otherwise false.
 bool GenerateInfoSpec(const std::string& values, int* result) {
   // Create a base::ListValue of strings.
   base::ListValue list_value;
   for (const std::string& cur :
@@ skipping 136 matching lines @@
   ExtensionWebRequestEventRouter::GetInstance()->AddEventListener(
       &profile_, extension_id, extension_id, events::FOR_TEST, kEventName,
       kEventName + "/1", filter, extra_info_spec_body, 0, 0,
       ipc_sender_factory.GetWeakPtr());
 
   FireURLRequestWithData(kMethodPost, kMultipart, form_1, form_2);
 
   // We inspect the result in the message list of |ipc_sender_| later.
   base::RunLoop().RunUntilIdle();
 
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+  ExtensionWebRequestEventRouter::EventListener::Identifier id1(
       &profile_, extension_id, kEventName + "/1", 0, 0);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id1);
 
   // Part 2.
   // Now subscribe to OnBeforeRequest *without* the requestBody requirement.
   ASSERT_TRUE(
       GenerateInfoSpec(string_spec_no_post, &extra_info_spec_empty));
   ExtensionWebRequestEventRouter::GetInstance()->AddEventListener(
       &profile_, extension_id, extension_id, events::FOR_TEST, kEventName,
       kEventName + "/1", filter, extra_info_spec_empty, 0, 0,
       ipc_sender_factory.GetWeakPtr());
 
   FireURLRequestWithData(kMethodPost, kMultipart, form_1, form_2);
 
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
-      &profile_, extension_id, kEventName + "/1", 0, 0);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id1);
 
   // Subscribe to OnBeforeRequest with requestBody requirement.
   ExtensionWebRequestEventRouter::GetInstance()->AddEventListener(
       &profile_, extension_id, extension_id, events::FOR_TEST, kEventName,
       kEventName + "/1", filter, extra_info_spec_body, 0, 0,
       ipc_sender_factory.GetWeakPtr());
 
   // Part 3.
   // Now send a POST request with body which is not parseable as a form.
   FireURLRequestWithData(kMethodPost, NULL /*no header*/, plain_1, plain_2);
 
   // Part 4.
   // Now send a PUT request with the same body as above.
   FireURLRequestWithData(kMethodPut, NULL /*no header*/, plain_1, plain_2);
 
   base::RunLoop().RunUntilIdle();
 
   // Clean-up.
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
-      &profile_, extension_id, kEventName + "/1", 0, 0);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id1);
 
   IPC::Message* message = NULL;
   TestIPCSender::SentMessages::const_iterator i = ipc_sender_.sent_begin();
   for (size_t test = 0; test < arraysize(kExpected); ++test) {
     SCOPED_TRACE(testing::Message("iteration number ") << test);
     EXPECT_NE(i, ipc_sender_.sent_end());
     message = (i++)->get();
     const base::DictionaryValue* details;
     ExtensionMsg_MessageInvoke::Param param;
     GetPartOfMessageArguments(message, &details, &param);
@@ skipping 55 matching lines @@
   // Only one request is sent, but more than one event will be triggered.
   for (size_t i = 1; i < arraysize(kExpected); ++i)
     ipc_sender_.PushTask(base::Bind(&base::DoNothing));
 
   const std::vector<char> part_of_body(1);
   FireURLRequestWithData("POST", nullptr, part_of_body, part_of_body);
 
   base::RunLoop().RunUntilIdle();
 
   // Clean-up
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+  ExtensionWebRequestEventRouter::EventListener::Identifier id1(
       &profile_, extension_id1, kEventName + "/1", 0, 0);
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+  ExtensionWebRequestEventRouter::EventListener::Identifier id2(
+      &profile_, extension_id1, kEventName + "/2", 0, 0);
+  ExtensionWebRequestEventRouter::EventListener::Identifier id3(
+      &profile_, extension_id2, kEventName + "/1", 0, 0);
+  ExtensionWebRequestEventRouter::EventListener::Identifier id4(
       &profile_, extension_id2, kEventName + "/2", 0, 0);
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
-      &profile_, extension_id1, kEventName + "/1", 0, 0);
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
-      &profile_, extension_id2, kEventName + "/2", 0, 0);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id1);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id2);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id3);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id4);
 
   TestIPCSender::SentMessages::const_iterator i = ipc_sender_.sent_begin();
 
   for (size_t test = 0; test < arraysize(kExpected); ++test, ++i) {
     SCOPED_TRACE(testing::Message("iteration number ") << test);
     EXPECT_NE(i, ipc_sender_.sent_end());
     IPC::Message* message = i->get();
     const base::DictionaryValue* details = nullptr;
     ExtensionMsg_MessageInvoke::Param param;
     GetPartOfMessageArguments(message, &details, &param);
@@ skipping 34 matching lines @@
     std::unique_ptr<net::URLRequest> request(context_->CreateRequest(
         request_url, net::DEFAULT_PRIORITY, &delegate_));
     request->set_method(kMethods[i]);
     ipc_sender_.PushTask(base::Bind(&base::DoNothing));
     request->Start();
   }
 
   // We inspect the result in the message list of |ipc_sender_| later.
   base::RunLoop().RunUntilIdle();
 
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+  ExtensionWebRequestEventRouter::EventListener::Identifier id1(
       &profile_, extension_id, kEventName + "/1", 0, 0);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id1);
 
   TestIPCSender::SentMessages::const_iterator i = ipc_sender_.sent_begin();
   for (size_t test = 0; test < arraysize(kMethods); ++test, ++i) {
     SCOPED_TRACE(testing::Message("iteration number ") << test);
     EXPECT_NE(i, ipc_sender_.sent_end());
     IPC::Message* message = i->get();
     const base::DictionaryValue* details = NULL;
     ExtensionMsg_MessageInvoke::Param param;
     GetPartOfMessageArguments(message, &details, &param);
     ASSERT_TRUE(details != NULL);
     EXPECT_FALSE(details->HasKey(keys::kRequestBodyKey));
   }
 
   EXPECT_EQ(i, ipc_sender_.sent_end());
 }
 
+// Tests that |embedder_process_id| is not relevant for adding and removing
+// listeners with |web_view_instance_id| = 0.
+TEST_F(ExtensionWebRequestTest, AddAndRemoveListeners) {
+  std::string ext_id("abcdefghijklmnopabcdefghijklmnop");
+  ExtensionWebRequestEventRouter::RequestFilter filter;
+  const std::string kEventName(web_request::OnBeforeRequest::kEventName);
+  const std::string kSubEventName = kEventName + "/1";
+  base::WeakPtrFactory<TestIPCSender> ipc_sender_factory(&ipc_sender_);
+  EXPECT_EQ(
+      0u,
+      ExtensionWebRequestEventRouter::GetInstance()->GetListenerCountForTesting(
+          &profile_, kEventName));
+
+  // Add two non-webview listeners.
+  ExtensionWebRequestEventRouter::GetInstance()->AddEventListener(
+      &profile_, ext_id, ext_id, events::FOR_TEST, kEventName, kSubEventName,
+      filter, 0, 1 /* embedder_process_id */, 0,
+      ipc_sender_factory.GetWeakPtr());
+  ExtensionWebRequestEventRouter::GetInstance()->AddEventListener(
+      &profile_, ext_id, ext_id, events::FOR_TEST, kEventName, kSubEventName,
+      filter, 0, 2 /* embedder_process_id */, 0,
+      ipc_sender_factory.GetWeakPtr());
+  EXPECT_EQ(
+      2u,
+      ExtensionWebRequestEventRouter::GetInstance()->GetListenerCountForTesting(
+          &profile_, kEventName));
+
+  // Now remove the events without passing an explicit process ID.
+  ExtensionWebRequestEventRouter::EventListener::Identifier id1(
+      &profile_, ext_id, kSubEventName, 0, 0);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id1);
+  EXPECT_EQ(
+      1u,
+      ExtensionWebRequestEventRouter::GetInstance()->GetListenerCountForTesting(
+          &profile_, kEventName));
+
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id1);
+  EXPECT_EQ(
+      0u,
+      ExtensionWebRequestEventRouter::GetInstance()->GetListenerCountForTesting(
+          &profile_, kEventName));
+}
+
+// The set of blocked requests should not grow unbounded.
+TEST_F(ExtensionWebRequestTest, BlockedRequestsAreRemoved) {
+  std::string extension_id("1");
+  ExtensionWebRequestEventRouter::RequestFilter filter;
+
+  // Subscribe to OnBeforeRequest.
+  const std::string kEventName(web_request::OnBeforeRequest::kEventName);
+  base::WeakPtrFactory<TestIPCSender> ipc_sender_factory(&ipc_sender_);
+  ExtensionWebRequestEventRouter::GetInstance()->AddEventListener(
+      &profile_, extension_id, extension_id, events::FOR_TEST, kEventName,
+      kEventName + "/1", filter, ExtraInfoSpec::BLOCKING, 0, 0,
+      ipc_sender_factory.GetWeakPtr());
+  ExtensionWebRequestEventRouter::EventListener::Identifier id(
+      &profile_, extension_id, kEventName + "/1", 0, 0);
+  ExtensionWebRequestEventRouter::EventListener* listener =
+      ExtensionWebRequestEventRouter::GetInstance()->FindEventListener(id);
+  ASSERT_NE(nullptr, listener);
+  EXPECT_EQ(0u, listener->blocked_requests.size());
+
+  // Send a request. It should block. Wait for the run loop to become idle.
+  GURL request_url("about:blank");
+  std::unique_ptr<net::URLRequest> request(
+      context_->CreateRequest(request_url, net::DEFAULT_PRIORITY, &delegate_));
+  // Extension response for OnErrorOccurred: Terminate the message loop.
+  {
+    base::RunLoop run_loop;
+    ipc_sender_.PushTask(
+        base::Bind(base::IgnoreResult(&base::SingleThreadTaskRunner::PostTask),
+                   base::ThreadTaskRunnerHandle::Get(), FROM_HERE,
+                   run_loop.QuitWhenIdleClosure()));
+    request->Start();
+    run_loop.Run();
+  }
+
+  // Confirm that there is a blocked request.
+  EXPECT_EQ(1u, listener->blocked_requests.size());
+
+  // Send a response through.
+  ExtensionWebRequestEventRouter::EventResponse* response =
+      new ExtensionWebRequestEventRouter::EventResponse(
+          extension_id, base::Time::FromDoubleT(1));
+  response->cancel = true;
+  ExtensionWebRequestEventRouter::GetInstance()->OnEventHandled(
+      &profile_, extension_id, kEventName, kEventName + "/1",
+      request->identifier(), response);
+  {
+    base::RunLoop run_loop;
+    run_loop.RunUntilIdle();
+  }
+
+  // Now there should be no blocked requests.
+  EXPECT_EQ(0u, listener->blocked_requests.size());
+
+  EXPECT_TRUE(!request->is_pending());
+  EXPECT_EQ(net::URLRequestStatus::FAILED, request->status().status());
+  EXPECT_EQ(net::ERR_BLOCKED_BY_CLIENT, request->status().error());
+  EXPECT_EQ(0U, ipc_sender_.GetNumTasks());
+
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id);
+}
+
 struct HeaderModificationTest_Header {
   const char* name;
   const char* value;
 };
 
 struct HeaderModificationTest_Modification {
   enum Type {
     SET,
     REMOVE
   };
@@ skipping 192 matching lines @@
       std::string value;
       ASSERT_TRUE(header->GetString(keys::kHeaderNameKey, &key));
       ASSERT_TRUE(header->GetString(keys::kHeaderValueKey, &value));
       observed_headers.SetHeader(key, value);
     }
 
     EXPECT_EQ(expected_headers.ToString(), observed_headers.ToString());
     ++num_headers_observed;
   }
   EXPECT_EQ(1, num_headers_observed);
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+  ExtensionWebRequestEventRouter::EventListener::Identifier id1(
       &profile_, extension1_id, kEventName + "/1", 0, 0);
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
+  ExtensionWebRequestEventRouter::EventListener::Identifier id2(
       &profile_, extension2_id, kEventName + "/2", 0, 0);
-  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(
-      &profile_, extension3_id,
-      std::string(keys::kOnSendHeadersEvent) + "/3", 0, 0);
+  ExtensionWebRequestEventRouter::EventListener::Identifier id3(
+      &profile_, extension3_id, std::string(keys::kOnSendHeadersEvent) + "/3",
+      0, 0);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id1);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id2);
+  ExtensionWebRequestEventRouter::GetInstance()->RemoveEventListener(id3);
 };
 
 namespace {
 
 void TestInitFromValue(const std::string& values, bool expected_return_code,
                        int expected_extra_info_spec) {
   int actual_info_spec;
   bool actual_return_code = GenerateInfoSpec(values, &actual_info_spec);
   EXPECT_EQ(expected_return_code, actual_return_code);
   if (expected_return_code)
@@ skipping 1249 matching lines @@
   EXPECT_TRUE(credentials_set);
   EXPECT_FALSE(auth3.Empty());
   EXPECT_EQ(username, auth1.username());
   EXPECT_EQ(password, auth1.password());
   EXPECT_EQ(1u, warning_set.size());
   EXPECT_TRUE(HasWarning(warning_set, "extid2"));
   EXPECT_EQ(3u, capturing_net_log.GetSize());
 }
 
 }  // namespace extensions