Use an optional permission for tabs (#145)

This will remove the need to grant a seemingly dodgy permission on install.

manifest.json

@@ -2,7 +2,8 @@
   "name": "PR Monitor",
   "version": "0.0.0",
   "description": "Get notified when you receive a pull request on GitHub.",
-  "permissions": ["alarms", "notifications", "storage", "tabs"],
+  "permissions": ["alarms", "notifications", "storage"],
+  "optional_permissions": ["tabs"],
   "background": {
     "scripts": ["background.js"],
     "persistent": true

src/chrome/fake-chrome.ts

@@ -39,6 +39,16 @@ export const fakeChrome = (<Partial<ChromeApi>>{
       }
     }
   },
+  permissions: {
+    request(_permissions, callback) {
+      if (callback) {
+        callback(true);
+      }
+    },
+    getAll(callback) {
+      callback({});
+    }
+  },
   storage: {
     // To simulate chrome.storage.local, we simply fall back to the localStorage API.
     local: {
@@ -63,6 +73,9 @@ export const fakeChrome = (<Partial<ChromeApi>>{
   tabs: {
     query(_queryInfo, callback) {
       callback([]);
+    },
+    create(properties) {
+      window.open(properties.url);
     }
   }
 }) as ChromeApi;

src/components/Popup.tsx

@@ -128,7 +128,7 @@ export class Popup extends Component<PopupProps, PopupState> {
   }
 
   private onOpen = (pullRequestUrl: string) => {
-    this.props.core.openPullRequest(pullRequestUrl);
+    this.props.core.openPullRequest(pullRequestUrl).catch(console.error);
   };
 
   private onMute = (pullRequest: PullRequest) => {

src/environment/implementation.ts

@@ -8,13 +8,14 @@ import { buildTabOpener } from "../tabs/implementation";
 import { Environment } from "./api";
 
 export function buildEnvironment(chromeApi: ChromeApi): Environment {
+  const store = buildStore(chromeApi);
   return {
-    store: buildStore(chromeApi),
+    store,
     githubLoader: buildGitHubLoader(),
     notifier: buildNotifier(chromeApi),
     badger: buildBadger(chromeApi),
     messenger: buildMessenger(chromeApi),
-    tabOpener: buildTabOpener(chromeApi),
+    tabOpener: buildTabOpener(chromeApi, store),
     isOnline: () => navigator.onLine
   };
 }

src/environment/testing/fake.ts

@@ -39,7 +39,8 @@ function fakeStore() {
     lastCheck: fakeStorage<LoadedState | null>(null),
     muteConfiguration: fakeStorage<MuteConfiguration>(NOTHING_MUTED),
     notifiedPullRequests: fakeStorage<string[]>([]),
-    token: fakeStorage<string | null>(null)
+    token: fakeStorage<string | null>(null),
+    lastRequestForTabsPermission: fakeStorage<number | null>(null)
   };
 }
 
@@ -121,7 +122,7 @@ function fakeTabOpener() {
   const openedUrls: string[] = [];
   return {
     openedUrls,
-    openPullRequest(pullRequestUrl: string) {
+    async openPullRequest(pullRequestUrl: string) {
       openedUrls.push(pullRequestUrl);
     }
   };

src/state/core.ts

@@ -28,7 +28,9 @@ export class Core {
         this.load();
       }
     });
-    this.env.notifier.registerClickListener(url => this.openPullRequest(url));
+    this.env.notifier.registerClickListener(url =>
+      this.openPullRequest(url).catch(console.error)
+    );
   }
 
   async load() {
@@ -95,8 +97,8 @@ export class Core {
     }
   }
 
-  openPullRequest(pullRequestUrl: string) {
-    this.env.tabOpener.openPullRequest(pullRequestUrl);
+  async openPullRequest(pullRequestUrl: string) {
+    await this.env.tabOpener.openPullRequest(pullRequestUrl);
   }
 
   async mutePullRequest(pullRequest: {

src/storage/api.ts

@@ -29,6 +29,11 @@ export interface Store {
    * Storage of the user's provided GitHub token.
    */
   token: ValueStorage<string | null>;
+
+  /**
+   * Storage of the last timestamp we requested the "tabs" permission.
+   */
+  lastRequestForTabsPermission: ValueStorage<number | null>;
 }
 
 export interface ValueStorage<T> {

src/storage/implementation.ts

@@ -21,6 +21,10 @@ export function buildStore(chromeApi: ChromeApi): Store {
       "lastSeenPullRequests",
       []
     ),
-    token: chromeValueStorage<string>(chromeApi, "gitHubApiToken")
+    token: chromeValueStorage<string>(chromeApi, "gitHubApiToken"),
+    lastRequestForTabsPermission: chromeValueStorage<number>(
+      chromeApi,
+      "lastRequestForTabsPermission"
+    )
   };
 }

src/tabs/api.ts

@@ -2,5 +2,5 @@
  * TabOpener opens tabs intelligently (reusing them when possible).
  */
 export interface TabOpener {
-  openPullRequest(pullRequestUrl: string): void;
+  openPullRequest(pullRequestUrl: string): Promise<void>;
 }

src/tabs/implementation.ts

@@ -1,21 +1,57 @@
 import { ChromeApi } from "../chrome/api";
+import { Store } from "../storage/api";
 import { TabOpener } from "./api";
 
-export function buildTabOpener(chromeApi: ChromeApi): TabOpener {
+export function buildTabOpener(chromeApi: ChromeApi, store: Store): TabOpener {
   return {
-    openPullRequest(pullRequestUrl: string) {
-      chromeApi.tabs.query({}, tabs => {
-        const existingTab = tabs.find(tab =>
-          tab.url ? tab.url.startsWith(pullRequestUrl) : false
+    async openPullRequest(pullRequestUrl: string) {
+      const lastRequestTimestamp = await store.lastRequestForTabsPermission.load();
+      if (lastRequestTimestamp !== null) {
+        // We requested the permission before already. Let's not be persistent.
+        chromeApi.permissions.getAll(permissions => {
+          const granted = permissions.permissions
+            ? permissions.permissions.indexOf("tabs") !== -1
+            : false;
+          openTab(chromeApi, pullRequestUrl, granted);
+        });
+      } else {
+        await store.lastRequestForTabsPermission.save(Date.now());
+        chromeApi.permissions.request(
+          {
+            permissions: ["tabs"]
+          },
+          granted => {
+            openTab(chromeApi, pullRequestUrl, granted);
+          }
         );
-        if (existingTab) {
-          chromeApi.tabs.highlight({
-            tabs: existingTab.index
-          });
-        } else {
-          window.open(pullRequestUrl);
-        }
-      });
+      }
     }
   };
 }
+
+function openTab(
+  chromeApi: ChromeApi,
+  pullRequestUrl: string,
+  permissionGranted: boolean
+) {
+  if (permissionGranted) {
+    chromeApi.tabs.query({}, tabs => {
+      const existingTab = tabs.find(tab =>
+        tab.url ? tab.url.startsWith(pullRequestUrl) : false
+      );
+      if (existingTab) {
+        chromeApi.tabs.highlight({
+          tabs: existingTab.index
+        });
+      } else {
+        chrome.tabs.create({
+          url: pullRequestUrl
+        });
+      }
+    });
+  } else {
+    chrome.tabs.create({
+      url: pullRequestUrl
+    });
+  }
+}