Bug 150058 – Add SPI for reloading without content blockers

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Add SPI for reloading without content blockers

[Alex Christensen]

Created attachment 262934 [details]
Patch

[Alex Christensen]

Created attachment 262936 [details]
Patch

[Alex Christensen]

Created attachment 263032 [details]
Patch

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Carlos Garcia Campos]

Comment on attachment 263032 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=263032&action=review

> Source/WebKit2/UIProcess/WebFrameProxy.cpp:251
> -            page->reload(false);
> +            page->reload(false, true);

I think this would be easier to read now if we use flags instead of booleans.

[Sam Weinig]

(In reply to comment #5)
> Comment on attachment 263032 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=263032&action=review
> 
> > Source/WebKit2/UIProcess/WebFrameProxy.cpp:251
> > -            page->reload(false);
> > +            page->reload(false, true);
> 
> I think this would be easier to read now if we use flags instead of booleans.

Agreed. Perhaps it would be nicer for reload to take a ReloadOptions object.

[Alex Christensen]

(In reply to comment #6)
> (In reply to comment #5)
> > Comment on attachment 263032 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=263032&action=review
> > 
> > > Source/WebKit2/UIProcess/WebFrameProxy.cpp:251
> > > -            page->reload(false);
> > > +            page->reload(false, true);
> > 
> > I think this would be easier to read now if we use flags instead of booleans.
> 
> Agreed. Perhaps it would be nicer for reload to take a ReloadOptions object.
I agree, but these are passed as parameters all over WebKit2 with IPC and WebCore.  This is the only place that is hard to read.  I think something like this would make it easier to read without requiring a newly defined type:
const bool reloadFromOrigin = false;
const bool contentBlockersEnabled = true;
page->reload(reloadFromOrigin, contentBlockersEnabled);

[Alex Christensen]

Created attachment 263092 [details]
Patch

[Alex Christensen]

Comment on attachment 263092 [details]
Patch

I named all the booleans at the call sites.  That achieves the readability without mucking up the code too much.  If we feel that there are too many parameters, we can wrap the bools in a struct or flags, but I think that's too invasive for this change.

[Alex Christensen]

http://trac.webkit.org/changeset/191063