599632 - [Fennec] SecurityUI reinitialized for the same window

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Honza Bambas (:mayhemer)]

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.

Comment 1

[Honza Bambas (:mayhemer)]

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.

Comment 2

[Honza Bambas (:mayhemer)]

Attachment: v1

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

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 481542 [details] [diff] [review]
v1

How does the securityUI get initialized at all?

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Oh, you're saying that we need to remove this code and apply the patch in bug 575950?

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

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

Comment 6

[Honza Bambas (:mayhemer)]

(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.

Comment 7

[Honza Bambas (:mayhemer)]

Attachment: v1.1

Comment 8

[Josh Matthews [:jdm]]

http://hg.mozilla.org/mobile-browser/rev/c87808b19e9b

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

code has landed so marking fixed. bug 575950 still needs to land