Encapsulate CSS selector declarative content condition tracking

chrome/browser/extensions/api/declarative_content/declarative_content_css_condition_tracker.cc

Index: chrome/browser/extensions/api/declarative_content/declarative_content_css_condition_tracker.cc
diff --git a/chrome/browser/extensions/api/declarative_content/declarative_content_css_condition_tracker.cc b/chrome/browser/extensions/api/declarative_content/declarative_content_css_condition_tracker.cc
new file mode 100644
index 0000000000000000000000000000000000000000..642645f3baa683a7a95516c782ae493f09183c0c
--- /dev/null
+++ b/chrome/browser/extensions/api/declarative_content/declarative_content_css_condition_tracker.cc
@@ -0,0 +1,203 @@
+// Copyright 2015 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 "chrome/browser/extensions/api/declarative_content/declarative_content_css_condition_tracker.h"
+
+#include <algorithm>
+
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_iterator.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "content/public/browser/navigation_details.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_source.h"
+#include "content/public/browser/render_process_host.h"
+#include "extensions/common/extension_messages.h"
+#include "ipc/ipc_message.h"
+#include "ipc/ipc_message_macros.h"
+
+namespace extensions {
+
+DeclarativeContentCssConditionTrackerDelegate::
+DeclarativeContentCssConditionTrackerDelegate() {}
+
+DeclarativeContentCssConditionTrackerDelegate::
+~DeclarativeContentCssConditionTrackerDelegate() {}
+
+DeclarativeContentCssConditionTracker::DeclarativeContentCssConditionTracker(
+    content::BrowserContext* context,
+    DeclarativeContentCssConditionTrackerDelegate* delegate)
+    : context_(context),
+      delegate_(delegate) {
+  // Observe all the existing WebContents for matched CSS selector changes.
+  for (chrome::BrowserIterator it; !it.done(); it.Next()) {
+    Browser* browser = *it;
+    if (!delegate_->ShouldManageConditionsForBrowserContext(browser->profile()))
+      continue;
+
+    for (int i = 0, tab_count = browser->tab_strip_model()->count();
+         i < tab_count; ++i) {
+      content::WebContents* contents =
+          browser->tab_strip_model()->GetWebContentsAt(i);
+      matched_selector_change_observers_[contents] =
+          make_linked_ptr(new MatchedSelectorChangeObserver(
+              contents,
+              base::Bind(&DeclarativeContentCssConditionTracker::
+                         UpdateMatchingCssSelectors,
+                         base::Unretained(this))));
+    }
+  }
+
+  registrar_.Add(this,
+                 content::NOTIFICATION_RENDERER_PROCESS_CREATED,
+                 content::NotificationService::AllBrowserContextsAndSources());
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_TAB_ADDED,
+                 content::NotificationService::AllSources());
+  registrar_.Add(this,
+                 content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
+                 content::NotificationService::AllBrowserContextsAndSources());
+}
+
+DeclarativeContentCssConditionTracker::
+~DeclarativeContentCssConditionTracker() {}
+
+// We use the sorted propery of the set for equality checks with
+// watched_css_selectors_, which is guaranteed to be sorted because it's set
+// from the set contents.
+void DeclarativeContentCssConditionTracker::SetWatchedCssSelectors(
+    const std::set<std::string>& new_watched_css_selectors) {
+  if (new_watched_css_selectors.size() != watched_css_selectors_.size() ||
+      !std::equal(new_watched_css_selectors.begin(),
+                  new_watched_css_selectors.end(),
+                  watched_css_selectors_.begin())) {
+    watched_css_selectors_.assign(new_watched_css_selectors.begin(),
+                                  new_watched_css_selectors.end());
+
+    for (content::RenderProcessHost::iterator it(
+             content::RenderProcessHost::AllHostsIterator());
+         !it.IsAtEnd();
+         it.Advance()) {
+      InstructRenderProcessIfManagingBrowserContext(it.GetCurrentValue());
+    }
+  }
+}
+
+void DeclarativeContentCssConditionTracker::OnWebContentsNavigation(

[not at google - send to devlin, 2015/06/01 22:51:11]

Can this be part of your own WebContentsObserver implementation? See comment
below.

[Mike Wittman, 2015/06/05 01:17:44]

It could as it stands now, but once I encapsulate the URL matcher tracking
ChromeContentRulesRegistry will still need to handle this and call into the
trackers itself to avoid inconsistency in condition evaluation. The problem is
that, if both trackers individually listen to this event, they'll each
individually request rule evaluation. Whichever tracker does this first will
have its conditions evaluated on the new URL, while the other tracker won't have
been notified will have its conditions evaluated on the old URL. If
ChromeContentRulesRegistry coordinates, then we can delay evaluation until all
trackers have had a chance to respond to the notification.

I suppose we could also address this by posting the rule evaluation invocation
back on the event loop. That would scale better to a large number of independent
trackers by eliminating coordination on common events, but I'm not sure if it's
worth doing at this point.

[not at google - send to devlin, 2015/06/05 17:16:30]

Ah right. No, what you have sounds right them. I don't think that posting to the
event loop will necessarily work, since you'd have the same problem when the
event loop next runs?

[Mike Wittman, 2015/06/05 21:00:46]

I think it should work if all the tracker state is updated synchronously from
the notification, and just the evaluation is posted back to the event loop. The
OnWebContentsNavigation notifications for a given navigation occur in a single
invocation from the event loop, right?

[not at google - send to devlin, 2015/06/05 21:15:21]

not sure, but I don't particularly trust that sort of async code as an
implementation detail? what you have is fine I think.

+    content::WebContents* contents,
+    const content::LoadCommittedDetails& details,
+    const content::FrameNavigateParams& params) {
+  if (details.is_in_page) {
+    // Within-page navigations don't change the set of elements that
+    // exist, and we only support filtering on the top-level URL, so
+    // this can't change which rules match.
+    return;
+  }
+
+  // Top-level navigation produces a new document. Initially, the
+  // document's empty, so no CSS rules match.  The renderer will send
+  // an ExtensionHostMsg_OnWatchedPageChange later if any CSS rules
+  // match.
+  matching_css_selectors_[contents].clear();
+  delegate_->RequestEvaluation(contents);
+}
+
+void DeclarativeContentCssConditionTracker::GetMatchingCssSelectors(
+    content::WebContents* contents,
+    base::hash_set<std::string>* css_selectors) {
+  css_selectors->insert(matching_css_selectors_[contents].begin(),
+                        matching_css_selectors_[contents].end());
+}
+
+void DeclarativeContentCssConditionTracker::
+UpdateMatchingCssSelectorsForTesting(
+    content::WebContents* contents,
+    const std::vector<std::string>& matching_css_selectors) {
+  UpdateMatchingCssSelectors(contents, matching_css_selectors);
+}
+
+DeclarativeContentCssConditionTracker::MatchedSelectorChangeObserver::
+MatchedSelectorChangeObserver(
+    content::WebContents* contents,
+    const NotificationCallback& notification_callback)
+    : WebContentsObserver(contents),
+      notification_callback_(notification_callback) {
+}
+
+DeclarativeContentCssConditionTracker::MatchedSelectorChangeObserver::
+~MatchedSelectorChangeObserver() {
+}
+
+bool
+DeclarativeContentCssConditionTracker::MatchedSelectorChangeObserver::
+OnMessageReceived(
+    const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(MatchedSelectorChangeObserver, message)
+    IPC_MESSAGE_HANDLER(ExtensionHostMsg_OnWatchedPageChange,
+                        OnWatchedPageChange)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
+void
+DeclarativeContentCssConditionTracker::MatchedSelectorChangeObserver::
+OnWatchedPageChange(
+    const std::vector<std::string>& css_selectors) {
+  notification_callback_.Run(web_contents(), css_selectors);
+}
+
+void DeclarativeContentCssConditionTracker::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  switch (type) {
+    case content::NOTIFICATION_RENDERER_PROCESS_CREATED: {
+      content::RenderProcessHost* process =
+          content::Source<content::RenderProcessHost>(source).ptr();
+      InstructRenderProcessIfManagingBrowserContext(process);
+      break;
+    }
+    case chrome::NOTIFICATION_TAB_ADDED: {
+      content::WebContents* contents =
+          content::Details<content::WebContents>(details).ptr();
+      matched_selector_change_observers_[contents] =
+          make_linked_ptr(new MatchedSelectorChangeObserver(
+              contents,
+              base::Bind(&DeclarativeContentCssConditionTracker::
+                         UpdateMatchingCssSelectors,
+                         base::Unretained(this))));
+      break;
+    }
+    case content::NOTIFICATION_WEB_CONTENTS_DESTROYED: {

[not at google - send to devlin, 2015/06/01 22:51:11]

You should be able to include this as part of a WebContentsObserver interface -
for example, MatchedSelectorChangeObserver. That should let you make some more
simplifications: MatchedSelectorChangeObserver would then be able to manage its
own lifetime, so you wouldn't need to hold onto it in |matching_css_selectors_|,
and given that you'd be able to define the entire class within the .cc file
rather than in the header file.

(you'd also want to rename it)

[Mike Wittman, 2015/06/05 01:17:44]

Good point. I think it makes sense to go further and make object own and manage
all of the per-WebContents state.

+      content::WebContents* contents =
+          content::Source<content::WebContents>(source).ptr();
+      matched_selector_change_observers_.erase(contents);
+      matching_css_selectors_.erase(contents);
+      break;
+    }
+  }
+}
+
+void DeclarativeContentCssConditionTracker::
+InstructRenderProcessIfManagingBrowserContext(
+    content::RenderProcessHost* process) {
+  if (delegate_->ShouldManageConditionsForBrowserContext(
+          process->GetBrowserContext())) {
+    process->Send(new ExtensionMsg_WatchPages(watched_css_selectors_));
+  }
+}
+
+void DeclarativeContentCssConditionTracker::UpdateMatchingCssSelectors(
+    content::WebContents* contents,
+    const std::vector<std::string>& matching_css_selectors) {
+  matching_css_selectors_[contents] = matching_css_selectors;
+
+  delegate_->RequestEvaluation(contents);
+}
+
+}  // namespace extensions