Closed Bug 1166405 Opened 6 years ago Closed 6 years ago

Consolidate classes into a general web manifest module

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: marcosc, Assigned: marcosc)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

Currently, the manifest stuff is abusing .jsm's to create classes. However, the right thing to do would be to have a single .jsm that gives consumers access to the obtainer and the processor classes. That would mean there would only be a WebManifest.jsm from which classes can be accessed.
Blocks: webmanifest
Summary: Consolidate classes into a general manifest module → Consolidate classes into a general web manifest module
Assignee: nobody → mcaceres
Ehsan, basically, I just renamed the files, removed a little bit of redundancy, and now just load the JS files into a singe JSM. This provides a tidy centralized place to work with web manifests.  

Happy to walk through it.
Attachment #8608410 - Flags: review?(ehsan)
Attached patch Cleaned up a bit more stuff... (obsolete) — Splinter Review
Attachment #8608410 - Attachment is obsolete: true
Attachment #8608410 - Flags: review?(ehsan)
Attachment #8608900 - Flags: review?(ehsan)
Attachment #8608900 - Flags: review?(ehsan) → review+
Attachment #8608900 - Attachment is obsolete: true
Keywords: checkin-needed
Well, this kind of sucks. Web manifest tests run on b2g opt in mochitest-6, which had been hidden for permafailure until just this morning, so last week you would have had to know to look at https://treeherder.mozilla.org/#/jobs?repo=try&revision=1595e5b7fef4&exclusion_profile=false by clicking the totally inexplicable empty rectangle to the left of treeherder's "Filter platforms & jobs" input to know that try was trying to tell you that your patch was going to cause a raft of test bustage on b2g emulator.

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/70768c4ba857
(In reply to Phil Ringnalda (:philor) from comment #7)
> Well, this kind of sucks. Web manifest tests run on b2g opt in mochitest-6,
> which had been hidden for permafailure until just this morning, so last week
> you would have had to know to look at
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=1595e5b7fef4&exclusion_profile=false by clicking the
> totally inexplicable empty rectangle to the left of treeherder's "Filter
> platforms & jobs" input to know that try was trying to tell you that your
> patch was going to cause a raft of test bustage on b2g emulator.
> 
> Backed out in
> https://hg.mozilla.org/integration/mozilla-inbound/rev/70768c4ba857

I'm confused. I've checked the tests an I can't see any related to my code? They all seem to be reftest timeouts, etc. unrelated to my stuff. Will send to try again.
Flags: needinfo?(philringnalda)
NM, I finally found them. Fixing.
Flags: needinfo?(philringnalda)
The only things that changed in this patch were:

s/const EXPORTED_SYMBOLS/this.EXPORTED_SYMBOLS/

In WebManifest.jsm. 

And browser_ManifestObtainer_obtain.js now only does 100 iterations instead of 1000 when flooding IPC.
https://hg.mozilla.org/mozilla-central/rev/f3d11d05b40b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.