Bug 90863 - [Qt][V8] Remove the V8 related codepaths and configuration
Summary: [Qt][V8] Remove the V8 related codepaths and configuration
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gabor Ballabas
URL:
Keywords:
Depends on: 91492
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-10 02:08 PDT by Andras Becsi
Modified: 2012-07-17 07:52 PDT (History)
13 users (show)

See Also:


Attachments
Patch (83.27 KB, patch)
2012-07-16 05:25 PDT, Gabor Ballabas
hausmann: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Remove v8 from QtWebKit (81.95 KB, patch)
2012-07-17 05:14 PDT, Gabor Ballabas
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andras Becsi 2012-07-10 02:08:37 PDT
Since the conclusion was to discontinue the effort for QtWebKit-V8 (http://lists.webkit.org/pipermail/webkit-qt/2012-June/thread.html#2871) we should remove all the codepaths and configuration options which were introduced for building with V8.
Comment 1 Balazs Kelemen 2012-07-10 02:10:08 PDT
Burn!
Comment 2 Balazs Kelemen 2012-07-10 02:11:54 PDT
These guys want to see the show too :D
Comment 3 Balazs Kelemen 2012-07-15 08:30:40 PDT
Gabor has started working on this.
Comment 4 Gabor Ballabas 2012-07-16 05:25:11 PDT
Created attachment 152517 [details]
Patch
Comment 5 WebKit Review Bot 2012-07-16 06:01:47 PDT
Comment on attachment 152517 [details]
Patch

Attachment 152517 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13245885
Comment 6 Simon Hausmann 2012-07-16 06:09:10 PDT
Comment on attachment 152517 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=152517&action=review

Looks good in general, a few small nitpicks

> Source/WebCore/DerivedSources.pri:621
> -v8: wrapperFactoryArg = --wrapperFactoryV8
> -else: wrapperFactoryArg = --wrapperFactory
> +wrapperFactoryArg = --wrapperFactory

You could eliminate the variable entirely I think

> Source/WebCore/DerivedSources.pri:638
>  svgnames.extra_sources = SVGElementFactory.cpp
> -v8 {
> -    svgnames.extra_sources += V8SVGElementWrapperFactory.cpp
> -} else {
>      svgnames.extra_sources += JSSVGElementWrapperFactory.cpp

You could merge line 637 and 638 or at least remove the indentation in 638 :)

> Source/WebCore/DerivedSources.pri:695
> +generator = JS

Same here, I think you could just get rid of the generator variable and pass --generator JS directly
Comment 7 Simon Hausmann 2012-07-16 22:47:45 PDT
Comment on attachment 152517 [details]
Patch

r=me if you do the small fix-ups I suggested before landing.
Comment 8 Kristóf Kosztyó 2012-07-17 03:59:40 PDT
Committed r122834: <http://trac.webkit.org/changeset/122834>
Comment 9 WebKit Review Bot 2012-07-17 04:25:02 PDT
Re-opened since this is blocked by 91492
Comment 10 Gabor Ballabas 2012-07-17 05:14:57 PDT
Created attachment 152737 [details]
Remove v8 from QtWebKit
Comment 11 WebKit Review Bot 2012-07-17 06:50:14 PDT
Comment on attachment 152737 [details]
Remove v8 from QtWebKit

Rejecting attachment 152737 [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/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/13276141
Comment 12 WebKit Review Bot 2012-07-17 07:52:07 PDT
Comment on attachment 152737 [details]
Remove v8 from QtWebKit

Clearing flags on attachment: 152737

Committed r122845: <http://trac.webkit.org/changeset/122845>
Comment 13 WebKit Review Bot 2012-07-17 07:52:13 PDT
All reviewed patches have been landed.  Closing bug.