Closed Bug 599632 Opened 9 years ago Closed 9 years ago

[Fennec] SecurityUI reinitialized for the same window

Categories

(Core :: Networking: HTTP, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
fennec 2.0b2+ ---

People

(Reporter: mayhemer, Assigned: mayhemer)

References

Details

Attachments

(1 file, 1 obsolete file)

In Fennec desktop build I don't get this notification in nsSecureBrowserUI::OnStateChange after the initial load of a page.  After reload the notification is obtained.

This is probably happening because the top level channel is not being removed from its load group.
tracking-fennec: --- → ?
The problem is in a different place.  We apparently load an about:blank URI for a window, then start loading a page, after we start, we create a new instance of SecurityUI for that window, while throwing away the old instance.  The new instance is an invalid internal state.  Going to investigate further.
Summary: [Fennec] Missing STATE_STOP status notification for LOAD_DOCUMENT_URI channels → [Fennec] SecurityUI reinitialized for the same window
Attached patch v1 (obsolete) — Splinter Review
Removing initiation of security ui from browser.js.  Security UI for the docshell is initiated properly in nsWebBrowser::Create() [1].

This code [2] is called twice and too late, after we start load of the document.

I assume this bug is more complex.  This patch is just a quick fix for a blocking bug.

[1] http://hg.mozilla.org/mozilla-central/annotate/4d2eff8cc112/embedding/browser/webBrowser/nsWebBrowser.cpp#l1138
[2] http://hg.mozilla.org/mobile-browser/annotate/5807503f22d8/chrome/content/bindings/browser.xml#l61
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Attachment #481542 - Flags: review?(mark.finkle)
tracking-fennec: ? → 2.0b2+
Comment on attachment 481542 [details] [diff] [review]
v1

How does the securityUI get initialized at all?
Oh, you're saying that we need to remove this code and apply the patch in bug 575950?
Comment on attachment 481542 [details] [diff] [review]
v1

Since we are removing everything in the receiveMessage method we can remove receiveMessage() and init() and not call SecurityUI.init() anymore
Attachment #481542 - Flags: review?(mark.finkle) → review+
(In reply to comment #3)
> Comment on attachment 481542 [details] [diff] [review]
> v1
> 
> How does the securityUI get initialized at all?

See comment 2.

(In reply to comment #4)
> Oh, you're saying that we need to remove this code and apply the patch in bug
> 575950?

Exactly.  This fixes the problem with the security bar info not filled after the first load of a page.

(In reply to comment #5)
> Comment on attachment 481542 [details] [diff] [review]
> v1
> 
> Since we are removing everything in the receiveMessage method we can remove
> receiveMessage() and init() and not call SecurityUI.init() anymore

I was thinking of it too, just needed to get it confirmed.

I'll address your comments.
Attached patch v1.1Splinter Review
Attachment #481542 - Attachment is obsolete: true
Attachment #482281 - Flags: review+
code has landed so marking fixed. bug 575950 still needs to land
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.