Always use main URL from main frame when searching for groups

Previously the groups service used the first HTTP(S) URL returned by
`store.searchUris()` to get the URL used to search for associated groups. In
practice, the URIs from the main frame always appeared first in this
list, and the main URL from that frame was the first entry within that sublist.
However, the `store.searchUris` function doesn't provide any guarantee of this
and future changes could break this invariant.

This commit adds a `store.mainFrame()` function which returns the main
frame, the one with no `id`, and modifies the groups service to use the
main URI for that frame.

A change in behavior here is that if the top-level frame was a non-HTTPS
frame but embedded an HTTPS frame, then the group search would have used
the URL from the HTTPS frame. Now it will use the non-HTTPS URL from the
top-level frame.

src/sidebar/services/groups.js

@@ -37,15 +37,11 @@ function groups(
 
   function getDocumentUriForGroupSearch() {
     function mainUri() {
-      const uris = store.searchUris();
-      if (uris.length === 0) {
+      const mainFrame = store.mainFrame();
+      if (!mainFrame) {
         return null;
       }
-
-      // We get the first HTTP URL here on the assumption that group scopes must
-      // be domains (+paths)? and therefore we need to look up groups based on
-      // HTTP URLs (so eg. we cannot use a "file:" URL or PDF fingerprint).
-      return uris.find(uri => uri.startsWith('http'));
+      return mainFrame.uri;
     }
     return awaitStateChange(store, mainUri);
   }

src/sidebar/services/test/groups-test.js

@@ -61,7 +61,7 @@ describe('groups', function() {
 
     fakeStore = fakeReduxStore(
       {
-        searchUris: ['http://example.org'],
+        mainFrame: { uri: 'http://example.org' },
         focusedGroup: null,
         groups: [],
         directLinkedGroupId: null,
@@ -80,8 +80,8 @@ describe('groups', function() {
         focusedGroup() {
           return this.getState().focusedGroup;
         },
-        searchUris() {
-          return this.getState().searchUris;
+        mainFrame() {
+          return this.getState().mainFrame;
         },
         focusedGroupId() {
           const group = this.getState().focusedGroup;
@@ -417,9 +417,9 @@ describe('groups', function() {
       it('waits for the document URL to be determined', () => {
         const svc = service();
 
-        fakeStore.setState({ searchUris: [] });
+        fakeStore.setState({ mainFrame: null });
         const loaded = svc.load();
-        fakeStore.setState({ searchUris: ['https://asite.com'] });
+        fakeStore.setState({ mainFrame: { uri: 'https://asite.com' } });
 
         return loaded.then(() => {
           assert.calledWith(fakeApi.groups.list, {
@@ -436,7 +436,7 @@ describe('groups', function() {
       });
 
       it('does not wait for the document URL', () => {
-        fakeStore.setState({ searchUris: [] });
+        fakeStore.setState({ mainFrame: null });
         const svc = service();
         return svc.load().then(() => {
           assert.calledWith(fakeApi.groups.list, {
@@ -804,14 +804,16 @@ describe('groups', function() {
       it('should refetch groups if main frame URL has changed', () => {
         const svc = service();
 
-        fakeStore.setState({ searchUris: ['https://domain.com/page-a'] });
+        fakeStore.setState({ mainFrame: { uri: 'https://domain.com/page-a' } });
         return svc
           .load()
           .then(() => {
             // Simulate main frame URL change, eg. due to client-side navigation in
             // a single page application.
             fakeApi.groups.list.resetHistory();
-            fakeStore.setState({ searchUris: ['https://domain.com/page-b'] });
+            fakeStore.setState({
+              mainFrame: { uri: 'https://domain.com/page-b' },
+            });
 
             return fakeRootScope.eventCallbacks[events.FRAME_CONNECTED]();
           })
@@ -823,7 +825,7 @@ describe('groups', function() {
       it('should not refetch groups if main frame URL has not changed', () => {
         const svc = service();
 
-        fakeStore.setState({ searchUris: ['https://domain.com/page-a'] });
+        fakeStore.setState({ mainFrame: { uri: 'https://domain.com/page-a' } });
         return svc
           .load()
           .then(() => {

src/sidebar/store/modules/frames.js

 'use strict';
 
+const { createSelector } = require('reselect');
+
 const util = require('../util');
 
 function init() {
@@ -71,6 +73,24 @@ function frames(state) {
   return state.frames;
 }
 
+/**
+ * Return the "main" frame that the sidebar is connected to.
+ *
+ * The sidebar may be connected to multiple frames from different URLs.
+ * For some purposes, the main frame (typically the top-level one that contains
+ * the sidebar) needs to be distinguished. This selector returns the main frame
+ * for that purpose.
+ *
+ * This may be `null` during startup.
+ */
+const mainFrame = createSelector(
+  state => state.frames,
+
+  // Sub-frames will all have a "frame identifier" set. The main frame is the
+  // one with a `null` id.
+  frames => frames.find(f => !f.id) || null
+);
+
 function searchUrisForFrame(frame) {
   let uris = [frame.uri];
 
@@ -113,6 +133,7 @@ module.exports = {
 
   selectors: {
     frames,
+    mainFrame,
     searchUris,
   },
 };

src/sidebar/store/modules/test/frames-test.js

 'use strict';
 
 const frames = require('../frames');
+const createStore = require('../../create-store');
+
 const session = require('../session');
 const util = require('../../util');
 const unroll = require('../../../../shared/test/util').unroll;
@@ -13,7 +15,16 @@ function init() {
   return Object.assign({}, frames.init(), session.init());
 }
 
-describe('frames reducer', function() {
+describe('sidebar/store/modules/frames', function() {
+  let store;
+
+  beforeEach(() => {
+    // Setup a store for tests. Note that some of the tests in this module
+    // pre-date the `createStore` helper and have not been refactored to use
+    // it yet.
+    store = createStore([frames]);
+  });
+
   describe('#connectFrame', function() {
     it('adds the frame to the list of connected frames', function() {
       const frame = { uri: 'http://example.com' };
@@ -71,6 +82,34 @@ describe('frames reducer', function() {
     });
   });
 
+  describe('#mainFrame', () => {
+    it('returns `null` if no frames are connected', () => {
+      assert.isNull(store.mainFrame());
+    });
+
+    [
+      {
+        frames: [{ id: null, uri: 'https://example.org' }],
+        expectedFrame: 0,
+      },
+      {
+        frames: [
+          // An iframe which is also connected.
+          { id: 'iframe1', uri: 'https://foo.com/' },
+
+          // The top-level frame.
+          { id: null, uri: 'https://example.org' },
+        ],
+        expectedFrame: 1,
+      },
+    ].forEach(({ frames, expectedFrame }) => {
+      it('returns the main frame from the frames connected to the sidebar', () => {
+        frames.forEach(frame => store.connectFrame(frame));
+        assert.equal(store.mainFrame(), frames[expectedFrame]);
+      });
+    });
+  });
+
   describe('#searchUris', function() {
     unroll(
       'returns the expected search URIs (#when)',