Bug 61479 - MemoryPressureHandler class should not be in WebCore/platform
: MemoryPressureHandler class should not be in WebCore/platform
Status: ASSIGNED
Product: WebKit
Classification: Unclassified
Component: WebCore Misc.
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To: Michael Saboff
:
Depends on:
Blocks: 21354
  Show dependency treegraph
 
Reported: 2011-05-25 15:55 PDT by Michael Saboff
Modified: 2012-01-30 17:00 PST (History)
9 users (show)

See Also:


Attachments
Proposed Patch (14.40 KB, patch)
2011-07-12 02:19 PDT, Mustafizur Rahaman (rahaman)
sam: review-
Details | Formatted Diff | Diff
Patch to incorporate Michael & Sam's review comment (23.08 KB, patch)
2011-07-13 23:04 PDT, Mustafizur Rahaman (rahaman)
no flags Details | Formatted Diff | Diff
Patch re-created on latest code base (23.11 KB, patch)
2011-07-14 23:27 PDT, Mustafizur Rahaman (rahaman)
sam: review-
Details | Formatted Diff | Diff
Move MemoryPressureHandler class from WebCore/platform to WebCore/page. (36.59 KB, patch)
2012-01-18 15:33 PST, Yongjun Zhang
gns: commit‑queue-
Details | Formatted Diff | Diff
fix gtk build break. (36.58 KB, patch)
2012-01-18 21:12 PST, Yongjun Zhang
no flags Details | Formatted Diff | Diff
Fix win build break. (37.39 KB, patch)
2012-01-19 09:42 PST, Yongjun Zhang
eric: review+
webkit.review.bot: commit‑queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2011-05-25 15:55:59 PDT
A comment came in after https://bugs.webkit.org/show_bug.cgi?id=61222 was landed that the new class, WebCore::MemoryPressureHandler, should not be in WebCore/platform.  The key point from the comment is "... files in platform/ should not be including or using classes outside of platform (and not in WTF or JSC) such as MemoryCache or PageCache.  We consider this to be a layering violation in WebCore."

It has been suggested that this class be moved to WebCore/page.
Comment 1 Mustafizur Rahaman (rahaman) 2011-07-12 02:19:41 PDT
Created attachment 100453 [details]
Proposed Patch

Moved the MemoryPressureHandler* source/header files from WebCore/platform to WebCore/page
Comment 2 Michael Saboff 2011-07-12 10:15:07 PDT
I think you also need to change Source/WebCore/WebCore.xcodeproj/project.pbxproj to support the move on Mac.  Besides that, the change looks good.
Comment 3 Sam Weinig 2011-07-12 20:33:17 PDT
Comment on attachment 100453 [details]
Proposed Patch

As Michael said, this will break the mac as is.
Comment 4 Mustafizur Rahaman (rahaman) 2011-07-13 23:04:53 PDT
Created attachment 100771 [details]
Patch to incorporate Michael & Sam's review comment

Modified the xcodeproj file & now it is building fine in Mac.
Comment 5 Sam Weinig 2011-07-14 11:02:47 PDT
Can you upload another one that merges cleanly with the bots so that we can see it if it really builds on the mac.
Comment 6 Mustafizur Rahaman (rahaman) 2011-07-14 23:27:38 PDT
Created attachment 100936 [details]
Patch re-created on latest code base

There was problem applying the previous patch. So, recreated the patch on the latest code base.
Comment 7 Sam Weinig 2011-07-20 14:44:43 PDT
Comment on attachment 100936 [details]
Patch re-created on latest code base

This will break the Lion build. You need to migrate MemoryPressureHandlerMac.mm as well.
Comment 8 Yongjun Zhang 2012-01-18 15:33:29 PST
Created attachment 123020 [details]
Move MemoryPressureHandler class from WebCore/platform to WebCore/page.
Comment 9 Gustavo Noronha (kov) 2012-01-18 18:49:33 PST
Comment on attachment 123020 [details]
Move MemoryPressureHandler class from WebCore/platform to WebCore/page.

Attachment 123020 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11282454
Comment 10 Yongjun Zhang 2012-01-18 21:12:15 PST
Created attachment 123065 [details]
fix gtk build break.
Comment 11 Yongjun Zhang 2012-01-19 09:42:12 PST
Created attachment 123141 [details]
Fix win build break.
Comment 12 Eric Seidel 2012-01-30 16:57:17 PST
Comment on attachment 123141 [details]
Fix win build break.

Seems reasonable.
Comment 13 WebKit Review Bot 2012-01-30 17:00:52 PST
Comment on attachment 123141 [details]
Fix win build break.

Rejecting attachment 123141 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
.mm
patching file Source/WebCore/platform/MemoryPressureHandler.cpp
rm 'Source/WebCore/platform/MemoryPressureHandler.cpp'
patching file Source/WebCore/platform/MemoryPressureHandler.h
rm 'Source/WebCore/platform/MemoryPressureHandler.h'
patching file Source/WebCore/platform/mac/MemoryPressureHandlerMac.mm
rm 'Source/WebCore/platform/mac/MemoryPressureHandlerMac.mm'

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Eric Seidel']" exit_code: 1

Full output: http://queues.webkit.org/results/11365779