[DNR] GetMatchedRules timestamp filtering

Introduce a new parameter to getMatchedRules which allows the extension
to specify a minimum timestamp, and all matched rules more recent than
that timestamp will be returned.

BUG=983761

Change-Id: I55a0f4fc9ffc2be53502930b7ac2695ca4d6c905
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1966375
Commit-Queue: Kelvin Jiang <kelvinjiang@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#730307}

chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc

@@ -21,12 +21,15 @@
 #include "base/path_service.h"
 #include "base/rand_util.h"
 #include "base/run_loop.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/lock.h"
 #include "base/task/post_task.h"
 #include "base/test/metrics/histogram_tester.h"
+#include "base/test/simple_test_clock.h"
 #include "base/threading/thread_restrictions.h"
+#include "base/time/time.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/browser/extensions/api/extension_action/test_extension_action_api_observer.h"
@@ -62,6 +65,7 @@
 #include "content/public/test/simple_url_loader_test_helper.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
+#include "extensions/browser/api/declarative_net_request/action_tracker.h"
 #include "extensions/browser/api/declarative_net_request/constants.h"
 #include "extensions/browser/api/declarative_net_request/rules_monitor_service.h"
 #include "extensions/browser/api/declarative_net_request/ruleset_manager.h"
@@ -556,6 +560,32 @@ class DeclarativeNetRequestBrowserTest
         base::StringPrintf(kGetMatchedRulesScript, tab_id_param.c_str()));
   }
 
+  // Calls getMatchedRules for |extension_id|, |tab_id| and optionally,
+  // |timestamp|. Returns the matched rule count for the tab specified by
+  // |tab_id| and are more recent than |timestamp| if specified.
+  std::string GetMatchedRuleCountForTab(const ExtensionId& extension_id,
+                                        int tab_id,
+                                        base::Optional<base::Time> timestamp) {
+    const char kGetMatchedRulesScript[] = R"(
+      chrome.declarativeNetRequest.getMatchedRules(%s, (rules) => {
+        var rule_count = rules.rulesMatchedInfo.length;
+        window.domAutomationController.send(rule_count.toString());
+      });
+    )";
+
+    std::string param_string = base::StringPrintf("{tabId: %d}", tab_id);
+
+    if (timestamp) {
+      double timestamp_in_js = timestamp->ToJsTimeIgnoringNull();
+      param_string = base::StringPrintf("{tabId: %d, minTimeStamp: %f}", tab_id,
+                                        timestamp_in_js);
+    }
+
+    return ExecuteScriptInBackgroundPage(
+        extension_id,
+        base::StringPrintf(kGetMatchedRulesScript, param_string.c_str()));
+  }
+
   std::set<GURL> GetAndResetRequestsToServer() {
     base::AutoLock lock(requests_to_server_lock_);
     std::set<GURL> results = requests_to_server_;
@@ -3566,6 +3596,9 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest,
     base::Optional<int> tab_id;
     std::string expected_rule_and_tab_ids;
   } test_cases[] = {
+      // No rules should be matched for |extension_1| and the unknown tab ID.
+      {extension_1_id, extension_misc::kUnknownTabId, ""},
+
       // No filter is specified for |extension_1|, therefore two MatchedRuleInfo
       // should be returned:
       // (abc_id, first_tab_id) and (abc_id, second_tab_id)
@@ -3687,6 +3720,98 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest,
   EXPECT_EQ("", actual_rule_and_tab_ids);
 }
 
+// Test that getMatchedRules only returns rules more recent than the provided
+// timestamp.
+IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest,
+                       GetMatchedRulesTimestampFiltering) {
+  base::Time start_time = base::Time::Now();
+
+  base::SimpleTestClock clock_;
+  clock_.SetNow(start_time);
+  auto* rules_monitor_service =
+      declarative_net_request::RulesMonitorService::Get(profile());
+  rules_monitor_service->action_tracker().SetClockForTests(&clock_);
+
+  // Load the extension with a background script so scripts can be run from its
+  // generated background page.
+  set_has_background_script(true);
+
+  const std::string kFrameName1 = "frame1";
+  const GURL page_url = embedded_test_server()->GetURL(
+      "nomatch.com", "/page_with_two_frames.html");
+
+  const std::string example_host = "example.com";
+  const GURL sub_frame_url = embedded_test_server()->GetURL(
+      example_host, "/pages_with_script/index.html");
+
+  TestRule rule = CreateGenericRule();
+  rule.id = kMinValidID;
+  rule.condition->url_filter = example_host;
+  rule.condition->resource_types = std::vector<std::string>({"sub_frame"});
+  rule.action->type = "block";
+
+  ASSERT_NO_FATAL_FAILURE(LoadExtensionWithRules({rule}, "extension_1", {}));
+  const ExtensionId& extension_id = last_loaded_extension_id();
+  ui_test_utils::NavigateToURL(browser(), page_url);
+
+  // Using subframes here to make requests without triggering main-frame
+  // navigations. This request will match with the block rule for example.com at
+  // |start_time|.
+  NavigateFrame(kFrameName1, sub_frame_url);
+
+  const char getMatchedRuleTimestampScript[] = R"(
+    chrome.declarativeNetRequest.getMatchedRules((rules) => {
+      var rule_count = rules.rulesMatchedInfo.length;
+      var timestamp = rule_count === 1 ?
+          rules.rulesMatchedInfo[0].timeStamp.toString() : '';
+
+      window.domAutomationController.send(timestamp);
+    });
+  )";
+
+  std::string timestamp_string = ExecuteScriptInBackgroundPage(
+      extension_id, getMatchedRuleTimestampScript);
+
+  double matched_rule_timestamp;
+  ASSERT_TRUE(base::StringToDouble(timestamp_string, &matched_rule_timestamp));
+
+  // Verify that the rule was matched at |start_time|.
+  EXPECT_DOUBLE_EQ(start_time.ToJsTimeIgnoringNull(), matched_rule_timestamp);
+
+  // Advance the clock to capture a timestamp after when the first request was
+  // made.
+  clock_.Advance(base::TimeDelta::FromMilliseconds(100));
+  base::Time timestamp_1 = clock_.Now();
+  clock_.Advance(base::TimeDelta::FromMilliseconds(100));
+
+  // Navigate to example.com again. This should cause |rule| to be matched.
+  NavigateFrame(kFrameName1, sub_frame_url);
+
+  // Advance the clock to capture a timestamp after when the second request was
+  // made.
+  clock_.Advance(base::TimeDelta::FromMilliseconds(100));
+  base::Time timestamp_2 = clock_.Now();
+
+  int first_tab_id = ExtensionTabUtil::GetTabId(web_contents());
+
+  // Two rules should be matched on |first_tab_id|.
+  std::string rule_count =
+      GetMatchedRuleCountForTab(extension_id, first_tab_id, base::nullopt);
+  EXPECT_EQ("2", rule_count);
+
+  // Only one rule should be matched on |first_tab_id| after |timestamp_1|.
+  rule_count =
+      GetMatchedRuleCountForTab(extension_id, first_tab_id, timestamp_1);
+  EXPECT_EQ("1", rule_count);
+
+  // No rules should be matched on |first_tab_id| after |timestamp_2|.
+  rule_count =
+      GetMatchedRuleCountForTab(extension_id, first_tab_id, timestamp_2);
+  EXPECT_EQ("0", rule_count);
+
+  rules_monitor_service->action_tracker().SetClockForTests(nullptr);
+}
+
 // Test fixture to verify that host permissions for the request url and the
 // request initiator are properly checked when redirecting requests. Loads an
 // example.com url with four sub-frames named frame_[1..4] from hosts

extensions/browser/api/declarative_net_request/action_tracker.cc

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/stl_util.h"
+#include "base/time/clock.h"
 #include "base/values.h"
 #include "extensions/browser/api/declarative_net_request/request_action.h"
 #include "extensions/browser/api/declarative_net_request/rules_monitor_service.h"
@@ -35,6 +36,12 @@ bool IsMainFrameNavigationRequest(const WebRequestInfo& request_info) {
          request_info.type == content::ResourceType::kMainFrame;
 }
 
+const base::Clock* g_test_clock = nullptr;
+
+base::Time GetNow() {
+  return g_test_clock ? g_test_clock->Now() : base::Time::Now();
+}
+
 }  // namespace
 
 ActionTracker::ActionTracker(content::BrowserContext* browser_context)
@@ -53,6 +60,11 @@ ActionTracker::~ActionTracker() {
   DCHECK(pending_navigation_actions_.empty());
 }
 
+// static
+void ActionTracker::SetClockForTests(const base::Clock* clock) {
+  g_test_clock = clock;
+}
+
 void ActionTracker::OnRuleMatched(const RequestAction& request_action,
                                   const WebRequestInfo& request_info) {
   DispatchOnRuleMatchedDebugIfNeeded(request_action,
@@ -213,14 +225,19 @@ void ActionTracker::ResetTrackedInfoForTab(int tab_id, int64_t navigation_id) {
 
 std::vector<dnr_api::MatchedRuleInfo> ActionTracker::GetMatchedRules(
     const ExtensionId& extension_id,
-    base::Optional<int> tab_id) const {
+    const base::Optional<int>& tab_id,
+    const base::Time& min_time_stamp) {
   std::vector<dnr_api::MatchedRuleInfo> matched_rules;
 
-  auto add_to_matched_rules = [this, &matched_rules](
+  auto add_to_matched_rules = [this, &matched_rules, &min_time_stamp](
                                   const std::list<TrackedRule>& tracked_rules,
                                   int tab_id) {
-    for (const TrackedRule& tracked_rule : tracked_rules)
-      matched_rules.push_back(CreateMatchedRuleInfo(tracked_rule, tab_id));
+    for (const TrackedRule& tracked_rule : tracked_rules) {
+      // Filter by the provided |min_time_stamp| for both active and non-active
+      // tabs.
+      if (tracked_rule.time_stamp >= min_time_stamp)
+        matched_rules.push_back(CreateMatchedRuleInfo(tracked_rule, tab_id));
+    }
   };
 
   if (tab_id.has_value()) {
@@ -290,7 +307,7 @@ bool ActionTracker::TrackedInfoContextKey<T>::operator<(
 ActionTracker::TrackedRule::TrackedRule(
     int rule_id,
     api::declarative_net_request::SourceType source_type)
-    : rule_id(rule_id), source_type(source_type) {}
+    : rule_id(rule_id), source_type(source_type), time_stamp(GetNow()) {}
 
 ActionTracker::TrackedInfo::TrackedInfo() = default;
 ActionTracker::TrackedInfo::~TrackedInfo() = default;
@@ -368,8 +385,8 @@ dnr_api::MatchedRuleInfo ActionTracker::CreateMatchedRuleInfo(
   dnr_api::MatchedRuleInfo matched_rule_info;
   matched_rule_info.rule = std::move(matched_rule);
   matched_rule_info.tab_id = tab_id;
+  matched_rule_info.time_stamp = tracked_rule.time_stamp.ToJsTimeIgnoringNull();
 
-  // TODO(crbug.com/983761): Populate timestamp for |matched_rule_info|.
   return matched_rule_info;
 }
 

extensions/browser/api/declarative_net_request/action_tracker.h

@@ -9,9 +9,14 @@
 #include <map>
 #include <vector>
 
+#include "base/time/time.h"
 #include "extensions/common/api/declarative_net_request.h"
 #include "extensions/common/extension_id.h"
 
+namespace base {
+class Clock;
+}
+
 namespace content {
 class BrowserContext;
 }
@@ -31,6 +36,10 @@ class ActionTracker {
   ActionTracker(const ActionTracker& other) = delete;
   ActionTracker& operator=(const ActionTracker& other) = delete;
 
+  // Sets a custom Clock to use in tests. |clock| should be owned by the caller
+  // of this function.
+  static void SetClockForTests(const base::Clock* clock);
+
   // Called whenever a request matches with a rule.
   void OnRuleMatched(const RequestAction& request_action,
                      const WebRequestInfo& request_info);
@@ -60,7 +69,8 @@ class ActionTracker {
   // rules matched for |tab_id| will be returned.
   std::vector<api::declarative_net_request::MatchedRuleInfo> GetMatchedRules(
       const ExtensionId& extension_id,
-      base::Optional<int> tab_id) const;
+      const base::Optional<int>& tab_id,
+      const base::Time& min_time_stamp);
 
   // Returns the number of matched rules in |rules_tracked_| for the given
   // |extension_id| and |tab_id|. Should only be used for tests.
@@ -103,6 +113,10 @@ class ActionTracker {
 
     const int rule_id;
     const api::declarative_net_request::SourceType source_type;
+
+    // The timestamp for when the rule was matched. This is set in the
+    // constructor.
+    const base::Time time_stamp;
   };
 
   // Info tracked for each ExtensionTabIdKey or ExtensionNavigationIdKey.

extensions/browser/api/declarative_net_request/declarative_net_request_api.cc

@@ -12,6 +12,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/task/post_task.h"
 #include "base/task_runner_util.h"
+#include "base/time/time.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/browser/api/declarative_net_request/action_tracker.h"
@@ -282,21 +283,26 @@ DeclarativeNetRequestGetMatchedRulesFunction::Run() {
   EXTENSION_FUNCTION_VALIDATE(error.empty());
 
   base::Optional<int> tab_id;
-  // TODO(crbug.com/983761): Add timestamp filtering as well.
+  base::Time min_time_stamp = base::Time::Min();
 
-  if (params->filter && params->filter->tab_id)
-    tab_id = *params->filter->tab_id;
+  if (params->filter) {
+    if (params->filter->tab_id)
+      tab_id = *params->filter->tab_id;
+
+    if (params->filter->min_time_stamp)
+      min_time_stamp = base::Time::FromJsTime(*params->filter->min_time_stamp);
+  }
 
   declarative_net_request::RulesMonitorService* rules_monitor_service =
       declarative_net_request::RulesMonitorService::Get(browser_context());
   DCHECK(rules_monitor_service);
 
-  const declarative_net_request::ActionTracker& action_tracker =
+  declarative_net_request::ActionTracker& action_tracker =
       rules_monitor_service->action_tracker();
 
   dnr_api::RulesMatchedDetails details;
   details.rules_matched_info =
-      action_tracker.GetMatchedRules(extension_id(), tab_id);
+      action_tracker.GetMatchedRules(extension_id(), tab_id, min_time_stamp);
 
   return RespondNow(
       ArgumentList(dnr_api::GetMatchedRules::Results::Create(details)));

extensions/common/api/declarative_net_request.idl

@@ -262,7 +262,7 @@ namespace declarativeNetRequest {
     long? tabId;
 
     // If specified, only matches rules after the given timestamp.
-    long? minTimeStamp;
+    double? minTimeStamp;
   };
 
   [nodoc]