[Webkit-unassigned] [Bug 87993] window.internals is not good for changing persistent settings

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=87993





--- Comment #3 from Hajime Morrita <morrita at google.com>  2012-06-05 17:33:27 PST ---
(In reply to comment #2)
> Hi! I will try to understand the issue related to this bug a bit further to see if I can help.
> 
> (In reply to comment #1)
> > (In reply to comment #0)
> > > 1. Changing a setting in one window.internals.settings object affects all frames in a page, but does not work across secondary windows. This is inconsistent.
> > > 
> > Right.
> 
> Any ideas? What would be more consistent, not affecting all frames in a page or working across secondary windows?
> 
> 
> > 
> > > 2. If a test navigates to a different URL (with waitUntilDone/notifyDone), settings are restored from last document's InternalSettings instead of first one's, so changes made in first one just leak to future tests.
> 
> Due to m_settings->restoreTo(document->page()->settings()) in Internals::reset(), right? We could fix this by keeping a reference to the first document and then using it to restore the settings, but is this what we want?

I don't think changing document lifetime by holding reference is a good idea.
It potentially hides real bugs since there had been man crashes at the end of
document life. We can hold a reference to Internals object though.

> 
> 
> > > 
> > > 3. Code design is not very clear. Settings are per-page, yet many Internals methods take Document, Frame or a global context.
> > This is also true. InternalSettings started from simple extraction from 
> > Internals object, which had no state at that time.
> > We could have better lifecycle representation around here.
> 
> What do you mean by better lifecycle representation more specifically?

My current thinking is to attach Internals object to the Page and
put it for each window, instead of creating Internals for each time.
By doing this, Internals and InternalSettings will have same lifetime as Page.

Does this make sense?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.