Android specific scripts and schemas are loaded too late

RESOLVED FIXED in Firefox 50

Status

defect
P1
normal
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: mattw, Assigned: mattw)

Tracking

(Blocks 1 bug)

unspecified
mozilla50
Unspecified
Android
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

(Whiteboard: android, triaged)

Attachments

(2 attachments, 1 obsolete attachment)

Assignee

Description

3 years ago
On Android, any extensions already installed are loaded before the Android specific scripts and schemas are registered.  They are currently registered in browser.js which is running too late, and AFAIK there isn't an equivalent to nsBrowserGlue.js in Fennec.  

What we can do, however, is switch to using the Category Manager to ensure all of the scripts and schemas are loaded in time.  There will be one category for scripts and another for the schemas that Android and Desktop will use.  Then, instead of calling ExtensionManagement.getSchemas/getScripts, we'll iterate over the respective categories to obtain all of the schemas and scripts we need to load.
Assignee

Comment 2

3 years ago
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62992/diff/1-2/
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

https://reviewboard.mozilla.org/r/62992/#review60162

::: browser/components/extensions/extension-categories.manifest:11
(Diff revision 2)
> +category scripts desktop-runtime chrome://browser/content/ext-desktop-runtime.js
> +category scripts history chrome://browser/content/ext-history.js
> +category scripts pageAction chrome://browser/content/ext-pageAction.js
> +category scripts tabs chrome://browser/content/ext-tabs.js
> +category scripts utils chrome://browser/content/ext-utils.js
> +category scripts windows chrome://browser/content/ext-windows.js

`webextension-scripts`

::: browser/components/extensions/extension-categories.manifest:22
(Diff revision 2)
> +category schemas context_menus chrome://browser/content/schemas/context_menus.json
> +category schemas context_menus_internal chrome://browser/content/schemas/context_menus_internal.json
> +category schemas history chrome://browser/content/schemas/history.json
> +category schemas page_action chrome://browser/content/schemas/page_action.json
> +category schemas tabs chrome://browser/content/schemas/tabs.json
> +category schemas windows chrome://browser/content/schemas/windows.json

`webextension-schemas`

::: browser/components/extensions/moz.build:10
(Diff revision 2)
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  JAR_MANIFESTS += ['jar.mn']
>  
> +EXTRA_COMPONENTS += [
> +    'extension-categories.manifest',

`extensions.manifest`

::: mobile/android/components/extensions/moz.build:10
(Diff revision 2)
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  JAR_MANIFESTS += ['jar.mn']
>  
> +EXTRA_COMPONENTS += [
> +    'extension-categories.manifest',

`extensions.manifest`

::: toolkit/components/extensions/Extension.jsm:119
(Diff revision 2)
> +    let schemas = categoryManager.enumerateCategory(CATEGORY_EXTENSION_SCHEMAS);
>      let promise = Schemas.load(BASE_SCHEMA).then(() => {
>        let promises = [];
> -      for (let schema of ExtensionManagement.getSchemas()) {
> -        promises.push(Schemas.load(schema));
> +      while (schemas.hasMoreElements()) {
> +        let schemaName = schemas.getNext().QueryInterface(Ci.nsISupportsCString).toString();
> +        let schemaURL = categoryManager.getCategoryEntry(CATEGORY_EXTENSION_SCHEMAS, schemaName);

Let's add a helper to XPCOMUtils to iterate over entries in a category. It's a common enough thing.

::: toolkit/components/extensions/Extension.jsm:128
(Diff revision 2)
>        return Promise.all(promises);
>      });
>  
> -    for (let script of ExtensionManagement.getScripts()) {
> +    let scripts = categoryManager.enumerateCategory(CATEGORY_EXTENSION_SCRIPTS);
> +    while (scripts.hasMoreElements()) {
> +      let scriptName = scripts.getNext().QueryInterface(Ci.nsISupportsCString).toString();

s/.toString()/.data/

::: toolkit/components/extensions/moz.build:19
(Diff revision 2)
>      'NativeMessaging.jsm',
>      'Schemas.jsm',
>  ]
>  
> +EXTRA_COMPONENTS += [
> +    'extension-categories.manifest',

`extensions.manifest`
Attachment #8769011 - Flags: review?(kmaglione+bmo)
Assignee

Comment 4

3 years ago
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62992/diff/2-3/
Attachment #8769011 - Flags: review?(kmaglione+bmo)
Assignee

Comment 5

3 years ago
https://reviewboard.mozilla.org/r/62992/#review60162

> Let's add a helper to XPCOMUtils to iterate over entries in a category. It's a common enough thing.

Sounds good to me. I noticed `XPCOMUtils` has a convenience getter for fetching the category manager and also has a helper for iterating over `nsISimpleEnumerator`. I used both of those to create the helper method.  Let me know what you think.

> `extensions.manifest`

The reason I didn't use `extensions.manifest` is because there is a name conflict with `toolkit/mozapps/extensions/extensions.manifest`.  I get the following error when building: `ValueError: Item already in manifest: components/extensions.manifest`.  Do you know of a way to fix this? For now I renamed them all to `webextensions.manifest`.

Updated

3 years ago
Priority: -- → P1
Whiteboard: android, triaged

Comment 6

3 years ago
Matt, is this something that needs to be uplifted? Not sure the impact of this issue.
Assignee: nobody → mwein
The feature this affects landed in Firefox 50, so it doesn't need to be uplifted.
Assignee

Comment 8

3 years ago
https://reviewboard.mozilla.org/r/62992/#review60162

> The reason I didn't use `extensions.manifest` is because there is a name conflict with `toolkit/mozapps/extensions/extensions.manifest`.  I get the following error when building: `ValueError: Item already in manifest: components/extensions.manifest`.  Do you know of a way to fix this? For now I renamed them all to `webextensions.manifest`.

This might be a different issue, because I seem to be getting this conflict now when I try to build for android using webextensions.manifest.  It builds fine for desktop.  I'm looking into the issue now.
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

https://reviewboard.mozilla.org/r/62992/#review60412

Looks good so for the most part. Mainly just nits at this point, and missing tests.

::: js/xpconnect/loader/XPCOMUtils.jsm:356
(Diff revision 3)
>    /**
>     * Helper which iterates over a nsISimpleEnumerator.
>     * @param e The nsISimpleEnumerator to iterate over.
>     * @param i The expected interface for each element.
>     */
>    IterSimpleEnumerator: function XPCU_IterSimpleEnumerator(e, i)

Can we make these star functions while we're here?

::: js/xpconnect/loader/XPCOMUtils.jsm:375
(Diff revision 3)
>        yield e.getNext();
>    },
>  
>    /**
> +   * Helper which iterates over the entries in a category.
> +   * @param aCategory The category over which to iterate.

s/category/name of the &/

::: js/xpconnect/loader/XPCOMUtils.jsm:377
(Diff revision 3)
>  
>    /**
> +   * Helper which iterates over the entries in a category.
> +   * @param aCategory The category over which to iterate.
> +   */
> +  IterCategory: function XPCU_IterCategory(aCategory)

This needs to be a star function. And it should probably be named something like `enumerateCategoryEntries`.

Also, this could use a test. And we should move the XPCOMUtils changes to a separate commit.

::: js/xpconnect/loader/XPCOMUtils.jsm:379
(Diff revision 3)
> +   * Helper which iterates over the entries in a category.
> +   * @param aCategory The category over which to iterate.
> +   */
> +  IterCategory: function XPCU_IterCategory(aCategory)
> +  {
> +    let categoryManager = this.categoryManager;

I don't think there's really any need for this variable.

::: js/xpconnect/loader/XPCOMUtils.jsm:381
(Diff revision 3)
> +   */
> +  IterCategory: function XPCU_IterCategory(aCategory)
> +  {
> +    let categoryManager = this.categoryManager;
> +    let category = categoryManager.enumerateCategory(aCategory);
> +    for (let entry in this.IterSimpleEnumerator(category, Ci.nsISupportsCString)) {

s/in/of/

::: js/xpconnect/loader/XPCOMUtils.jsm:382
(Diff revision 3)
> +  IterCategory: function XPCU_IterCategory(aCategory)
> +  {
> +    let categoryManager = this.categoryManager;
> +    let category = categoryManager.enumerateCategory(aCategory);
> +    for (let entry in this.IterSimpleEnumerator(category, Ci.nsISupportsCString)) {
> +      yield {

Let's just yield a two-item array.

::: toolkit/components/extensions/Extension.jsm:113
(Diff revision 3)
>  
>      // Load order matters here. The base manifest defines types which are
>      // extended by other schemas, so needs to be loaded first.
>      let promise = Schemas.load(BASE_SCHEMA).then(() => {
>        let promises = [];
> -      for (let schema of ExtensionManagement.getSchemas()) {
> +      for (let entry in XPCOMUtils.IterCategory(CATEGORY_EXTENSION_SCHEMAS)) {

s/in/of/
Attachment #8769011 - Flags: review?(kmaglione+bmo)
Assignee

Comment 10

3 years ago
Review commit: https://reviewboard.mozilla.org/r/63762/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/63762/
Attachment #8769011 - Attachment description: Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec. → Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.
Attachment #8770272 - Flags: review?(kmaglione+bmo)
Attachment #8769011 - Flags: review?(kmaglione+bmo)
Assignee

Comment 11

3 years ago
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62992/diff/3-4/
Assignee

Comment 12

3 years ago
https://reviewboard.mozilla.org/r/62992/#review60412

> I don't think there's really any need for this variable.

The reason I did this is because it is accessed using a getter which fetches the service:
  get categoryManager() {
    return Components.classes["@mozilla.org/categorymanager;1"]
           .getService(Ci.nsICategoryManager);
  },
Is that okay?
https://reviewboard.mozilla.org/r/62992/#review60412

> The reason I did this is because it is accessed using a getter which fetches the service:
>   get categoryManager() {
>     return Components.classes["@mozilla.org/categorymanager;1"]
>            .getService(Ci.nsICategoryManager);
>   },
> Is that okay?

We should actually probably replace this getter with a `defineLazyServiceGetter` call after the XPCOMUtils object has been created.
Attachment #8770272 - Flags: review?(kmaglione+bmo) → review+
Comment on attachment 8770272 [details]
Part 2 - Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec.

https://reviewboard.mozilla.org/r/63762/#review60796

::: js/xpconnect/tests/unit/test_xpcomutils.js:206
(Diff revision 1)
>    print("Check there are no more or less than expected entries.");
>    do_check_eq(foundEntriesCount, EXPECTED_ENTRIES.length);
>  
>    run_next_test();
> +
>  });

Please remove the whitespace changes to this file from this patch.
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

https://reviewboard.mozilla.org/r/62992/#review60798

::: js/xpconnect/tests/unit/test_xpcomutils.js:199
(Diff revision 4)
> -               getService(Ci.nsICategoryManager);
> -  let entries = catMan.enumerateCategory(CATEGORY_NAME);
> -  while (entries.hasMoreElements()) {
>      foundEntriesCount++;
> -    let entry = entries.getNext().QueryInterface(Ci.nsISupportsCString).data;
> -    print("Check the found category entry (" + entry + ") is expected.");  
> +    print(`Check the found category entry (${name}) is expected.`);
> +    do_check_true(EXPECTED_ENTRIES.indexOf(name) != -1);

`EXPECTED_ENTRIES.includes(name)`.

But let's make `EXPECTED_ENTRIES` a map instead so that we can also check the values.
Attachment #8769011 - Flags: review?(kmaglione+bmo)
https://reviewboard.mozilla.org/r/62992/#review60798

> `EXPECTED_ENTRIES.includes(name)`.
> 
> But let's make `EXPECTED_ENTRIES` a map instead so that we can also check the values.

Also, to be honest, this test is a bit dodgy. Let's delete the entry from the map at the end of the loop, and then just check that the expected entries map is empty at the end, rather than checking the count. That way duplicate entries won't unexpectedly cause the test to pass.
Assignee

Comment 17

3 years ago
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62992/diff/4-5/
Attachment #8769011 - Flags: review?(kmaglione+bmo)
Assignee

Comment 18

3 years ago
Comment on attachment 8770272 [details]
Part 2 - Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63762/diff/1-2/
Assignee

Comment 19

3 years ago
https://reviewboard.mozilla.org/r/62992/#review60412

> We should actually probably replace this getter with a `defineLazyServiceGetter` call after the XPCOMUtils object has been created.

Sgtm.
Assignee

Comment 20

3 years ago
https://reviewboard.mozilla.org/r/62992/#review60798

> Also, to be honest, this test is a bit dodgy. Let's delete the entry from the map at the end of the loop, and then just check that the expected entries map is empty at the end, rather than checking the count. That way duplicate entries won't unexpectedly cause the test to pass.

Good call, that sgtm.
Attachment #8769011 - Flags: review?(kmaglione+bmo)
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

https://reviewboard.mozilla.org/r/62992/#review60820

::: js/xpconnect/loader/XPCOMUtils.jsm:451
(Diff revisions 4 - 5)
>  XPCOMUtils.defineLazyModuleGetter(this, "Preferences",
>                                    "resource://gre/modules/Preferences.jsm");
>  XPCOMUtils.defineLazyModuleGetter(this, "Services",
>                                    "resource://gre/modules/Services.jsm");
>  
> +XPCOMUtils.defineLazyServiceGetter(this,

No, `XPCOMUtils.defineLazyServiceGetter(XPCOMUtils, "categoryManager", ...)`

::: js/xpconnect/tests/unit/test_xpcomutils.js:191
(Diff revisions 4 - 5)
>    );
>  
>    // Load test components.
>    do_load_manifest("CatRegistrationComponents.manifest");
>  
> -  const EXPECTED_ENTRIES = ["CatAppRegisteredComponent",
> +  const EXPECTED_ENTRIES = new Map();

... = new Map([
      ["CatRegistered...", "..."],
    ]);

::: js/xpconnect/tests/unit/test_xpcomutils.js:198
(Diff revisions 4 - 5)
>  
> -  // Check who is registered in "test-cat" category.
> +  // Verify the correct entries are registered in the "test-cat" category.
> -  let foundEntriesCount = 0;
>    for (let [name, value] of XPCOMUtils.enumerateCategoryEntries(CATEGORY_NAME)) {
> -    foundEntriesCount++;
> -    print(`Check the found category entry (${name}) is expected.`);
> +    print("Verify that the name/value pair exists in the expected entries.");
> +    do_check_eq(EXPECTED_ENTRIES.get(name), value);

Also `ok(EXPECTED_ENTRIES.has(name))`

::: js/xpconnect/tests/unit/test_xpcomutils.js:202
(Diff revisions 4 - 5)
> -    foundEntriesCount++;
> -    print(`Check the found category entry (${name}) is expected.`);
> -    do_check_true(EXPECTED_ENTRIES.indexOf(name) != -1);
> +    print("Verify that the name/value pair exists in the expected entries.");
> +    do_check_eq(EXPECTED_ENTRIES.get(name), value);
> +    EXPECTED_ENTRIES.delete(name);
>    }
> -
> -  print("Check there are no more or less than expected entries.");
> +  print("Check that all of the expected entries have been deleted.");
> +  do_check_eq(0, EXPECTED_ENTRIES.size);

Reverse the order of the arguments, please.
Assignee

Comment 22

3 years ago
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62992/diff/5-6/
Attachment #8769011 - Flags: review?(kmaglione+bmo)
Assignee

Comment 23

3 years ago
Comment on attachment 8770272 [details]
Part 2 - Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63762/diff/2-3/
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

https://reviewboard.mozilla.org/r/62992/#review60830

::: js/xpconnect/loader/XPCOMUtils.jsm:451
(Diff revisions 5 - 6)
>  XPCOMUtils.defineLazyModuleGetter(this, "Preferences",
>                                    "resource://gre/modules/Preferences.jsm");
>  XPCOMUtils.defineLazyModuleGetter(this, "Services",
>                                    "resource://gre/modules/Services.jsm");
>  
> -XPCOMUtils.defineLazyServiceGetter(this,
> +XPCOMUtils.defineLazyServiceGetter(this, "categoryManager",

No, not `this`, `XPCOMUtils`

::: js/xpconnect/tests/unit/test_xpcomutils.js:193
(Diff revisions 5 - 6)
>    // Load test components.
>    do_load_manifest("CatRegistrationComponents.manifest");
>  
> -  const EXPECTED_ENTRIES = new Map();
> -  EXPECTED_ENTRIES.set("CatRegisteredComponent", "@unit.test.com/cat-registered-component;1");
> -  EXPECTED_ENTRIES.set("CatAppRegisteredComponent", "@unit.test.com/cat-app-registered-component;1");
> +  const EXPECTED_ENTRIES = new Map([
> +    ["CatRegisteredComponent", "@unit.test.com/cat-registered-component;1"],
> +    ["CatAppRegisteredComponent", "@unit.test.com/cat-app-registered-component;1"]

Please add trailing comma.
Attachment #8769011 - Flags: review?(kmaglione+bmo)
Assignee

Comment 25

3 years ago
https://reviewboard.mozilla.org/r/62992/#review60830

> No, not `this`, `XPCOMUtils`

Could you explain why? Is there something special about Preferences and Services to have them defined in `this` while the categoryManager should be defined in the `XPCOMUTils` object?
Assignee

Comment 26

3 years ago
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62992/diff/6-7/
Attachment #8769011 - Flags: review?(kmaglione+bmo)
Assignee

Comment 27

3 years ago
Comment on attachment 8770272 [details]
Part 2 - Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63762/diff/3-4/
https://reviewboard.mozilla.org/r/62992/#review60830

> Could you explain why? Is there something special about Preferences and Services to have them defined in `this` while the categoryManager should be defined in the `XPCOMUTils` object?

It's part of the public API. It needs to be exported via that object.
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

https://reviewboard.mozilla.org/r/62992/#review60864

::: js/xpconnect/loader/XPCOMUtils.jsm:371
(Diff revisions 6 - 7)
>     * Helper which iterates over the entries in a category.
>     * @param aCategory The name of the category over which to iterate.
>     */
>    enumerateCategoryEntries: function* XPCOMUtils_enumerateCategoryEntries(aCategory)
>    {
> -    let category = categoryManager.enumerateCategory(aCategory);
> +    let category = XPCOMUtils.categoryManager.enumerateCategory(aCategory);

s/XPCOMUtils/this/
Attachment #8769011 - Flags: review?(kmaglione+bmo) → review+
Assignee

Comment 30

3 years ago
https://reviewboard.mozilla.org/r/62992/#review60830

> It's part of the public API. It needs to be exported via that object.

Gotcha, thanks!
Assignee

Comment 32

3 years ago
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62992/diff/7-8/
Assignee

Comment 33

3 years ago
Comment on attachment 8770272 [details]
Part 2 - Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63762/diff/4-5/
Assignee

Updated

3 years ago
Keywords: checkin-needed

Comment 34

3 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/62d6a314557c
Part 1 - Add a helper to XPCOMUtils to iterate over entries in a category. r=kmag
https://hg.mozilla.org/integration/fx-team/rev/576835bcf8e3
Part 2 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec. r=kmag
Keywords: checkin-needed
sorry had to back this out since i guess this caused https://treeherder.mozilla.org/logviewer.html#?job_id=10538106&repo=fx-team
Flags: needinfo?(mwein)

Comment 36

3 years ago
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/04a72d8ca780
Backed out changeset 576835bcf8e3 
https://hg.mozilla.org/integration/fx-team/rev/1d5210d6f961
Backed out changeset 62d6a314557c for test failures in browser_dbg_sources-webext-contentscript.js
Assignee

Comment 39

3 years ago
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62992/diff/8-9/
Assignee

Comment 40

3 years ago
Comment on attachment 8770272 [details]
Part 2 - Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63762/diff/5-6/
Assignee

Comment 41

3 years ago
Carsten, I think this just needs a clobber in order to work.  Could you please r+ the CLOBBER change and then try landing it again?
Flags: needinfo?(mwein)
Keywords: checkin-needed
Assignee

Updated

3 years ago
Flags: needinfo?(cbook)
Assignee

Comment 42

3 years ago
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62992/diff/9-10/
Assignee

Comment 43

3 years ago
Comment on attachment 8770272 [details]
Part 2 - Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63762/diff/6-7/
Assignee

Updated

3 years ago
Attachment #8772188 - Attachment is obsolete: true
Assignee

Comment 44

3 years ago
Moved the CLOBBER change into the commit that modifies that manifest files as per kmag's recommendation.
Flags: needinfo?(cbook)
Assignee

Comment 45

3 years ago
s/that/the

Comment 46

3 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/2ba4e664fa4e
Part 1 - Add a helper to XPCOMUtils to iterate over entries in a category. r=kmag
https://hg.mozilla.org/integration/fx-team/rev/d950882eb6ce
Part 2 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec. r=kmag
Keywords: checkin-needed

Comment 48

3 years ago
Backout by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/6850035575f7
Backed out changeset 2ba4e664fa4e for xpcshell and M(10) failures
https://hg.mozilla.org/integration/fx-team/rev/c0f1a96173fc
Backed out changeset d950882eb6ce
Assignee

Comment 49

3 years ago
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62992/diff/10-11/
Assignee

Comment 50

3 years ago
Comment on attachment 8770272 [details]
Part 2 - Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63762/diff/7-8/
Assignee

Comment 51

3 years ago
It looks like another call to ExtensionManagement.registerScript is being made in browser/components/extensions/test/xpcshell/head.js that I failed to remove.  I searched the source code and confirmed that this is the only other instance left that I need to remove.
Flags: needinfo?(mwein)
Assignee

Comment 56

3 years ago
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62992/diff/11-12/
Assignee

Comment 57

3 years ago
Comment on attachment 8770272 [details]
Part 2 - Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63762/diff/8-9/
Comment on attachment 8770272 [details]
Part 2 - Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec.

https://reviewboard.mozilla.org/r/63762/#review63864

::: browser/components/extensions/ext-history.js:24
(Diff revision 9)
>  const {
>    normalizeTime,
>    SingletonEventManager,
>  } = ExtensionUtils;
>  
> -const History = PlacesUtils.history;
> +let historyService = Ci.nsINavHistoryService;

This name is inaccurate. Please call this something like `nsINavHistoryService`.
Attachment #8770272 - Flags: review+
Assignee

Comment 60

3 years ago
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62992/diff/12-13/
Attachment #8770272 - Flags: review?(kmaglione+bmo)
Assignee

Comment 61

3 years ago
Comment on attachment 8770272 [details]
Part 2 - Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63762/diff/9-10/
Assignee

Comment 63

3 years ago
Comment on attachment 8770272 [details]
Part 2 - Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63762/diff/10-11/
Comment on attachment 8770272 [details]
Part 2 - Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec.

https://reviewboard.mozilla.org/r/63762/#review64302

::: toolkit/components/extensions/Schemas.jsm:1411
(Diff revision 11)
>        return new IntegerType(type, type.minimum || -Infinity, type.maximum || Infinity);
>      } else if (type.type == "boolean") {
>        checkTypeProperties();
>        return new BooleanType(type);
>      } else if (type.type == "function") {
> -      let isAsync = Boolean(type.async);
> +      let isAsync = type.async && Boolean(type.async);

`!!type.async`
Attachment #8770272 - Flags: review?(kmaglione+bmo) → review+
Assignee

Comment 66

3 years ago
Comment on attachment 8769011 [details]
Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62992/diff/13-14/
Assignee

Comment 67

3 years ago
Comment on attachment 8770272 [details]
Part 2 - Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63762/diff/11-12/
Assignee

Updated

3 years ago
Keywords: checkin-needed

Comment 68

3 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3a7713a6735
Part 1: Add a helper to XPCOMUtils to iterate over entries in a category. r=kmag
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9ca8dc4b42e
Part 2: Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec. r=kmag
Keywords: checkin-needed

Comment 69

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e3a7713a6735
https://hg.mozilla.org/mozilla-central/rev/e9ca8dc4b42e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50

Updated

Last year
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.