feat(coil-extension): add iframes impl #415

sublimator · 2020-02-27T02:36:20Z

This pull request implements Web Monetization inside <iframe> elements. Only iframes that are direct children of the top frame, with monetization in their allow feature set policy will be monetized.

It does this by changing the 1:1 mapping of WM streams to browser tabs to M:1, with the 1:1 mapping now of WM streams to iframes.

To do so, it takes advantage of:

webNavigation wext apis
- New BackgroundFrameService
  - Keeps track of frames, including the current readyState/url/parentFrameId etc.
  - use frameChanged event where using chrome.tabs.onUpdated before (which was problematic)
- Will require extra permissions which may be off-putting to some users
  - This will not actually give us any more private information than we already have. This may not be clear to all users though.
MessageSender['frameId'], MessageSender['tab'] attributes
Messages sent using chrome.runtime.sendMessage api from content scripts
will be recieved in the background script onMessage listener with
tabId/frameId implied (i.e. not inside the message payload).

Whenever a monetization request occurs inside an iframe (that is only one level deep), a request will be sent to the background page to check if that iframe is allowed.

The request handler in the background page consults the BackgroundFramesService to determine the parent frame of the sender (currently always 0) then sends a message to the associated content script via tabs.sendMessage(tabId, msg, {frameId: parentId}). The msg contains the {tabId, frameId} monetization is requested from.

Inside the parent frame content script, a WeakMap of iframe elements to Promise<{tabId, frameId}> is maintained as well as a promise queue via a Map of correlationId to promise resolver.
A linear search is done over a query of all iframes, consulting the WeakMap, queuing new promises where necessary, awaiting each promise in turn. The promise result frame is checked against the search element frame.
- All <iframe>s in the page that have no promise queued already are sent a correlationId via frame.contentWindow!.postMessage
- Concurrent searches initiated via multiple child frames requesting monetization are handled fine using this method
Content scripts listen for the correlationId via adding listeners to the window message event
- This is then reported to the background page, which looks up the parent frame of the sender and reports the correlationId/frame pair and the correlationId associated promise is resolved with the frame.
- It seems highly unlikely that nefarious actors could listen in on the event, replaying the message to another window and beat the content script listener to use the correlationId.
  - We could take extra measures from the background page to ensure ids are only ever used once but it does not really seem necessary.
    - unknown correlationIds are ignored

Questions:

Should we limit the amount of concurrently monetized frames ?
- If so, how ?
Do we need to do anything special client side to adjust bandwidth? Or will the server do it all ?
- Is there anything needed server side to optimally support this ?
  - Seems to be some (transient) errors when a bunch of streams connect at the same time
  - it seems the retry logic handles them though
Should we allow arbitrarily nested iframes to monetize as long as long as all iframes in the iframe hierarchy have specified allow="monetization" ?
What does the text in the popup mean now that 'content' could refer to inner frames as well as the top page ?
Should there be some kind of 'in-page' indication that an iframe is being monetized ? (@fruehle offhand suggested maybe some kind of WM logo watermark)

MacOS Chrome 80.0.3987.132 (Official Build) (64-bit)

sublimator · 2020-02-27T03:42:43Z

packages/coil-extension/manifest.json

@@ -31,5 +31,5 @@
      "run_at": "document_start"
    }
  ],
-  "permissions": [ "activeTab", "storage", "notifications", "tabs", "<all_urls>" ]
+  "permissions": [ "webNavigation", "activeTab", "storage", "notifications", "tabs", "<all_urls>" ]


This is going to require extra permissions

Nothing more revealing (in terms of privacy) than what we already require but it could be off-putting to some users.

packages/coil-extension/src/content/services/ContentScript.ts

packages/coil-extension/src/background/services/BackgroundScript.ts

packages/coil-extension/src/content/services/ContentScript.ts

packages/coil-extension/src/content/services/Frames.ts

sublimator · 2020-03-02T04:42:59Z

packages/coil-extension/src/content/services/ContentScript.ts

+              this.runtime.sendMessage(message, resolve)
+            })
+        if (!allowed) {
+          console.error(


It might be nicer to log the actual element from the parent frame content script

packages/coil-extension/src/background/services/BackgroundFramesService.ts

sublimator · 2020-03-03T08:30:32Z

.circleci/config.yml

@@ -4,7 +4,7 @@ jobs:
  # Run Linting
  lint-all:
    docker:
-      - image: circleci/node:12.14.1-buster-browsers
+        - image: circleci/node:12.16.1-buster-browsers


Just hail Mary attempts at fixing some tests that fail on CI and not locally on my dev machine.

This reverts commit 6da1a6a.

sharafian

Have tried it out and it looks good! I think we should merge this and roll out a v47 with this and the tipping beta. The one issue I noticed is that mutating the allow attribute of an iframe doesn't get picked up by the extension, but we can always fix that at a later time if we decide that's supported