The enum ReasonForCallingCanExecuteScripts is present in both the JSC and V8 versions of ScriptController.h, and is used in ScriptControllerBase.cpp. We should move this enum into a new header, say ScriptControllerBase.h, so that it is defined in one file. This will also be useful in the patch for Bug #39249, which proposes adding an enum to be used in ScriptController.
Created attachment 56448 [details] Patch No functionality was changed, so no new tests.
Comment on attachment 56448 [details] Patch Seems OK to move this to a separate file. I'm not so fond of the name ScriptControllerBase.h for this.
(In reply to comment #2) > (From update of attachment 56448 [details]) > Seems OK to move this to a separate file. I'm not so fond of the name ScriptControllerBase.h for this. I am open to suggestions. There will be at least two enums in this file: ReasonForCallingCanExecuteScripts, and ShouldAllowXSS (will update patch for Bug #39249).
Committed r60017: <http://trac.webkit.org/changeset/60017>
This broke the Windows and Qt bots. Committed revert in 60019 <http://trac.webkit.org/changeset/60019> while I look into this some more.
The problem with Qt build was the missing include path. You can fix it easyily, I think the second one is the nicer. 1.) diff --git a/WebCore/bindings/js/ScriptController.h b/WebCore/bindings/js/ScriptController.h -#include "ScriptControllerBase.h" +#include "../ScriptControllerBase.h" 2.) diff --git a/WebCore/WebCore.pro b/WebCore/WebCore.pro index 6fbeffc..034e45f 100644 --- a/WebCore/WebCore.pro +++ b/WebCore/WebCore.pro @@ -170,6 +170,7 @@ include(WebCore.pri) INCLUDEPATH = \ $$PWD \ $$PWD/accessibility \ + $$PWD/bindings \ $$PWD/bindings/js \ $$PWD/bridge \ $$PWD/bridge/c \
(In reply to comment #6) > The problem with Qt build was the missing include path. > You can fix it easyily, I think the second one is the nicer. > > 1.) > diff --git a/WebCore/bindings/js/ScriptController.h b/WebCore/bindings/js/ScriptController.h > -#include "ScriptControllerBase.h" > +#include "../ScriptControllerBase.h" > > 2.) > diff --git a/WebCore/WebCore.pro b/WebCore/WebCore.pro > index 6fbeffc..034e45f 100644 > --- a/WebCore/WebCore.pro > +++ b/WebCore/WebCore.pro > @@ -170,6 +170,7 @@ include(WebCore.pri) > INCLUDEPATH = \ > $$PWD \ > $$PWD/accessibility \ > + $$PWD/bindings \ > $$PWD/bindings/js \ > $$PWD/bridge \ > $$PWD/bridge/c \ Thanks Csaba. I also prefer the second option of adding "$$PWD/bindings \" to WebCore.pri. Will give this a try.
Comment on attachment 56448 [details] Patch Clearing r+ flag to remove this from pending-commit.
Created attachment 61145 [details] Patch Updated patch based on Csaba's suggestion and fixed Windows build issues.
Comment on attachment 61145 [details] Patch There's probably more we can move into that header too. I think some of the build systems are unhappy with this change. WebCore/bindings/ScriptControllerBase.h:35 + enum ReasonForCallingCanExecuteScripts { No namespace indent.
(In reply to comment #10) > (From update of attachment 61145 [details]) > There's probably more we can move into that header too. I think some of the build systems are unhappy with this change. > > WebCore/bindings/ScriptControllerBase.h:35 > + enum ReasonForCallingCanExecuteScripts { > No namespace indent. Will fix when I land. This change built on my Mac and Windows builds. The EWS bots don't show any descriptive error messages and/or other error output.
Committed r63050: <http://trac.webkit.org/changeset/63050>