Bug 49778 – Add USE(CHROMIUM_NET)

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[William Chan]

Add USE(CHROMIUM_NET)

[William Chan]

Created attachment 74340 [details]
Patch

[William Chan]

Er, webkit-patch doesn't seem to have an option to add a description, unless I confused it with the title.  Sorry if I missed something.

I plan to use USE(CHROMIUM_NET) to unthrottle some preloading, like so:

diff --git a/WebCore/loader/cache/CachedResourceLoader.cpp b/WebCore/loader/cache/CachedResourceLoader.cpp
index 15c5f03..0e2a393 100644
--- a/WebCore/loader/cache/CachedResourceLoader.cpp
+++ b/WebCore/loader/cache/CachedResourceLoader.cpp
@@ -377,6 +377,9 @@ int CachedResourceLoader::requestCount()
     
 void CachedResourceLoader::preload(CachedResource::Type type, const String& url, const String& charset, bool referencedFromBody)
 {
+    // CHROMIUM_NET will prioritize network requests appropriately, so always
+    // request a preload.
+#if !USE(CHROMIUM_NET)
     bool hasRendering = m_document->body() && m_document->body()->renderer();
     if (!hasRendering && (referencedFromBody || type == CachedResource::ImageResource)) {
         // Don't preload images or body resources before we have something to draw. This prevents
@@ -385,6 +388,8 @@ void CachedResourceLoader::preload(CachedResource::Type type, const String& url,
         m_pendingPreloads.append(pendingPreload);
         return;
     }
+#endif  // !USE(CHROMIUM_NET)
+
     requestPreload(type, url, charset);
 }

Let me know if I should add this change into the proposed patch.  I wasn't sure if they should be separate or not.

[WebKit Review Bot]

Attachment 74340 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'JavaScriptCore/ChangeLog', u'JavaScriptCore/wtf/Platform.h']" exit_code: 1
JavaScriptCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[William Chan]

Created attachment 74344 [details]
Patch

[David Levin]

Why do we need another flag as opposed to just using PLATFORM(CHROMIUM)? It feels like another flag is only needed if you would choose CHROMIUM_NET independently from PLATFORM(CHROMIUM).

Re description: In general, the ChangeLog is the description.

[William Chan]

(In reply to comment #5)
> Why do we need another flag as opposed to just using PLATFORM(CHROMIUM)? It feels like another flag is only needed if you would choose CHROMIUM_NET independently from PLATFORM(CHROMIUM).

I am ill-equipped to justify this, due to my lack of WebKit knowledge.  I asked abarth how to implement my change for preload scanning and he suggested a USE macro.  If PLATFORM is the way to go, I'm happy to use that instead.  I will note that the Android browser (which I believe is the Android port in the webkit tree, although I'm not totally sure) uses the Chromium network stack.  So if it doesn't/shouldn't use PLATFORM(CHROMIUM), then perhaps USE is more appropriate here.  But I don't particularly care.

> 
> Re description: In general, the ChangeLog is the description.

Cool, great.

[WebKit Commit Bot]

Comment on attachment 74344 [details]
Patch

Rejecting patch 74344 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'build', '--no-clean', '--no-update', '--build-style=both']" exit_code: 2
Last 500 characters of output:

    setenv YACC /Developer/usr/bin/yacc
    /bin/sh -c /Projects/CommitQueue/WebKitBuild/WebCore.build/Release/WebCore.build/Script-5DF50887116F3077005202AB.sh

** BUILD FAILED **


The following build commands failed:
WebCore:
	CompileC /Projects/CommitQueue/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/RenderSVGTransformableContainer.o /Projects/CommitQueue/WebCore/rendering/RenderSVGTransformableContainer.cpp normal x86_64 c++ com.apple.compilers.gcc.4_2
(1 failure)


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

[Ryosuke Niwa]

Comment on attachment 74344 [details]
Patch

Try it again.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 74344 [details]:

fast/workers/storage/use-same-database-in-page-and-workers.html
http/tests/appcache/fail-on-update-2.html

Please file bugs against the tests.  These tests were authored by [e-mail] and [e-mail].  The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 74344 [details]
Patch

Clearing flags on attachment: 74344

Committed r72464: <http://trac.webkit.org/changeset/72464>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.