1195686 - [meta] Reduce usage of CPOW usage in storage inspector tests

Status: RESOLVED FIXED

(DevTools :: Storage Inspector, defect, P1)

Description

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

> ::: toolkit/devtools/server/tests/browser/browser_storage_updates.js
> @@ +21,5 @@
> > +      info('win.addCookie("c2", "foobar2")');
> > +      win.addCookie("c2", "foobar2");
> > +
> > +      info('win.localStorage.setItem("l1", "foobar1")');
> > +      win.localStorage.setItem("l1", "foobar1");
> 
> There's a lot of direct access to the content window in this TESTS array,
> which means, a lot of CPOWs usage, which we're trying to get rid of (see
> http://mzl.la/1Nn6e6m ).
> I'm ok to do this in a separate bug because this was here before and we
> don't have a frame script setup in the test directory yet. But we should do
> it.
>

Comment 1

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Filter on HOTFROG.

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

See https://reviewboard.mozilla.org/r/227608/diff/#index_header for a preview of tests affected.

Comment 5

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

This is a priority as platform is working on a new localStorage implementation and this means absolutely no CPOWs in Storage Inspector tests... no complaints as I have already begun the process of removing CPOWs.

My first instinct is to try to find some kind of flag we can set to completely disable CPOWs in our tests as this will prevent regressions.

Comment 6

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

The complete list of Storage Inspector tests that reportedly contain CPOWs is:

  - browser_storage_basic_usercontextid_1.js]
  - browser_storage_basic_usercontextid_2.js]
  - browser_storage_basic_with_fragment.js]
  - browser_storage_cache_delete.js]
  - browser_storage_cache_error.js]
  - browser_storage_cookies_add.js]
  - browser_storage_cookies_delete_all.js]
  - browser_storage_cookies_domain.js]
  - browser_storage_cookies_domain_port.js]
  - browser_storage_cookies_edit.js]
  - browser_storage_cookies_edit_keyboard.js]
  - browser_storage_cookies_samesite.js]
  - browser_storage_cookies_tab_navigation.js]
  - browser_storage_delete.js]
  - browser_storage_delete_all.js]
  - browser_storage_delete_tree.js]
  - browser_storage_delete_usercontextid.js]
  - browser_storage_dom_cache_disabled.js]
  - browser_storage_dynamic_updates_cookies.js]
  - browser_storage_dynamic_updates_localStorage.js]
  - browser_storage_dynamic_updates_sessionStorage.js]
  - browser_storage_empty_objectstores.js]
  - browser_storage_indexeddb_delete.js]
  - browser_storage_indexeddb_delete_blocked.js]
  - browser_storage_indexeddb_duplicate_names.js]
  - browser_storage_localstorage_add.js]
  - browser_storage_localstorage_edit.js]
  - browser_storage_localstorage_error.js]
  - browser_storage_localstorage_rapid_add_remove.js]
  - browser_storage_overflow.js]
  - browser_storage_search.js]
  - browser_storage_search_keyboard_trap.js]
  - browser_storage_sessionstorage_add.js]
  - browser_storage_sessionstorage_edit.js]
  - browser_storage_sidebar.js]
  - browser_storage_sidebar_toggle.js]
  - browser_storage_sidebar_update.js]

Comment 7

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

The complete list of Storage Inspector tests that reportedly contain CPOWs is:

  - browser_storage_basic_usercontextid_1.js
  - browser_storage_basic_usercontextid_2.js
  - browser_storage_basic_with_fragment.js
  - browser_storage_cache_delete.js
  - browser_storage_cache_error.js
  - browser_storage_cookies_add.js
  - browser_storage_cookies_delete_all.js
  - browser_storage_cookies_domain.js
  - browser_storage_cookies_domain_port.js
  - browser_storage_cookies_edit.js
  - browser_storage_cookies_edit_keyboard.js
  - browser_storage_cookies_samesite.js
  - browser_storage_cookies_tab_navigation.js
  - browser_storage_delete.js
  - browser_storage_delete_all.js
  - browser_storage_delete_tree.js
  - browser_storage_delete_usercontextid.js
  - browser_storage_dom_cache_disabled.js
  - browser_storage_dynamic_updates_cookies.js
  - browser_storage_dynamic_updates_localStorage.js
  - browser_storage_dynamic_updates_sessionStorage.js
  - browser_storage_empty_objectstores.js
  - browser_storage_indexeddb_delete.js
  - browser_storage_indexeddb_delete_blocked.js
  - browser_storage_indexeddb_duplicate_names.js
  - browser_storage_localstorage_add.js
  - browser_storage_localstorage_edit.js
  - browser_storage_localstorage_error.js
  - browser_storage_localstorage_rapid_add_remove.js
  - browser_storage_overflow.js
  - browser_storage_search.js
  - browser_storage_search_keyboard_trap.js
  - browser_storage_sessionstorage_add.js
  - browser_storage_sessionstorage_edit.js
  - browser_storage_sidebar.js
  - browser_storage_sidebar_toggle.js
  - browser_storage_sidebar_update.js

Comment 8

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

I can at least add browser_storage_updates.js to that list so I want a more foolproof way of detecting tests with CPOWs.

Comment 9

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Also:

  - browser_storage_dynamic_windows.js
  - browser_storage_listings.js
  - browser_storage_updates.js

Comment 10

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe] from comment #5)
> My first instinct is to try to find some kind of flag we can set to
> completely disable CPOWs in our tests as this will prevent regressions.

When talking to :kmag, he suggested altering the CPOW safety check[1] could be useful for at least auditing where they all are.  This could be done for just unsafe CPOWs or all CPOWs.

With the changes in bug 1445551, the `uses-unsafe-cpows` annotation appears to mean that test file is loaded into sandbox with `Cu.permitCPOWsInScope` set.

[1]: https://searchfox.org/mozilla-central/rev/8976abf9cab8eb4661665cc86bd355cd08238011/js/ipc/JavaScriptParent.cpp#90

Comment 11

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

There are no longer any CPOWs in Storage Inspector tests... closing.