Closed Bug 749397 Opened 9 years ago Closed 8 years ago

Network Monitor should be embedded into Firefox

Categories

(DevTools :: General, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: jwalker, Assigned: jwalker)

References

Details

Attachments

(2 files, 3 obsolete files)

No description provided.
Summary: HttpMonitor/NetPanel should be embedded into Firefox → Network Monitor should be embedded into Firefox
Whiteboard: [netpanel] → [netmonitor]
Attached patch Upload 1 (obsolete) — Splinter Review
Attachment #624449 - Flags: feedback?(odvarko)
Attached patch Upload 2 (obsolete) — Splinter Review
Working patch!
Attachment #624449 - Attachment is obsolete: true
Attachment #624449 - Flags: feedback?(odvarko)
Attachment #624719 - Flags: feedback?(odvarko)
Attachment #624719 - Flags: feedback?(mratcliffe)
> +  // Enable network monitor?
> +  let devToolbarEnabled =
The var name should be rather 'netMonitorEnabled' instead of 'devToolbarEnabled'


Why NewHttpMonitor? It's just a workaround?

What about following:
//Options.initialize("extensions.httpmonitor");
//Options.registerDefaultPrefs(DefaultPrefs);

Should we use NetMonitor.jsm and httpmonitor/app/main as the place for specific intialization steps?


> this._panelWindow.foo = this._httpMonitor;
What foo is for?

> NetMonitor.prototype.handleEvent = function DT_handleEvent(aEvent)
Where this is used?

Honza
Btw. where is the script that generates monitor.jsm?

Honza
I am also seeing:

JavaScript error: resource:///modules/devtools/NetMonitor.jsm, line 160: TypeError: this._panelWindow is undefined

When closing NetMonitor console.

I guess because:

this._onunload = this._onunload.bind(this);

is missing.

Honza
Another exception:

JavaScript error: resource:///modules/devtools/NetMonitor.jsm, line 121: TypeErr
or: this._browser is undefined

STR
1) Open NetMonitor
2) Close NetMinotaur
3) Open NetMonitor --> Exception

Also there is a beast hidden in the STR (just a little prank in case you would be bored by my comments :)

Honza
(In reply to Jan Honza Odvarko from comment #3)
> > +  // Enable network monitor?
> > +  let devToolbarEnabled =
> The var name should be rather 'netMonitorEnabled' instead of
> 'devToolbarEnabled'
> 
> 
> Why NewHttpMonitor? It's just a workaround?

Yes, and a totally nasty one. I wanted to have access to the HttpMonitor variable at the higher scope. I'd like there to be a better option, but not put much thought into it so far. Suggestions welcome.

Perhaps the best place to discuss these types of changes is in the github branch for this?
https://github.com/joewalker/httpmonitor/commits/mozilla-build

> What about following:
> //Options.initialize("extensions.httpmonitor");
> //Options.registerDefaultPrefs(DefaultPrefs);
> 
> Should we use NetMonitor.jsm and httpmonitor/app/main as the place for
> specific intialization steps?

I think Firefox specific initialization should be done in NetMonitor.jsm and general init should be done in httpmonitor


> > this._panelWindow.foo = this._httpMonitor;
> What foo is for?

Testing. It should go.


> > NetMonitor.prototype.handleEvent = function DT_handleEvent(aEvent)
> Where this is used?

It's not. It can probably go.
(In reply to Jan Honza Odvarko from comment #4)
> Btw. where is the script that generates monitor.jsm?

https://github.com/joewalker/httpmonitor/blob/mozilla-build/build.js#L44
(In reply to Jan Honza Odvarko from comment #5)
> I am also seeing:
> 
> JavaScript error: resource:///modules/devtools/NetMonitor.jsm, line 160:
> TypeError: this._panelWindow is undefined
> 
> When closing NetMonitor console.
> 
> I guess because:
> 
> this._onunload = this._onunload.bind(this);
> 
> is missing.

type-type-type
*was* missing.

Thanks.
(In reply to Jan Honza Odvarko from comment #6)
> Another exception:
> 
> JavaScript error: resource:///modules/devtools/NetMonitor.jsm, line 121:
> TypeErr
> or: this._browser is undefined
> 
> STR
> 1) Open NetMonitor
> 2) Close NetMinotaur
> 3) Open NetMonitor --> Exception
> 
> Also there is a beast hidden in the STR (just a little prank in case you
> would be bored by my comments :)

:)
Added to todo-list.
Comment on attachment 624719 [details] [diff] [review]
Upload 2

Just updating feedback flag, excellent work!
Honza
Attachment #624719 - Flags: feedback?(odvarko) → feedback+
Attached file Upload 3 (obsolete) —
Attachment #624719 - Attachment is obsolete: true
Attachment #624719 - Flags: feedback?(mratcliffe)
I can't start the Net Minotaur ... my error:
log: NetMonitor._onload()
error
TypeError
  - prototype TypeError
    - fileName = resource:///modules/devtools/NetMonitor.jsm
    - lineNumber = 146
  - prototype Error
  - prototype Object

Seems like this._httpMonitor is an empty object. Not sure why ... timing issue maybe.
Seems like the bug from comment 13 was fixed by bug 756887
No longer blocks: 749405
Attached patch Upload 4Splinter Review
Rebase
Attachment #625989 - Attachment is obsolete: true
Network Monitor being done elsewhere.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → INVALID
Whiteboard: [netmonitor]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.