Bug 30200 - Move executeScript from FrameLoader to ScriptController
: Move executeScript from FrameLoader to ScriptController
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Page Loading
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks: 29947
  Show dependency treegraph
 
Reported: 2009-10-08 01:04 PDT by Adam Barth
Modified: 2009-10-14 23:08 PDT (History)
1 user (show)

See Also:


Attachments
Patch v1 (25.36 KB, patch)
2009-10-08 01:14 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch v1 (25.91 KB, patch)
2009-10-08 01:16 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch v1 (30.98 KB, patch)
2009-10-08 10:26 PDT, Adam Barth
eric: review+
commit-queue: commit‑queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2009-10-08 01:04:33 PDT
executeScript and executeIfJavaScriptURL are really about controlling scripts, not about loading pages.  Therefore, we should move them from FrameLoader to ScriptController!
Comment 1 Adam Barth 2009-10-08 01:14:33 PDT
Created attachment 40856 [details]
Patch v1
Comment 2 Adam Barth 2009-10-08 01:16:10 PDT
Created attachment 40857 [details]
Patch v1
Comment 3 Eric Seidel 2009-10-08 09:32:29 PDT
Comment on attachment 40857 [details]
Patch v1

Can you first invent a x-platform ScriptController if your'e going to add duplicate code?

ScriptController

And then ScriptControllerJSC and ScriptControllerV8 which inherit from it?

Or heck, you don't even need them to inherit.  Just add a new ScriptController.cpp at the root level which has these fixes, and makes sure both builds include it, no?

Unless this code is not copy/paste and I'm mising something?

r- for adding copy/paste code.
Comment 4 Adam Barth 2009-10-08 10:26:06 PDT
Created attachment 40890 [details]
Patch v1
Comment 5 Eric Seidel 2009-10-08 19:13:25 PDT
Comment on attachment 40890 [details]
Patch v1

ChangeLog is out of date.  ScriptControllerBase being a real class would be better, but this is OK.
Comment 6 WebKit Commit Bot 2009-10-08 21:47:44 PDT
Comment on attachment 40890 [details]
Patch v1

Rejecting patch 40890 from commit-queue.

Patch https://bugs.webkit.org/attachment.cgi?id=40890 from bug 30200 failed to download and apply.
Comment 7 Adam Barth 2009-10-08 22:09:03 PDT
Committed r49372: <http://trac.webkit.org/changeset/49372>
Comment 8 Eric Seidel 2009-10-09 14:33:30 PDT
XCode project simply went out of date.