Bug 32684 – [Qt] Allow the application to override online/offline network status

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Yael]

The application should be able to force offline mode even if there is a network connection.

[Yael]

Created attachment 45109 [details]
Patch.

Add an attribute to QWebSettings.

[Kenneth Rohde Christiansen]

Comment on attachment 45109 [details]
Patch.

Personally I find the name a bit confusing. Also, as the bug touches API you need to make it a blocker on our 4.7 API bug.

[Yael]

(In reply to comment #2)
> (From update of attachment 45109 [details])
> Personally I find the name a bit confusing. Also, as the bug touches API you
> need to make it a blocker on our 4.7 API bug.

This API is temporary for S60. QNAM will provide the final version of the API, and when that is released, we would not need to use this API anymore.

[Simon Hausmann]

Comment on attachment 45109 [details]
Patch.

If this is temporary until QNAM supports it, then it should be made private API, not public API.

[Laszlo Gombos]

I do not see an easy way to create a "non-public setting" so maybe a simple qt_networkAccessAllowed(bool) API will do. 

qt_networkAccessAllowed need also be listed in the Symbian def files so that they are exported (see symbian/bwins/QtWebKitu.def and symbian/eabi/QtWebKitu.def).

I think patch does not need the ENABLE(QT_BEARER) guard.

Yael, Can you recreate the patch ? - it should apply against http://gitorious.org/+qtwebkit-developers/webkit/qtwebkit. Thanks.

[Yael]

Thank you for your comments. Can someone please tell me how does bearer management get included in the gitorious builds? I noticed that the first patch was removed from the build in r52654.

[Kenneth Rohde Christiansen]

(In reply to comment #5)
> I do not see an easy way to create a "non-public setting" so maybe a simple
> qt_networkAccessAllowed(bool) API will do.

That would be my suggestion :-)

[Yael]

Created attachment 45969 [details]
Patch v2

Updated the patch to use private API. I am not comfortable with removing the build time flag for this patch, because if the patch was committed to the trunk, we would want to keep these flags.

[Laszlo Gombos]

(In reply to comment #8)
> Created an attachment (id=45969) [details]
> Patch v2

I think this patch will need a def file update for Symbian. Just a nit - no need to cache the networkAccessAllowed state info in qwebsettings.cpp.

Also, even though this was prepared specifically for 4.6. branch, we might want to consider landing on the trunk as well (with the private API).

[Yael]

(In reply to comment #9)
> Also, even though this was prepared specifically for 4.6. branch, we might want
> to consider landing on the trunk as well (with the private API).

We are negotiating with Bearer Management team a new API, that will eliminate the need for this patch. It was created strictly as a short-term stop gap. Once we have the new Bearer Management API available, this patch will not be needed at all.

[Kenneth Rohde Christiansen]

(In reply to comment #10)
> (In reply to comment #9)
> > Also, even though this was prepared specifically for 4.6. branch, we might want
> > to consider landing on the trunk as well (with the private API).
> 
> We are negotiating with Bearer Management team a new API, that will eliminate
> the need for this patch. It was created strictly as a short-term stop gap. Once
> we have the new Bearer Management API available, this patch will not be needed
> at all.

But if this is not committed then I cannot build anything requiring 2010.1 using trunk right? If that is the case, we should add it to trunk.

[Yael]

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > Also, even though this was prepared specifically for 4.6. branch, we might want
> > > to consider landing on the trunk as well (with the private API).
> > 
> > We are negotiating with Bearer Management team a new API, that will eliminate
> > the need for this patch. It was created strictly as a short-term stop gap. Once
> > we have the new Bearer Management API available, this patch will not be needed
> > at all.
> 
> But if this is not committed then I cannot build anything requiring 2010.1
> using trunk right? If that is the case, we should add it to trunk.

I would not say "I cannot build anything". This patch is to allow something like "work offline" menu in firefox browser. The dependency for this feature is still under development and I think this is still not ready to go to the trunk.

[Laszlo Gombos]

Yael promised to create another patch with the (private) API always exposed (not guarded).

[Yael]

(In reply to comment #13)
> Yael promised to create another patch with the (private) API always exposed
> (not guarded).

Thinking about this a little more, I came to the conclusion that the private API has to stay in QWebSettings and not be moved to NetworkNotifierQt, as we agreed. The reason is simple, when the flag is truned off, that file is not part of the build, so it kind of invalidates comment #15.

[Laszlo Gombos]

(In reply to comment #14)
> (In reply to comment #13)
> > Yael promised to create another patch with the (private) API always exposed
> > (not guarded).
> 
> Thinking about this a little more, I came to the conclusion that the private
> API has to stay in QWebSettings and not be moved to NetworkNotifierQt, as we
> agreed. The reason is simple, when the flag is truned off, that file is not
> part of the build, so it kind of invalidates comment #15.

I was thinking doing something like this - so that the (private) API is always exported:

Index: WebKit/qt/Api/qwebsettings.cpp
===================================================================

+void QWEBKIT_EXPORT qt_networkAccessAllowed(bool isAllowed)
+{
+#if ENABLE(QT_BEARER)
+    networkAccessAllowed = isAllowed;
+    WebCore::networkStateNotifier().setNetworkAccessAllowed(networkAccessAllowed);
+#endif
+}

[Yael]

(In reply to comment #15)
> I was thinking doing something like this - so that the (private) API is always
> exported:
> 
> Index: WebKit/qt/Api/qwebsettings.cpp
> ===================================================================
> 
> +void QWEBKIT_EXPORT qt_networkAccessAllowed(bool isAllowed)
> +{
> +#if ENABLE(QT_BEARER)
> +    networkAccessAllowed = isAllowed;
> +   
> WebCore::networkStateNotifier().setNetworkAccessAllowed(networkAccessAllowed);
> +#endif
> +}

Then we are in agreement:-) last night we talked about moving it to NetwrokNotifierQt, but it will not work.

[Yael]

Created attachment 46058 [details]
Patch v3

Moved the guard to be inside the new private API. people thought it would be better if this patch is landed in webkit.org too, so now it is ready for review.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 46058 [details] without any errors.

[Simon Hausmann]

Comment on attachment 46058 [details]
Patch v3

will apply manually

[Simon Hausmann]

Committed r52928: <http://trac.webkit.org/changeset/52928>

[Simon Hausmann]

Cherry-picked into qtwebkit-4.6 as 99ccc1c3e4db5354246720f9b9aa3d282e64497d