Bug 32684 - [Qt] Allow the application to override online/offline network status
Summary: [Qt] Allow the application to override online/offline network status
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Yael
URL:
Keywords: Qt
Depends on:
Blocks:
 
Reported: 2009-12-17 16:14 PST by Yael
Modified: 2010-01-07 09:30 PST (History)
4 users (show)

See Also:


Attachments
Patch. (7.57 KB, patch)
2009-12-17 16:25 PST, Yael
no flags Details | Formatted Diff | Diff
Patch v2 (5.92 KB, patch)
2010-01-06 11:10 PST, Yael
no flags Details | Formatted Diff | Diff
Patch v3 (5.50 KB, patch)
2010-01-07 09:17 PST, Yael
kenneth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yael 2009-12-17 16:14:39 PST
The application should be able to force offline mode even if there is a network connection.
Comment 1 Yael 2009-12-17 16:25:55 PST
Created attachment 45109 [details]
Patch.

Add an attribute to QWebSettings.
Comment 2 Kenneth Rohde Christiansen 2009-12-18 03:54:14 PST
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.
Comment 3 Yael 2009-12-21 07:20:01 PST
(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.
Comment 4 Simon Hausmann 2009-12-29 03:22:52 PST
Comment on attachment 45109 [details]
Patch.

If this is temporary until QNAM supports it, then it should be made private API, not public API.
Comment 5 Laszlo Gombos 2009-12-30 10:16:20 PST
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.
Comment 6 Yael 2009-12-30 16:00:34 PST
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.
Comment 7 Kenneth Rohde Christiansen 2009-12-31 04:26:08 PST
(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 :-)
Comment 8 Yael 2010-01-06 11:10:52 PST
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.
Comment 9 Laszlo Gombos 2010-01-06 11:44:46 PST
(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).
Comment 10 Yael 2010-01-06 11:51:37 PST
(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.
Comment 11 Kenneth Rohde Christiansen 2010-01-06 11:56:25 PST
(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.
Comment 12 Yael 2010-01-06 12:11:01 PST
(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.
Comment 13 Laszlo Gombos 2010-01-06 14:49:47 PST
Yael promised to create another patch with the (private) API always exposed (not guarded).
Comment 14 Yael 2010-01-07 05:37:23 PST
(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.
Comment 15 Laszlo Gombos 2010-01-07 05:53:02 PST
(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
+}
Comment 16 Yael 2010-01-07 05:55:54 PST
(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.
Comment 17 Yael 2010-01-07 09:17:44 PST
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.
Comment 18 WebKit Review Bot 2010-01-07 09:20:28 PST
style-queue ran check-webkit-style on attachment 46058 [details] without any errors.
Comment 19 Simon Hausmann 2010-01-07 09:28:43 PST
Comment on attachment 46058 [details]
Patch v3

will apply manually
Comment 20 Simon Hausmann 2010-01-07 09:29:49 PST
Committed r52928: <http://trac.webkit.org/changeset/52928>
Comment 21 Simon Hausmann 2010-01-07 09:30:16 PST
Cherry-picked into qtwebkit-4.6 as 99ccc1c3e4db5354246720f9b9aa3d282e64497d