Bug 24487 - [Gtk] Implement LayoutTestControllerGtk::setPrivateBrowsingEnabled
: [Gtk] Implement LayoutTestControllerGtk::setPrivateBrowsingEnabled
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: WebKit Gtk
: 528+ (Nightly build)
: PC Linux
: P2 Normal
Assigned To: Nobody
: Gtk
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-03-10 03:32 PDT by Jan Alonzo
Modified: 2009-04-05 01:27 PDT (History)
2 users (show)

See Also:


Attachments
Implement private browsing in DRT and add option in WebKit/Gtk (10.57 KB, patch)
2009-03-10 03:44 PDT, Jan Alonzo
no flags Details | Formatted Diff | Diff
updated documentation (1.39 KB, patch)
2009-03-21 03:03 PDT, Jan Alonzo
zecke: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Alonzo 2009-03-10 03:32:44 PDT
Enable private browsing adding a 'enable-private-browsing' setting in WebKitWebSettings and enabling it in DRT. 

Doing this passes http/tests/security/cross-frame-access-private-browsing.html.
Comment 1 Jan Alonzo 2009-03-10 03:44:09 PDT
Created attachment 28433 [details]
Implement private browsing in DRT and add option in WebKit/Gtk
Comment 2 Holger Freyther 2009-03-11 02:03:56 PDT
Comment on attachment 28433 [details]
Implement private browsing in DRT and add option in WebKit/Gtk


> +    resetWebViewToConsistentStateBeforeTesting();
> +

you might want to copy more there... e.g. the loading of about:blank (which we should kill at some point)


ate after certain tests.
>      webkit_web_view_open(webView, "about:blank");
>  
> -    gLayoutTestController->setJavaScriptProfilingEnabled(false);

NO! The g_object_set in resetWebView... is no good replacement! setJSProfilingEnabled will disable both Developer Extras and JS profiling. Is there a reason you want to keep Developer Extras enabled?
Comment 3 Jan Alonzo 2009-03-11 02:24:03 PDT
(In reply to comment #2)
> (From update of attachment 28433 [details] [review])
> 
> > +    resetWebViewToConsistentStateBeforeTesting();
> > +
> 
> you might want to copy more there... e.g. the loading of about:blank (which we
> should kill at some point)

I'll do that in a different patch.

> 
> 
> ate after certain tests.
> >      webkit_web_view_open(webView, "about:blank");
> >  
> > -    gLayoutTestController->setJavaScriptProfilingEnabled(false);
> 
> NO! The g_object_set in resetWebView... is no good replacement!
> setJSProfilingEnabled will disable both Developer Extras and JS profiling. Is
> there a reason you want to keep Developer Extras enabled?

Why not? Why is the resetWebView... not a good replacement? developer extras is also disabled in resetWebView...

Thanks
Comment 4 Holger Freyther 2009-03-11 03:25:45 PDT
(In reply to comment #3)

> 
> Why not? Why is the resetWebView... not a good replacement? developer extras is
> also disabled in resetWebView...

right, didn't see it/looked for it in the g_object_set. Please describe this change in the ChangeLog and why it is semantically equivalent.
Comment 5 Jan Alonzo 2009-03-11 05:28:02 PDT
Landed in r41583. Thanks a lot dude.
Comment 6 Christian Dywan 2009-03-15 08:19:20 PDT
(In reply to comment #5)
> Landed in r41583. Thanks a lot dude.

This new setting badly needs documentation, neither Xan, Gustavo or me could grasp what it does without digging inside WebCore. So I'm listing behaviour which may be affected, and we need to verify and unit test those points.

- "Follow up to <rdar://problem/5394877> Safari should not log unsafe JavaScript attempts when in private browsing mode (only an issue if Log JavaScript Exceptions is turned on)"
- No page navigation ends up in history?
- Does it affect cookie storage?
- Cache is not used?
- Form completion is discarded?

The last two probably don't apply simply because we don't have these features.
Comment 7 Gustavo Noronha (kov) 2009-03-16 12:06:37 PDT
Comment on attachment 28433 [details]
Implement private browsing in DRT and add option in WebKit/Gtk

Clearing review flag so that it stops showing up in the pending commit queue.
Comment 8 Jan Alonzo 2009-03-21 03:03:14 PDT
Created attachment 28820 [details]
updated documentation
Comment 9 Jan Alonzo 2009-03-21 03:07:16 PDT
I uploaded a new patch to fix these. I appreciate any feedback if any. I tagged that feature as experimental for WebKitGtk since some features like on-disk caching are, i think, not yet implemented for Gtk.

(In reply to comment #6)
> grasp what it does without digging inside WebCore. So I'm listing behaviour
> which may be affected, and we need to verify and unit test those points.
> 
> - "Follow up to <rdar://problem/5394877> Safari should not log unsafe
> JavaScript attempts when in private browsing mode (only an issue if Log
> JavaScript Exceptions is turned on)"
> - No page navigation ends up in history?
> - Does it affect cookie storage?
> - Cache is not used?
> - Form completion is discarded?
> 
> The last two probably don't apply simply because we don't have these features.

That's right. I think the only thing to test here is the cookie storage. Is there a way to test that?


Comment 10 Holger Freyther 2009-03-30 06:12:35 PDT
Cookies are added by WebCore to the session, caching data to secondary storage is something that soup would do, so there might be a need to synchronize this flag with the soup session...
Comment 11 Holger Freyther 2009-04-02 23:42:44 PDT
Comment on attachment 28820 [details]
updated documentation 


> +    * This is currently experimental for WebKitGtk.

Okay, change that to WebKitGTK+ (as this seems to be our name now) :)
Comment 12 Jan Alonzo 2009-04-05 01:27:09 PDT
Landed in r42231. Thanks!