1190685 - Implement webNavigation.getFrame/getAllFrames

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect, P1)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://developer.chrome.com/extensions/webNavigation#method-getFrame
https://developer.chrome.com/extensions/webNavigation#method-getAllFrames

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: 0001-Bug-1190685-webext-Implements-webNavigation.getFrame.patch

This patch:

- defines the getFrame / getAllFrames API methods

- updates the webNavigation.jsom schema

- manages getFrame / getAllFrames request/response exchange, request tracking and cleanup when the originating context is closed (which is a part that definitely needs feedback)

- WebNavigation is now initialized on the first listener registered OR the first call to getFrame or getAllFrames

- prevents WebProgressListener from be uninitialized twice 

- defines handlers for the getFrame / getAllFrames requests received by the webNavigation framescript
  (search / collect all the docShell objects , retrieve the frame details that
  needs to be sent to the main process and routed to the extension context
  which originated the request)

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: 0002-Bug-1190685-webext-Add-webNavigation.getFrame-getAll.patch

This patch contains a new test case for the getFrame/getAllFrames API methods.

Comment 3

[Kris Maglione [:kmag]]

Comment on attachment 8709084 [details] [diff] [review]
0001-Bug-1190685-webext-Implements-webNavigation.getFrame.patch

Review of attachment 8709084 [details] [diff] [review]:
-----------------------------------------------------------------

This looks pretty good.

I haven't looked over all of WebNavigation.json or WebNavigationContent.js yet, because I think it needs to wait for my current patch to land rather than re-implement cross-process callback logic in yet another way. Sorry to block you on that. I'll get it up for review today.

::: toolkit/components/extensions/schemas/web_navigation.json
@@ +21,5 @@
>      ],
>      "functions": [
>        {
>          "name": "getFrame",
> +        "unsupported": false,

It's better to just remove the property. Same for the ones below.

@@ +31,5 @@
>              "name": "details",
>              "description": "Information about the frame to retrieve information about.",
>              "properties": {
>                "tabId": { "type": "integer", "minimum": 0, "description": "The ID of the tab in which the frame is." },
> +              "processId": {"unsupported": false, "type": "integer", "description": "The ID of the process runs the renderer for this tab."},

I think this is still unsupported.

::: toolkit/modules/addons/WebNavigation.jsm
@@ +128,5 @@
> +  },
> +
> +  getFrame(tab, context, details, callback) {
> +    if (!this.initialized) {
> +      this.init();

I'd rather avoid initializing all of the change listeners when we're just getting details on frames. Let's initialize the frame listeners and the change listeners separately.

@@ +145,5 @@
> +
> +    let reqId = this.addExtensionRequest(context, callback);
> +    let data = { details, reqId };
> +    let mm = tab.linkedBrowser.messageManager;
> +    mm.sendAsyncMessage("Extension:GetAllFrames:Req", data);

I'm trying to avoid adding any more ad-hoc callback routing. We should wait for my current patch to land. It should make this much easier.

::: toolkit/modules/addons/WebNavigationContent.js
@@ +110,5 @@
> + * @return   {nsIDOMWindow}          - the DOMWindow associated to the docShell.
> + */
> +function docShellToWindow(docShell) {
> +  return docShell.QueryInterface(Ci.nsIInterfaceRequestor)
> +    .getInterface(Ci.nsIDOMWindow);

Please align the '.' with the previous line.

@@ +135,5 @@
> +function convertWindowToFrameDetail(window) {
> +  return {
> +    windowId: getWindowId(window),
> +    parentWindowId: getParentWindowId(window),
> +    url: window.location.toString(),

|window.location.href|. Also, please try to avoid using |.toString()|. |String(...)| does the job better.

@@ +143,5 @@
> +// NOTE: Compute the Error Page loadType value. (See Bug 1190685 for rationale)
> +// - LOAD_CMD_NORMAL (as defined in "docshell/base/nsIDocShell.idl"):
> +// - LOAD_FLAGS_ERROR_PAGE (as defined in "docshell/base/nsDocShellLoadTypes.h")
> +// - MAKE_LOAD_TYPE (as defined in "docshell/base/nsDocShellLoadTypes.h")
> +// - LOAD_ERROR_PAGE (as defined in "docshell/base/nsDocShellLoadTypes.h")

Why aren't you using the constants directly from the interfaces?

@@ +156,5 @@
> + * @param    {nsIDocShell} docShell - the docShell object to be converted into a FrameDetail JSON object.
> + * @return   {FrameDetail} the FrameDetail JSON object which represents the docShell.
> + */
> +function convertDocShellToFrameDetail(docShell) {
> +  if (!docShell instanceof Ci.nsIDocShell) {

You need parens, otherwise you're checking |true| or |false| against the interface.

Why would this ever not be a docshell?

@@ +176,5 @@
> + * @return {FrameDetail} the FrameDetail JSON object which represents the docShell.
> + */
> +function findFrame(windowId, docShell) {
> +  let docShellsEnum = docShell.getDocShellEnumerator(
> +    Ci.nsIDocShellTreeItem.typeAll,

Let's go with |typeContent|. I don't think we really want extensions to know about chrome docshells.

@@ +182,5 @@
> +  );
> +
> +  while (docShellsEnum.hasMoreElements()) {
> +    let docShell = docShellsEnum.getNext();
> +    let frameDetail = convertDocShellToFrameDetail(docShell);

It would be nice if we didn't have to create a frame detail object for every docshell when we're just trying to match a window ID. How about we just check the window ID before creating a frame detail object for the match?

@@ +202,5 @@
> + * @return   {nsIDocShell[]}        - the array of the frames' docShells collected
> + */
> +function getChildDocShells(docShell) {
> +  let docShellsEnum = docShell.getDocShellEnumerator(
> +    Ci.nsIDocShellTreeItem.typeAll,

|typeContent|

@@ +210,5 @@
> +  let docShells = [];
> +  while (docShellsEnum.hasMoreElements()) {
> +    let docShell = docShellsEnum.getNext();
> +    docShell.QueryInterface(Ci.nsIInterfaceRequestor)
> +      .getInterface(Ci.nsIWebProgress);

There's no need to do this. You're just throwing away the webprogress object.

@@ +211,5 @@
> +  while (docShellsEnum.hasMoreElements()) {
> +    let docShell = docShellsEnum.getNext();
> +    docShell.QueryInterface(Ci.nsIInterfaceRequestor)
> +      .getInterface(Ci.nsIWebProgress);
> +    docShells.push(docShell);

I'd make this a star function, and yield rather than append to an array. You can use |Array.from(getChildDocShells(), convertDocShellToFrameDetail)| below.

Comment 4

[Kris Maglione [:kmag]]

Comment on attachment 8709085 [details] [diff] [review]
0002-Bug-1190685-webext-Add-webNavigation.getFrame-getAll.patch

Review of attachment 8709085 [details] [diff] [review]:
-----------------------------------------------------------------

It would be nice to have a test that didn't rely on |onErrorOccurred|.

::: browser/components/extensions/test/browser/browser_ext_webNavigation_getFrames.js
@@ +9,5 @@
> +    background: "(" + function() {
> +      let results = [];
> +      results.push(new Promise((resolve) => {
> +        browser.webNavigation.getAllFrames({ tabId: 0 }, (res) => {
> +          browser.test.assertEq(res.length, 0, "getAllFrames should return an empty array for non existent tab");

Should it? I'd expect `undefined` with `lastError` set.

Why are you assuming `0` is a nonexistent tab?

@@ +14,5 @@
> +          resolve();
> +        });
> +      }));
> +      results.push(new Promise((resolve) => {
> +        browser.webNavigation.getFrame({ tabId: 0, frameId: 15, processId: 20}, (res) => {

Missing space before the }

@@ +45,5 @@
> +      browser.webNavigation.onErrorOccurred.addListener((errorOccurredDetails) => {
> +        let { tabId } = errorOccurredDetails;
> +        browser.webNavigation.getAllFrames({ tabId }, (getAllFramesDetails) => {
> +          let { frameId } = getAllFramesDetails[0];
> +          browser.webNavigation.getFrame({ tabId, frameId, processId: 0 }, (getFrameDetail) => {

We don't support |processId| yet. I don't think I'd expect |processId: 0| to do the right thing if we did, though.

@@ +73,5 @@
> +  } = yield extension.awaitMessage("getAllFrames.details");
> +
> +  is(errorOccurredDetails.url, NO_CERT_URL, "an onErrorOccurred event should has been raised with the expected URL");
> +
> +  let errorOccurredFrameFound = getAllFramesDetails.filter((frameDetail) => {

|getAllFrameDetails.some(...)|

@@ +108,5 @@
> +        browser.webNavigation.getAllFrames({ tabId }, (getAllFramesDetails) => {
> +          let getFramePromises = getAllFramesDetails.map((frameDetail) => {
> +            let { frameId } = frameDetail;
> +            return new Promise((resolve) => {
> +              browser.webNavigation.getFrame({ tabId, frameId, processId: 0 }, resolve);

Same thing. We don't support |processId|...

@@ +164,5 @@
> +  let errorFrames = getAllFramesDetails.filter((el) => el.errorOccurred);
> +  is(errorFrames.length, 1, "expected number of frames with errors found");
> +  is(errorFrames[0].url, NO_CERT_URL, "expected url found with errors");
> +
> +  is(JSON.stringify(getFrameResults), JSON.stringify(getAllFramesDetails),

|JSON.stringify| isn't guaranteed to stringify things in a consistent way. Please use |Assert.deepEqual| instead.

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Kris Maglione [:kmag] from comment #3)
> Comment on attachment 8709084 [details] [diff] [review]
> 0001-Bug-1190685-webext-Implements-webNavigation.getFrame.patch
> 
> Review of attachment 8709084 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks pretty good.
> 
> I haven't looked over all of WebNavigation.json or WebNavigationContent.js
> yet, because I think it needs to wait for my current patch to land rather
> than re-implement cross-process callback logic in yet another way. Sorry to
> block you on that. I'll get it up for review today.

Absolutely, The "cross-process callback logic" part is the main reason I asked a preliminary feedback instead of a full review :

this kind of "cross-process RPC" is already necessary elsewhere, so it is definitely better if we write it once and reuse it elsewhere in our
WebExtensions internals 

(and in the mean time I wanted to just be sure that I could make the "frames details" data to flow from the content to the main process, like I was planning to)

> @@ +31,5 @@
> >              "name": "details",
> >              "description": "Information about the frame to retrieve information about.",
> >              "properties": {
> >                "tabId": { "type": "integer", "minimum": 0, "description": "The ID of the tab in which the frame is." },
> > +              "processId": {"unsupported": false, "type": "integer", "description": "The ID of the process runs the renderer for this tab."},
> 
> I think this is still unsupported.

Sorry, I forgot to mention in the previous comments the same notes that I put in the weekly meeting notes and in the webNavigation roadmap:

By marking this as "unsupported" we're going to create an annoying behaviour for any cross-browsers add-ons which wants to use the getFrame method:

- on Chromium browsers, |processId| is a mandatory field of the |getFrame| API method,
if it is missing it will raise an exception

- On Firefox, |processId| is marked unsupported and if it exists it will raise an 
  exception

Initially, I proposed to just ignore it or (even fake it, like suggested by Giorgio), but then I found that the Addon-SDK retrieve a real processID from the runtime service [1], so maybe it could be even better if we can use a proper processId, e.g.:
   
    let runtime = Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULRuntime);
    let processId = runtime.processID;

 
> ::: toolkit/modules/addons/WebNavigation.jsm
> @@ +128,5 @@
> > +  },
> > +
> > +  getFrame(tab, context, details, callback) {
> > +    if (!this.initialized) {
> > +      this.init();
> 
> I'd rather avoid initializing all of the change listeners when we're just
> getting details on frames. Let's initialize the frame listeners and the
> change listeners separately.

I like this idea, in the mean time I'm going to look into it.

> @@ +143,5 @@
> > +// NOTE: Compute the Error Page loadType value. (See Bug 1190685 for rationale)
> > +// - LOAD_CMD_NORMAL (as defined in "docshell/base/nsIDocShell.idl"):
> > +// - LOAD_FLAGS_ERROR_PAGE (as defined in "docshell/base/nsDocShellLoadTypes.h")
> > +// - MAKE_LOAD_TYPE (as defined in "docshell/base/nsDocShellLoadTypes.h")
> > +// - LOAD_ERROR_PAGE (as defined in "docshell/base/nsDocShellLoadTypes.h")
> 
> Why aren't you using the constants directly from the interfaces?

It seems that only LOAD_CMD_NORMAL can be currently retrieved from "Ci.nsIDocShell"
(and I'm going to fix it immediately), but the other constants seems to be currently defined and used only in the C++ code.

[1]: https://github.com/mozilla/addon-sdk/blob/5ac44b7e088c986bbbd8aaea57b95738eb071ead/lib/sdk/system/runtime.js#L18

Comment 6

[Kris Maglione [:kmag]]

(In reply to Luca Greco [:rpl] from comment #5)
> Initially, I proposed to just ignore it or (even fake it, like suggested by
> Giorgio), but then I found that the Addon-SDK retrieve a real processID from
> the runtime service [1], so maybe it could be even better if we can use a
> proper processId, e.g.:
>    
>     let runtime =
> Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULRuntime);
>     let processId = runtime.processID;

It's probably not ideal to expose OS process IDs to WebExtensions, but I
suppose it's OK for now. Ideally, we should probably map them to a separate,
internal ID. But that could get complicated.

You can get this more directly from `Services.appinfo.processID`,
incidentally.

> It seems that only LOAD_CMD_NORMAL can be currently retrieved from
> "Ci.nsIDocShell"
> (and I'm going to fix it immediately), but the other constants seems to be
> currently defined and used only in the C++ code.

Hm. We shouldn't be using private constants that way. If you need to use it,
you should add it to an IDL, which would require review by a peer of that
module.

I'm not sure this is the right approach, though. I think the `errorOccurred`
flag should be set in a lot of cases where we're not showing an error page. We
should probably just have a progress listener that stores a flag for the
window in a WeakMap whenever a request starts or stops. It should probably use
the same logic as the onErrorOccurred listener.

Comment 7

[Kris Maglione [:kmag]]

You should be OK to work on this now. Let me know if you have any questions about bug 1210583.

I'd suggest splitting this into multiple bugs:

1) The basic implementation of getFrame/getAllFrames.
2) Adding support for processId.
3) Adding support for errorOccurred.

Comment 8

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: 0001-Bug-1190685-webext-Implements-webNavigation.getFrame.patch

Updated version of the patch which introduces the basic implementation of the webNavigation "getFrame / getAllFrames" methods:

- RPC mechanisms ported to the new MessageChannel (new message names registered and handled by augmenting the existent ExtensionContent.jsm module)

- webNavigation frames helpers and "getFrame / getAllFrames" implementation moved in the new "WebNavigationFrames.jsm" module, imported and used from the "ExtensionContent.jsm" module and the "WebNavigationContent.js" framescript

- ext-webNavigation API methods implementation ported to the new Promise based API

- fix getFrame and getAllFrames results on non existent tabs (parameter is null and runtime.lastError is undefined)

- removed errorOccurred (moved in a followup),

- use typeContent instead of typeAll to iterate in the docShell tree

- define a generators to iterate over the docShell tree and reuse it
  to map all frames and searching for a single frame

- updates to the API schema:
  - getFrame / getAllFrames marked as supported
  - getFrame's processId parameter marked as optional
  - explicitly mark errorOccurred as unsupported

- minor tweaks:
  - aligned . on the previous line
  - removed toString usage

Comment 9

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: 0002-Bug-1190685-webext-Add-webNavigation.getFrame-getAll.patch

Updated version of the test case:

- cleanup (e.g. removed errorOccurred dependency)
- use Assert.deepEqual

Comment 10

[Kris Maglione [:kmag]]

Comment on attachment 8716082 [details] [diff] [review]
0001-Bug-1190685-webext-Implements-webNavigation.getFrame.patch

Review of attachment 8716082 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Thanks.

::: toolkit/components/extensions/ExtensionContent.jsm
@@ +699,5 @@
>  
>    receiveMessage({ target, messageName, recipient, data }) {
>      switch (messageName) {
>        case "Extension:Capture":
> +        return this.handleExtensionCapture(data);

Let's deal with extracting the message data here rather than passing the raw data to the handlers.

  return this.handleExtensionCapture(data.options);

@@ +701,5 @@
>      switch (messageName) {
>        case "Extension:Capture":
> +        return this.handleExtensionCapture(data);
> +      case "Extension:Execute":
> +        return this.handleExtensionExecute(target, recipient, data);

return this.handleExtensionExecute(target, recipient.extensionId, data.options);

@@ +743,4 @@
>  
> +  handleWebNavigationGetFrame(data) {
> +    let deferred = PromiseUtils.defer();
> +    deferred.resolve(WebNavigationFrames.getFrame(this.global.docShell, data));

No need to create a deferred object. Just `return WebNavigationFrames.getFrame(this.global.docShell, data)`

If you need an explicit promise, you can always use `Promise.resolve(...)`

::: toolkit/components/extensions/ext-webNavigation.js
@@ +96,5 @@
> +      getFrame(details) {
> +        let tab = TabManager.getTab(details.tabId);
> +        if (!tab) {
> +          // On non existent tabs, getFrame resolve to null and no runtime.lastError is set.
> +          return Promise.resolve(null);

Let's return a rejected promise for an invalid tabId. I'm not sure why Chrome doesn't, but it's inconsistent with what we do elsewhere.

::: toolkit/modules/addons/WebNavigationFrames.jsm
@@ +71,5 @@
> +    Ci.nsIDocShell.ENUMERATE_FORWARDS
> +  );
> +
> +  while (docShellsEnum.hasMoreElements()) {
> +    let docShell = docShellsEnum.getNext();

No need for the variable. `yield docShellsEnum.getNext();` should do fine.

::: toolkit/modules/moz.build
@@ +1,1 @@
> +

Extra line.

Comment 11

[Kris Maglione [:kmag]]

Comment on attachment 8716083 [details] [diff] [review]
0002-Bug-1190685-webext-Add-webNavigation.getFrame-getAll.patch

Review of attachment 8716083 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/extensions/test/browser/browser_ext_webNavigation_getFrames.js
@@ +9,5 @@
> +      results.push(new Promise((resolve) => {
> +        // There is no "tabId = 0" because the id assigned by TabManager (defined in ext-utils.js)
> +        // starts from 1.
> +        browser.webNavigation.getAllFrames({ tabId: 0 }, (res) => {
> +          browser.test.assertEq(null, res, "getAllFrames should pass a null value for non existent tab");

Let's check for lastError here. And we may as well just use the promise-based variants now rather than creating an explicit promise.

@@ +17,5 @@
> +      results.push(new Promise((resolve) => {
> +        // There is no "tabId = 0" because the id assigned by TabManager (defined in ext-utils.js)
> +        // starts from 1, processId is currently marked as optional and it is ignored.
> +        browser.webNavigation.getFrame({ tabId: 0, frameId: 15, processId: 20 }, (res) => {
> +          browser.test.assertEq(null, res, "getFrame should pass a null value for non existent frame ");

I don't think this is valid, because we're already bailing out for the `tabId == 0`. We should use a valid tab ID.

Also, you have an extra space at the end of your string. (and "nonexistent" is one word).

@@ +130,5 @@
> +  Assert.deepEqual(getFrameResults, getAllFramesDetails, "getFrame and getAllFrames should return the same results");
> +
> +  info(`check frame details collected and retrieved with getAllFrames`);
> +
> +  for (let i = 0; i < collectedDetails.length; i++) {

`for (let [i, collected] of collectedDetails.entries())`

@@ +138,5 @@
> +    is(getAllFramesDetail.frameId, collected.frameId, "frameId");
> +    is(getAllFramesDetail.parentFrameId, collected.parentFrameId, "parentFrameId");
> +    is(getAllFramesDetail.tabId, collected.tabId, "tabId");
> +
> +    // Bug-XXX: moz-extension url are resolved as jar urls in webNavigation events

Hm. Why?

Comment 12

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Kris Maglione [:kmag] from comment #11)
> Comment on attachment 8716083 [details] [diff] [review]
> 0002-Bug-1190685-webext-Add-webNavigation.getFrame-getAll.patch
> 
> Review of attachment 8716083 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> :::
> browser/components/extensions/test/browser/
> browser_ext_webNavigation_getFrames.js
> @@ +138,5 @@
> > +    is(getAllFramesDetail.frameId, collected.frameId, "frameId");
> > +    is(getAllFramesDetail.parentFrameId, collected.parentFrameId, "parentFrameId");
> > +    is(getAllFramesDetail.tabId, collected.tabId, "tabId");
> > +
> > +    // Bug-XXX: moz-extension url are resolved as jar urls in webNavigation events
> 
> Hm. Why?

Filed Bug 1246125 to better describe the issue and its possible fix.

Comment 13

[Kris Maglione [:kmag]]

(In reply to Luca Greco [:rpl] from comment #12)
> Filed Bug 1246125 to better describe the issue and its possible fix.

Ah. Yeah, I guess that makes sense. Thanks.

Comment 14

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: 0001-Bug-1190685-webext-Implements-webNavigation.getFrame.patch

Updated version of the main patch with applied tweaks related to the last review comments:

- cleaned up ExtensionGlobal.receiveMessage (extract message data before calling the handlers, removed remaining deferred objects used in previous versions of this patch)

- returns a rejected promise with a descriptive error messages when the requested tab or frame does not exist

- removed the extra line in moz.build

- minor tweak on WebNavigationFrame.jsm (directly yield the docShellsEnum.next() return value)

Applied the previous r+ flag.

Comment 15

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: 0002-Bug-1190685-webext-Add-webNavigation.getFrame-getAll.patch

Updated version of the patch with the new test file with the following tweaks applied based on the last review comments:

- port the test case to the new Promise-based API
- check the Promise rejects on nonexistent tabs and frames, with the expected error message (non existent frame rejection is tested on an existent tab)

Applied the previous r+ flag.

Comment 16

[Kris Maglione [:kmag]]

Comment on attachment 8717079 [details] [diff] [review]
0002-Bug-1190685-webext-Add-webNavigation.getFrame-getAll.patch

Review of attachment 8717079 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/extensions/test/browser/browser_ext_webNavigation_getFrames.js
@@ +8,5 @@
> +      let results = [
> +        // There is no "tabId = 0" because the id assigned by TabManager (defined in ext-utils.js)
> +        // starts from 1.
> +        browser.webNavigation.getAllFrames({ tabId: 0 }).then(() => {
> +          browser.test.assertTrue(false, "getAllFrames Promise should be rejected on error");

You can use `browser.test.fail` for this.

Comment 17

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: 0002-Bug-1190685-webext-Add-webNavigation.getFrame-getAll.patch

Applied minor tweaks to the test case (using the "browser.test.fail" method to force a failure when needed is definitely better)

Comment 18

[Luca Greco [:rpl] [:luca] [:lgreco]]

Try push results:

- https://treeherder.mozilla.org/#/jobs?repo=try&revision=18b831ed48f5&selectedJob=16496095

Comment 19

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/e941c359f792
https://hg.mozilla.org/integration/fx-team/rev/377d0dc81a91

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e941c359f792
https://hg.mozilla.org/mozilla-central/rev/377d0dc81a91

Comment 21

[Will Bamberg [:wbamberg]]

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/WebNavigation/onHistoryStateUpdated
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/WebNavigation/getAllFrames