This is a part of refactoring under Bug 76423.
Created attachment 123269 [details] Patch
Comment on attachment 123269 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123269&action=review > Source/WebCore/ChangeLog:12 > + [Refactoring][Internals] Should have InternalSettings > + https://bugs.webkit.org/show_bug.cgi?id=76424 > + > + Reviewed by NOBODY (OOPS!). > + > + - Invoked Internals::reset() in the constructor to employ Document object. > + - Moved setting and configuration related Internals methods to > + newly introduced InternalSettings object. > + > + No new tests, covered by existing tests. The main visible change of this patch is to introduce window.internals.settings. But the ChangeLog doesn't mention it. > Source/WebCore/WebCore.xcodeproj/project.pbxproj:20947 > 41815C1F138319830057AAA4 /* WebCoreTestSupport.h in Headers */, > + A7BF7EE014C9175A0014489D /* InternalSettings.h in Headers */, > + A740B5A514C935AB00A77FA4 /* JSInternalSettings.h in Headers */, We had better sort lines. > Source/WebCore/WebCore.xcodeproj/project.pbxproj:24401 > 417DA71D13735DFA007C57FB /* JSInternals.cpp in Sources */, > 41815C1E138319830057AAA4 /* WebCoreTestSupport.cpp in Sources */, > 07230CBC14C10ED900F6B702 /* JSInternalsCustom.cpp in Sources */, > + A7BF7EDF14C9175A0014489D /* InternalSettings.cpp in Sources */, > + A740B5A714C935AF00A77FA4 /* JSInternalSettings.cpp in Sources */, ditto. > Source/WebCore/testing/InternalSettings.cpp:2 > + * Copyright (C) 2011 Google Inc. All rights reserved. 2012 > Source/WebCore/testing/InternalSettings.cpp:128 > +void InternalSettings::setForceCompositingMode(bool enabled, ExceptionCode& ec) > +{ > + if (!settings()) { > + ec = INVALID_ACCESS_ERR; > + return; > + } > + > + settings()->setForceCompositingMode(enabled); > +} > + > +void InternalSettings::setEnableCompositingForFixedPosition(bool enabled, ExceptionCode& ec) > +{ > + if (!settings()) { > + ec = INVALID_ACCESS_ERR; > + return; > + } > + > + settings()->setAcceleratedCompositingForFixedPositionEnabled(enabled); > +} Repeating very similar implementations is not good. I think it's ok to use a preprocessor macro to generate functions.
Comment on attachment 123269 [details] Patch Attachment 123269 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11297039
Comment on attachment 123269 [details] Patch Attachment 123269 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11297048
Created attachment 123283 [details] Patch
Comment on attachment 123269 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123269&action=review Hi Kent-san, thanks for your responsive feedback! I updated the patch to address it. >> Source/WebCore/WebCore.xcodeproj/project.pbxproj:20947 >> + A740B5A514C935AB00A77FA4 /* JSInternalSettings.h in Headers */, > > We had better sort lines. Done. >> Source/WebCore/WebCore.xcodeproj/project.pbxproj:24401 >> + A740B5A714C935AF00A77FA4 /* JSInternalSettings.cpp in Sources */, > > ditto. Done. >> Source/WebCore/testing/InternalSettings.cpp:2 >> + * Copyright (C) 2011 Google Inc. All rights reserved. > > 2012 Done. >> Source/WebCore/testing/InternalSettings.cpp:128 >> +} > > Repeating very similar implementations is not good. > I think it's ok to use a preprocessor macro to generate functions. Good point. I realized that there are unfortunate ad-hoc naming trend and ifdefs that prevent us from generating whole function. So I extracted error check part instead.
Comment on attachment 123283 [details] Patch Attachment 123283 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11297076
Comment on attachment 123283 [details] Patch Attachment 123283 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11313079
Comment on attachment 123283 [details] Patch Pleas fix build.
Created attachment 123663 [details] Attemt to fix broken builds
Comment on attachment 123663 [details] Attemt to fix broken builds Attachment 123663 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11335299 New failing tests: fast/dom/window-inner-size-scaling.html fast/frames/frame-set-rotation-hit.html fast/dom/iframe-inner-size-scaling.html
Created attachment 123760 [details] Updated to ToT for catching up new tests
Comment on attachment 123760 [details] Updated to ToT for catching up new tests View in context: https://bugs.webkit.org/attachment.cgi?id=123760&action=review > Source/WebCore/testing/InternalSettings.cpp:51 > +#define INTERNAL_SETTINGS_GUARD_FOR_SETTINGS_RETURN(returnValue) \ http://www.webkit.org/coding/coding-style.html#names-define-non-const
Created attachment 123954 [details] Patch for landing
Kent-san, thank you for taking a look! > > Source/WebCore/testing/InternalSettings.cpp:51 > > +#define INTERNAL_SETTINGS_GUARD_FOR_SETTINGS_RETURN(returnValue) \ > > http://www.webkit.org/coding/coding-style.html#names-define-non-const Fixed in the landing patch.
Comment on attachment 123954 [details] Patch for landing Rejecting attachment 123954 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/11344621
Landed at http://trac.webkit.org/changeset/105900
The addition of V8InternalsSettings.* to WebCore.gyp is causing unnecessary rebuilds on Windows because it doesn't seem to exist. Will that file exist soon, or is it not supposed to be there?
Oh, it looks like it's supposed to be V8InternalSettings.h, not V8Internal_s_Settings.h. Could you fix that?
Please don't use ::-webkit-scrollbar style to hide scrollbars. This technique doesn't work in WebKit1 windows, so affects all the Mac DRT results.
(In reply to comment #20) > Please don't use ::-webkit-scrollbar style to hide scrollbars. This technique doesn't work in WebKit1 windows, so affects all the Mac DRT results. @smfr looks like you are talking about about a different bug.
Sorry, I just looked at the last change to: * compositing/geometry/fixed-position-composited-page-scale-down.html: * compositing/geometry/fixed-position-composited-page-scale.html: * compositing/geometry/fixed-position-composited-switch.html: * compositing/geometry/fixed-position-iframe-composited-page-scale-down.html: * compositing/geometry/fixed-position-iframe-composited-page-scale.html: * compositing/geometry/fixed-position-transform-composited-page-scale-down.html: * compositing/geometry/fixed-position-transform-composited-page-scale.html: I really wanted bug 71225.