Script: removed a few opts::get()

[oneturkmen]

I removed only a few opts::get() from the components/script because all other code with "opts::get()" is reused by many other parts within the same component.

---

• ./mach build -d does not report any errors
• ./mach test-tidy does not report any errors
• These changes fix partially #22854 (GitHub issue number if applicable)

• These changes do not require tests because these are cleanup changes.

---

This change is 

[highfive]

Heads up! This PR modifies the following files:

• @asajeffrey: components/script/script_thread.rs, components/script/dom/window.rs, components/constellation/pipeline.rs
• @cbrewster: components/constellation/pipeline.rs
• @paulrouget: components/constellation/pipeline.rs
• @KiChjang: components/script/script_thread.rs, components/script/dom/window.rs, components/script_traits/lib.rs

[highfive]

Warning

• These commits modify script code, but no tests are modified. Please consider adding a test!

[oneturkmen]

r? @jdm

[oneturkmen]

@jdm could you bors-retry since there's been a network issue again. Thank you!

caused by: failed to make network request

[CYBAI]

@bors-servo try=wpt

@oneturkmen No worries about the Travis failure if it's network issue :) We mainly see the CI result from bors-servo. btw, bors can't retry Travis 😂

[bors-servo]

⌛ Trying commit 270dff5 with merge d1766fa...

[bors-servo]

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
State: approved= try=True

[jdm]

@oneturkmen Could you show a few examples of what you mean by "all other code with "opts::get()" is reused by many other parts within the same component"?

[servo-wpt-sync]

Error syncing changes upstream. Logs saved in error-snapshot-1560658517075.

[jdm]

Looks like the rebase didn't go as planned. I recommend git reset --hard fd174c54ef4fa6574ae782dacccaeccd14abb936; git cherry-pick 270dff58b400ab4e41b11c12f848dbf30bad5f6c and force pushing the result.

[oneturkmen]

@jdm sorry for the long delay. I will push the changes for the three bools in a bit.

One example of what I meant would be this opts::get(). It is defined in function new_rt_and_cx_with_parent(...), which is reused (or wrapped) by public functions new_child_runtime() (here) and new_rt_and_cx() (here). And recurringly (if such word exists), they are used, e.g. in script/dom/serviceworkerglobalscope.rs, or script/dom/worklet.rs ... Do I make sense?

Another example is with opts::get() in dom/permissions.rs. The same recurring call pattern exists there.

I hope this clarifies the issue.

[jdm]

I think in many cases we can add such options as members of GlobalScope, and then code can either access it transitively through the parent field (https://doc.servo.org/script/dom/index.html#inheritance), or by calling the global() API on a DOM object (https://doc.servo.org/script/dom/bindings/reflector/trait.DomObject.html#method.global), or accessing a GlobalScope object that's already present.

[oneturkmen]

@jdm I may need your guidance here, if you don't mind.

I am currently trying to remove opts::get().userscripts from components/script/dom/userscripts.rs. The function load_scripts where this opts::get() is defined is used by dom/headelement.rs here.

As you mentioned, I am passing parameters all along through "pub fn new()" and then via "fn new_inherited()" within the same struct implementation. However, I reached the moment where I have to change the signature of Document constructor (i.e. pub fn new()), but it is used in other 10-12 places ... I feel like I am doing something wrong here. Could you give any hint/prompt on how (or where) to look to make change(s)?

[jdm]

Great, that's a very concrete question I can answer :) I recommend storing a userscripts_path member in Window, and then that code can access it by calling a method that returns it. Since there is only one place that creates Window objects (script_thread.rs), the value can be stored in ScriptThread and passed to the Window::new constructor.

[oneturkmen]

@jdm thanks for the answer! do you think I could do the same (if technically possible) for other opts::get()?

Update: never mind. It's a vague question to ask since it really depends on the context, i.e. methods/functions and structs used. If I find something very ambiguous, I will let you know (with another specific question hehe).

[bors-servo]

⌛ Testing commit 6b578e9 with merge 53e79fc...

[jdm]

Non-linux builds reported this error:

error: cannot find macro `pref!` in this scope
   --> components/script/dom/permissions.rs:317:12
    |
317 |         if pref!(dom.permissions.testing.allowed_in_nonsecure_contexts) {
    |            ^^^^
error: aborting due to previous error

[oneturkmen]

Non-linux builds reported this error:

error: cannot find macro `pref!` in this scope
   --> components/script/dom/permissions.rs:317:12
    |
317 |         if pref!(dom.permissions.testing.allowed_in_nonsecure_contexts) {
    |            ^^^^
error: aborting due to previous error

Yup, removed.

[jdm]

@bors-servo r+

[bors-servo]

📌 Commit 0da2b74 has been approved by jdm

[bors-servo]

⌛ Testing commit 0da2b74 with merge 7833332...

[oneturkmen]

Seems that it fails even on warnings (forgot to prefix the unused variable). Pushing changes now.

[jdm]

@bors-servo r+
Indeed. Thanks!

[bors-servo]

📌 Commit 4256928 has been approved by jdm

[bors-servo]

⌛ Testing commit 4256928 with merge 61cadfa...

[bors-servo]

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: jdm
Pushing 61cadfa to master...