Closed Bug 1563555 Opened 9 months ago Closed 8 months ago

Improve ergonomics to use Gecko prefs from rust

Categories

(Core :: Preferences: Backend, task)

task
Not set

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox69 --- wontfix
firefox70 --- fixed

People

(Reporter: padenot, Assigned: njn)

References

(Blocks 1 open bug)

Details

Attachments

(5 files, 2 obsolete files)

Emilio tells me that something hacky exists, but only for StaticPrefs, used in Servo, but it would be nice to have something a bit better and safe to use Gecko prefs from rust code.

It's a bit annoying that this is using a class with mostly static members, and I think only static members when it comes to the external interface that Gecko developers doing rust want to use.

Also this uses quite a lot of overloading and the names are going to be mangled anyways, so having something (bindgen) do it automatically is probably a lost cause.

We should maybe just do a crate with simple functions that expose the simple Get* and Set* API (not the *Complex API), that would cover most usage of this from rust code. Callbacks would be good to have but I'm afraid that it's going to be a bit hard to bind, it's a bit template heavy.

Emilio, thoughts? I can have a go at it (I need it for something), but feel free to steal.

Flags: needinfo?(emilio)

Paul, can you give a specific example of something that you want to do that you currently cannot? libpref is (unfortunately) sufficient complex that "use Gecko prefs from Rust" could mean a variety of things.

Flags: needinfo?(padenot)

Get the value of a pref from a rust crate, at run time, in a content process however quite early (just before the sandbox lockdown).

Flags: needinfo?(padenot)

Which pref or prefs?

Flags: needinfo?(padenot)

The kind of thing I was thinking of was something like:

  • Moving StaticPrefList.py to Python / toml / json / something.
  • Use that to generate both StaticPrefList.h, and a pref! macro from rust much like the atom! macro works today, potentially just calling into C++ code.

The way the macro could work could just be having an FFI function like:

extern "C" Type StaticPrefs_foo_bar() {
  return StaticPrefs::foo_bar();
}

And in rust an auto-generated macro like:

macro_rules! pref {
    ("foo.bar") => {{ unsafe { StaticPrefs_foo_bar() }},
   // ...
}

So the rust usage would look like if something && pref!("foo.bar") {...

If StaticPrefs allow for Paul's use-case it'd be nice, since it'd clean up pref usage from the style system code as well.

Flags: needinfo?(emilio)

So the rust usage would look like if something && pref!("foo.bar").

As opposed to the current if something && sVarCache_foo_bar, right? I can see it's nicer to write, and that's worth having, but I don't think it provides new functionality. Or am I missing something? I don't understand the complaint about static members in comment 1.

Blocks: rust-great

(In reply to Nicholas Nethercote [:njn] from comment #6)

As opposed to the current if something && sVarCache_foo_bar, right? I can see it's nicer to write, and that's worth having, but I don't think it provides new functionality. Or am I missing something? I don't understand the complaint about static members in comment 1.

You can only use the sVarCache stuff from style system code, which is the one that runs bindgen atm. So most rust code in m-c doesn't have that capability.

(In reply to Nicholas Nethercote [:njn] from comment #4)

Which pref or prefs?

security.sandbox.content.level, for now.

My complaint about static methods was that it means we'll have to wrap all of them in functions with a C mangling, instead of wrapping them directly.

Flags: needinfo?(padenot)
Depends on: 1564724
Type: enhancement → task

The kind of thing I was thinking of was something like:

  • Moving StaticPrefList.py to Python / toml / json / something.
  • Use that to generate both StaticPrefList.h,

This is now done. modules/libpref/init/StaticPrefList.yaml is the definitions file, and modules/libpref/init/generate_static_pref_list.py is the generator script.

and a pref! macro from rust much like the atom! macro works today, potentially just calling into C++ code.

The way the macro could work could just be having an FFI function like:

extern "C" Type StaticPrefs_foo_bar() {
  return StaticPrefs::foo_bar();
}

All those functions would be in one generated .cpp file somewhere, right? Would there need to be a .h file with the declarations as well?

And in rust an auto-generated macro like:

macro_rules! pref {
    ("foo.bar") => {{ unsafe { StaticPrefs_foo_bar() }},
   // ...
}

Likewise, would all these definitions be in a single .rs file? Would that .rs file be in its own crate?

We're close to having this, but the structure of the generated code is still a bit unclear to me, hence my questions.

Flags: needinfo?(emilio)

My complaint about static methods was that it means we'll have to wrap all of them in functions with a C mangling, instead of wrapping them directly.

Sorry, I still don't understand this fully. Can you explain more what you mean by "wrapping them directly?"

Here's how I'm imagining all this will work, based on what Emilio wrote above.

  • If you want to access a pref from Rust, you'll have to put it into StaticPrefList.yaml.
  • That will cause a C FFI function to be generated, along with a Rust macro that calls the function.
  • You can then call the macro from your Rust code.

(In reply to Nicholas Nethercote [:njn] from comment #9)

The kind of thing I was thinking of was something like:

  • Moving StaticPrefList.py to Python / toml / json / something.
  • Use that to generate both StaticPrefList.h,

This is now done. modules/libpref/init/StaticPrefList.yaml is the definitions file, and modules/libpref/init/generate_static_pref_list.py is the generator script.

That's great, thanks! \o/

and a pref! macro from rust much like the atom! macro works today, potentially just calling into C++ code.

The way the macro could work could just be having an FFI function like:

extern "C" Type StaticPrefs_foo_bar() {
  return StaticPrefs::foo_bar();
}

All those functions would be in one generated .cpp file somewhere, right? Would there need to be a .h file with the declarations as well?

Not necessarily. You can have it, but I don't think anything needs to use the .h file, so as long as the rust code has all the declarations, and the cpp code all the definitions it should be fine.

And in rust an auto-generated macro like:

macro_rules! pref {
    ("foo.bar") => {{ unsafe { StaticPrefs_foo_bar() }},
   // ...
}

Likewise, would all these definitions be in a single .rs file? Would that .rs file be in its own crate?

Probably? And probably too, so that the rest of Gecko and not only style can read them. That has the issue of touching StaticPrefList.yaml causing all the rust dependencies of this crate to get rebuilt. You could do something like what you've done in bug 1563139 but with crates, but it seems a bit more overkill, I don't know.

Flags: needinfo?(emilio)

I have a draft patch working. The generated C++ code looks like this:

extern "C" int32_t StaticPrefs_dom_min_timeout_value() {
  return StaticPrefs::dom_min_timeout_value();
}            
             
extern "C" bool StaticPrefs_layout_css_font_display_enabled() {
  return StaticPrefs::layout_css_font_display_enabled();
}

The generated Rust code looks like this:

extern "C" {
    pub fn StaticPrefs_dom_min_timeout_value() -> i32;
    pub fn StaticPrefs_layout_css_font_display_enabled() -> bool;
}

#[macro_export]
macro_rules! static_pref {
    ("dom.min_timeout_value") => {{
      unsafe { StaticPrefs_dom_min_timeout_value() }
    }};
    ("layout.css.font-display.enabled") => {{
      unsafe { StaticPrefs_layout_css_font_display_enabled() }
    }};
}

Rust code that uses the prefs looks like this:

use static_prefs::*;
let pref = static_pref!("layout.css.font-display.enabled");

All the generated C++ code is in a single file. That file will get recompiled any time StaticPrefList.yaml changes, but it's only one .cpp file so that's not bad. I guess it could be split up into pieces to slightly reduce the amount of recompilation that occurs.

All the generated Rust code will also go into a single file. Splitting it into pieces seems much harder, and hopefully incremental compilation will handle things nicely to minimize recompilation costs there anyway...

Emilio: Does this look reasonable to you? Any improvements you can think of?

That all looks pretty great to me. I agree that splitting the rust code is probably not worth it. Same for the cpp file.

Are the StaticPrefs::foo_bar_enabled() calls inlined out of curiosity? If not, it may be worth moving the generated cpp file to the same translation unit as where they're defined... Though maybe we're doing enough LTO and co that it doesn't matter at all in practice.

The use static_prefs::* is a bit unfortunate, I think we can avoid it easily by using $crate in the generated macro. That'd only require to import the macro itself.

I wouldn't bother with the static_ prefix in the macro name, I think we should encourage the usage of it and make it shorter. Maybe just pref!? that'd also match the name Servo uses for their macros, which would save a bunch of conditional compilation in some cases. But if you feel strongly about that then this is fine of course.

Are the StaticPrefs::foo_bar_enabled() calls inlined out of curiosity? If not, it may be worth moving the generated cpp file to the same translation unit as where they're defined... Though maybe we're doing enough LTO and co that it doesn't matter at all in practice.

The C++ StaticPrefs::foo_bar_enabled() functions are defined as inline in StaticPrefListBegin.h, and the generated files containing the C StaticPrefs_foo_bar_enabled() function include that header file. So the C++ getter should be inlined into the C function. (I think that's what you're asking.)

The use static_prefs::* is a bit unfortunate, I think we can avoid it easily by using $crate in the generated macro. That'd only require to import the macro itself.

Good suggestion, it does work.

I wouldn't bother with the static_ prefix in the macro name, I think we should encourage the usage of it and make it shorter. Maybe just pref!? that'd also match the name Servo uses for their macros, which would save a bunch of conditional compilation in some cases. But if you feel strongly about that then this is fine of course.

I have ended up using static_prefs::pref!("foo.bar.baz") in most places. What do you think about that? Having said that, it seems like all this code is marked with #[cfg(feature = "gecko")], so I don't understand why the Servo macros matter.

generate_static_pref_list currently has two functions:

  • generate_header(): this checks the YAML and generates most of the code.
  • emit_header(): this does a bit of additional code generation and writes the
    code to file.

This patch gives it a cleaner structure:

  • check_pref_list(): this checks the YAML.
  • generate_code(): this generates all the code. (It calls check_pref_list()
    first.)
  • emit_code(): this just writes the code to file.

The patch also improves the testing in test_generate_static_pref_list.py,
so that it checks the full contents of all generated files.

All these improvements will help with the next patch, which adds two additional
generated files.

Attachment #9082529 - Attachment is obsolete: true

This patch introduces a new Rust crate called static_prefs.

It also changes generate_static_pref_list.py to generate two new files.

  • StaticPrefsCGetters.cpp: contains C getters, which are just wrappers around
    the C++ getters. This is included into Preferences.cpp.

  • static_prefs.rs: contains declarations for the C getters, plus the pref!
    macro which provides nice syntax for calling the C getters. This is included
    into static_prefs/src/lib.rs.

The new code is only generated for prefs marked with the new rust field in
the YAML. It's opt-in because there's no point generating additional code for
900+ static prefs when only about 20 are currently used from Rust.

This patch only marks a single pref (browser.display.document_color_use) with
rust: true. That pref isn't accessed from Rust code in this patch, but it's
necessary because the generated Rust code is invalid if there are zero
Rust-accessed prefs. (The next patch will access that pref and others from Rust
code.

Depends on D40790

It's much nicer.

One nice thing about this is that the new code is subject to the existing
threadedness checking, which identified that several of these should be atomic
because they're accessed off the main thread.

Depends on D40791

Bindgen is no longer necessary now that Rust bindings are generated by
generate_static_pref_list.py.

Depends on D40792

Another step in the renaming of VarCache variables as mirror variables,
matching the 'mirror' field used in StaticPrefList.yaml.

Depends on D40793

Pushed by nnethercote@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/45ed45bbbddc
Improve structure of generate_static_pref_list.py. r=glandium
https://hg.mozilla.org/integration/autoland/rev/7f9de13dc2e7
Generate static pref getters usable from Rust code. r=glandium
Keywords: leave-open
Pushed by nnethercote@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e3c86ffa168c
Use `static_prefs::pref!` in Stylo. r=emilio
https://hg.mozilla.org/integration/autoland/rev/2559f5ddf8e0
Don't bindgen static prefs. r=emilio
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception&fromchange=2559f5ddf8e06ec761e35aaa401a66e6de5189fa&tochange=978d8be38d3a2e0e58755826993f099dcf39cc39&searchStr=mn&selectedJob=260309534

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=260309534&repo=autoland&lineNumber=32201

Backout link: https://hg.mozilla.org/integration/autoland/rev/7e98e814517c1c4b6ab999a81e9db5d54dc98a6a

[task 2019-08-07T08:41:28.742Z] 08:41:28     INFO - PROCESS-CRASH | testing/marionette/harness/marionette_harness/tests/unit/test_profile_management.py TestSwitchProfileWithoutWorkspace.test_replace_with_external_profile | application crashed [@ <style::stylesheets::rule_parser::NestedRuleParser as cssparser::rules_and_declarations::AtRuleParser>::parse_block]
[task 2019-08-07T08:41:28.742Z] 08:41:28     INFO - Crash dump filename: /var/folders/k9/gj8x8g2n7tzb8c8h_sj_f3bm000017/T/tmpt1TMztexternal/minidumps/2B6D8607-7EA6-41D8-ABA2-B55805146C8B.dmp
[task 2019-08-07T08:41:28.742Z] 08:41:28     INFO - Operating system: Mac OS X
[task 2019-08-07T08:41:28.742Z] 08:41:28     INFO -                   10.14.5 18F132
[task 2019-08-07T08:41:28.742Z] 08:41:28     INFO - CPU: amd64
[task 2019-08-07T08:41:28.742Z] 08:41:28     INFO -      family 6 model 69 stepping 1
[task 2019-08-07T08:41:28.742Z] 08:41:28     INFO -      4 CPUs
[task 2019-08-07T08:41:28.742Z] 08:41:28     INFO - 
[task 2019-08-07T08:41:28.742Z] 08:41:28     INFO - GPU: UNKNOWN
[task 2019-08-07T08:41:28.742Z] 08:41:28     INFO - 
[task 2019-08-07T08:41:28.742Z] 08:41:28     INFO - Crash reason:  EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
[task 2019-08-07T08:41:28.742Z] 08:41:28     INFO - Crash address: 0x0
[task 2019-08-07T08:41:28.742Z] 08:41:28     INFO - Process uptime: 4 seconds
[task 2019-08-07T08:41:28.742Z] 08:41:28     INFO - 
[task 2019-08-07T08:41:28.742Z] 08:41:28     INFO - Thread 30 (crashed)
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -  0  XUL!<style::stylesheets::rule_parser::NestedRuleParser as cssparser::rules_and_declarations::AtRuleParser>::parse_block [rule_parser.rs:2559f5ddf8e06ec761e35aaa401a66e6de5189fa : 468 + 0x11]
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     rax = 0x000000010bd817fa   rdx = 0x7369642d746e6f66
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     rcx = 0x0000000110796e30   rbx = 0x000070000530dbd0
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     rsi = 0x000000010b7a25e4   rdi = 0x000000010cf8d9e0
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     rbp = 0x000070000530e0e0   rsp = 0x000070000530d870
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -      r8 = 0x000000011f302282    r9 = 0x000070000530e4d0
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     r10 = 0x000000011f302276   r11 = 0x000070000530e030
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     r12 = 0x000000000000000c   r13 = 0x000070000530e080
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     r14 = 0x000070000530dad0   r15 = 0x000070000530e178
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     rip = 0x000000010b7a2087
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     Found by: given as instruction pointer in context
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -  1  XUL!cssparser::rules_and_declarations::parse_at_rule [rule_parser.rs:2559f5ddf8e06ec761e35aaa401a66e6de5189fa : 249 + 0x5]
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     rbp = 0x000070000530e450   rsp = 0x000070000530e0f0
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     rip = 0x000000010b7935de
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     Found by: previous frame's frame pointer
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -  2  XUL!style::stylesheets::stylesheet::StylesheetContents::from_str + 0x8ce
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     rbp = 0x000070000530e850   rsp = 0x000070000530e460
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     rip = 0x000000010b7bb97e
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     Found by: previous frame's frame pointer
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -  3  XUL!geckoservo::stylesheet_loader::AsyncStylesheetParser::parse [stylesheet_loader.rs:2559f5ddf8e06ec761e35aaa401a66e6de5189fa : 128 + 0x2b]
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     rbp = 0x000070000530ea10   rsp = 0x000070000530e860
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     rip = 0x000000010b4deeb8
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     Found by: previous frame's frame pointer
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -  4  XUL!<rayon_core::job::HeapJob<BODY> as rayon_core::job::Job>::execute [glue.rs:2559f5ddf8e06ec761e35aaa401a66e6de5189fa : 1434 + 0x4c]
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     rbp = 0x000070000530eaf0   rsp = 0x000070000530ea20
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     rip = 0x000000010b4dec2d
[task 2019-08-07T08:41:28.743Z] 08:41:28     INFO -     Found by: previous frame's frame pointer
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -  5  XUL!rayon_core::registry::WorkerThread::wait_until_cold [job.rs:2559f5ddf8e06ec761e35aaa401a66e6de5189fa : 59 + 0x6]
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     rbp = 0x000070000530eb30   rsp = 0x000070000530eb00
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     rip = 0x000000010b86bdb5
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     Found by: previous frame's frame pointer
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -  6  XUL!std::sys_common::backtrace::__rust_begin_short_backtrace [backtrace.rs:a53f9df32fbb0b5f4382caaad8f1a46f36ea887c : 136 + 0x5fd]
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     rbp = 0x000070000530edb0   rsp = 0x000070000530eb40
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     rip = 0x000000010b86c88f
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     Found by: previous frame's frame pointer
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -  7  XUL!core::ops::function::FnOnce::call_once{{vtable.shim}} [function.rs:a53f9df32fbb0b5f4382caaad8f1a46f36ea887c : 231 + 0x76]
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     rbp = 0x000070000530ee30   rsp = 0x000070000530edc0
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     rip = 0x000000010b86c206
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     Found by: previous frame's frame pointer
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -  8  XUL!<alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once [boxed.rs:a53f9df32fbb0b5f4382caaad8f1a46f36ea887c : 704 + 0x2c]
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     rbp = 0x000070000530eed0   rsp = 0x000070000530ee40
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     rip = 0x000000010b8c127d
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     Found by: previous frame's frame pointer
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -  9  XUL!std::sys::unix::thread::Thread::new::thread_start [thread.rs:a53f9df32fbb0b5f4382caaad8f1a46f36ea887c : 79 + 0x79]
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     rbp = 0x000070000530ef10   rsp = 0x000070000530eee0
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     rip = 0x000000010b8c3597
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     Found by: previous frame's frame pointer
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO - 10  0x7fff60c782eb
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     rbp = 0x000070000530ef30   rsp = 0x000070000530ef20
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     rip = 0x00007fff60c782eb
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     Found by: previous frame's frame pointer
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO - 11  0x7fff60c7b249
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     rbp = 0x000070000530ef50   rsp = 0x000070000530ef40
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     rip = 0x00007fff60c7b249
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO -     Found by: previous frame's frame pointer
[task 2019-08-07T08:41:28.744Z] 08:41:28     INFO - 12  0x7fff60c7740d
[task 2019-08-07T08:41:28.745Z] 08:41:28     INFO -     rbp = 0x000070000530ef78   rsp = 0x000070000530ef60
[task 2019-08-07T08:41:28.745Z] 08:41:28     INFO -     rip = 0x00007fff60c7740d
[task 2019-08-07T08:41:28.745Z] 08:41:28     INFO -     Found by: previous frame's frame pointer
[task 2019-08-07T08:41:28.745Z] 08:41:28     INFO - 13  XUL + 0x60a8510
[task 2019-08-07T08:41:28.745Z] 08:41:28     INFO -     rsp = 0x000070000530f090   rip = 0x000000010b8c3510
[task 2019-08-07T08:41:28.745Z] 08:41:28     INFO -     Found by: stack scanning
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Attachment #9083577 - Attachment is obsolete: true
Flags: needinfo?(n.nethercote)
Pushed by nnethercote@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/02395d06e804
Use `static_prefs::pref!` in Stylo. r=emilio
Flags: needinfo?(n.nethercote)
Keywords: leave-open
Pushed by nnethercote@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/aaa04efb13f9
Don't bindgen static prefs. r=emilio
https://hg.mozilla.org/integration/autoland/rev/40f5cf9e18d1
Rename sVarCache_* as sMirror_*. r=KrisWright
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
You need to log in before you can comment on or make changes to this bug.