Closed Bug 1159731 Opened 9 years ago Closed 9 years ago

Move Addon actor specifics to its own file

Categories

(DevTools :: Framework, defect)

defect
Not set
normal

Tracking

(firefox40 fixed)

RESOLVED FIXED
Firefox 40
Tracking Status
firefox40 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

Details

Attachments

(1 file, 1 obsolete file)

While working on bug 1159727, I realised that webbrowser.js was made of more and more random stuff.
As ContentActor and ChromeActor, BrowserAddonActor should be moved out of webbrowser.js.
While doing that I thought it would be also a good idea to move AddonWebConsoleActor in that same browser-addon.js file...
Attachment #8599320 - Flags: review?(ejpbruel)
Just a move of code... I tested it manually against simple addon.
Note that there is some random exception due to gcli that doesn't seem to break anything (unrelated to this patch, it also happens on nightly)
Assignee: nobody → poirot.alex
Comment on attachment 8599320 [details] [diff] [review]
patch v1

Review of attachment 8599320 [details] [diff] [review]:
-----------------------------------------------------------------

I would prefer to name the file "addon.js", but it's your call. Otherwise, this patch looks good to me.

::: toolkit/devtools/server/actors/webbrowser.js
@@ -110,5 @@
>  }
>  
>  exports.sendShutdownEvent = sendShutdownEvent;
>  
>  /**

Why did this move to script.js? Is it never used in this file?
Attachment #8599320 - Flags: review?(ejpbruel) → review+
(In reply to Eddy Bruel [:ejpbruel] from comment #3)
> Comment on attachment 8599320 [details] [diff] [review]
> patch v1
> 
> Review of attachment 8599320 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I would prefer to name the file "addon.js", but it's your call.

addon.js sounds good as well, especially if I put more than just BrowserAddonActor!

> ::: toolkit/devtools/server/actors/webbrowser.js
> @@ -110,5 @@
> >  }
> >  
> >  exports.sendShutdownEvent = sendShutdownEvent;
> >  
> >  /**
> 
> Why did this move to script.js? Is it never used in this file?

It is used in both, but sounds more something related to script.js as it relates to Debugger.Object API.
In this patch I'm pulling it from script.js from webbrowser.js, but I can do the opposite if you prefer.

See webbrowser.js head:
  loader.lazyRequireGetter(this, "unwrapDebuggerObjectGlobal", "devtools/server/actors/script", true);
Attached patch patch v2Splinter Review
Renamed to addon.js
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d28f81df84f8
Attachment #8599320 - Attachment is obsolete: true
Attachment #8602141 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9d96f30f4f41
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.