Closed Bug 1285063 Opened 8 years ago Closed 8 years ago

Android specific scripts and schemas are loaded too late

Categories

(WebExtensions :: Untriaged, defect, P1)

Product:
WebExtensions
Component:
Untriaged
Platform:
Unspecified
Android
Type:
defect

Tracking

(firefox50 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla50
Iteration:
50.3 - Jul 18
Tracking Flags:
Tracking Status
firefox50 --- fixed

People

(Reporter: mattw, Assigned: mattw)

References

Details

(Whiteboard: android, triaged)

Attachments

(2 files, 1 obsolete file)

Part 1 - Bug 1285063 - Add a helper to XPCOMUtils to iterate over entries in a category.
58 bytes, text/x-review-board-request
kmag
: review+
Details
Part 2 - Bug 1285063 - Switch to using the Category Manager to register scripts/schemas in order to load them in time for Fennec.
58 bytes, text/x-review-board-request
kmag
: review+
Details 
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.
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)
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)
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`.
Priority: -- → P1
Whiteboard: android, triaged
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.
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)
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)
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/
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.
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)
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/
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.
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.
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)
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)
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?
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)
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+
https://reviewboard.mozilla.org/r/62992/#review60830

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

Gotcha, thanks!
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/
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/
Keywords: checkin-needed
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)
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
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/
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/
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
Flags: needinfo?(cbook)
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/
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/
Attachment #8772188 - Attachment is obsolete: true
Moved the CLOBBER change into the commit that modifies that manifest files as per kmag's recommendation.
Flags: needinfo?(cbook)
s/that/the
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
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
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/
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/
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)
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/
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+
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)
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/
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+
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/
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/
Keywords: checkin-needed
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
https://hg.mozilla.org/mozilla-central/rev/e3a7713a6735
https://hg.mozilla.org/mozilla-central/rev/e9ca8dc4b42e
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: