[Webkit-unassigned] [Bug 42265] Runtime feature switch for device orientation

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jul 15 04:31:13 PDT 2010


--- Comment #16 from hans at chromium.org  2010-07-15 04:31:13 PST ---
(In reply to comment #11)
> (From update of attachment 61531 [details])
> WebCore/page/Page.cpp:143
>  +      , m_deviceOrientationController(RuntimeEnabledFeatures::deviceOrientationEnabled() ? new DeviceOrientationController(this, deviceOrientationClient) : 0)
> Should we be creating this lazily on first use?  I see that at least the geolocation one isn't, but maybe that should be changed too?  (It's a shame to slow down Page creation for features that may or may never be accessed.  Doing this lazily is generally the webkit way.)  It's ok to do this in another patch, but I think it should be done unless there's a good reason not to.
Let's aim for doing it in another patch, then.

> WebKit/chromium/src/WebRuntimeFeatures.cpp:231
> Do we need these guards?  It seems like not.
Right, let's remove it.

> WebKit/chromium/src/WebRuntimeFeatures.cpp:236
>  +  bool WebRuntimeFeatures::isDeviceOrientationEnabled()
> Interesting.  I was going to say the same here, but I realized that we don't want it to say true when support isn't even compiled in.  But then I realized I could say the same about what's in WebCore as well.  I almost wonder if it should default to false if the feature isn't compiled in.  But in that case, we should probably do the same for the other features.
> What do you think?
I don't think it matters as much what the flag in WebCore is when the feature is not compiled in, as calling code in WebCore would be behind compile-time enable guard anyway.

In Chromium, we're going to flip the compile-time flag anyway (and expect it to be on, as some code will break without it), so we might as well drop the guards here as well. And I agree with Steve that the fever guards we need, the better.

Uploading new patch with changes to the makefiles to hopefully make the ews bots happy. We should watch this closely when it lands eventually to make sure nothing else breaks. Please chime in if I have missed any obvious build file.

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

More information about the webkit-unassigned mailing list