New issue
Advanced search Search tips

Issue 728627 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Feature

Blocked on:
issue 741946



Sign in to add a comment

Refactor SharedBuffer to minimize friction

Project Member Reported by fmalita@chromium.org, Jun 1 2017

Issue description

SharedBuffer currently offers a mixed bag of semantics:

  * efficient/segmented append
  * efficient sequential access
  * consolidated-data and random access
  * no thread safety

Due to the lack of thread safety, we are forced to make a copy before passing the data to rasterizer/image decoding threads.  In order to avoid duplication, the original data is discarded - and (if later needed on the main thread) recreated on-demand, via another copy.

The supporting code is quite complex; this is a tracking bug for related work, as an effort to simplify/improve SharedBuffer.

Some ideas:

1) remove the random-access methods - no clients seem to actually need them

2) remove the data-flattening methods - most clients seem like could be trivially refactored for segmented access

3) make SharedBuffer thread-safe to avoid copying data - SkRWBuffer and its read-only/thread-safe snapshot mechanism seem like a good semantic match

 
Cc: haraken@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7631256819385426bb224a42667129716ae1718b

commit 7631256819385426bb224a42667129716ae1718b
Author: Florin Malita <fmalita@chromium.org>
Date: Thu Jun 01 15:52:42 2017

Consolidate SharedBuffer::{GetAsBytes,GetPartAsBytes}

All GetPartAsBytes() callers request data from the beginning of the buffer,
so the method doesn't need a position argument.  Then it essentially turns
into GetAsBytes(), and we can consolidate the two.

(the hidden motivation here is to remove SharedBuffer random-access
semantics, to facilitate future refactoring).

BUG=728627

Change-Id: I63f9ff2f87a01cc4980d38526531dc9118dd87c8
Reviewed-on: https://chromium-review.googlesource.com/519682
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Leon Scroggins <scroggo@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#476304}
[modify] https://crrev.com/7631256819385426bb224a42667129716ae1718b/third_party/WebKit/Source/bindings/core/v8/ScriptStreamer.cpp
[modify] https://crrev.com/7631256819385426bb224a42667129716ae1718b/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp
[modify] https://crrev.com/7631256819385426bb224a42667129716ae1718b/third_party/WebKit/Source/modules/indexeddb/IDBValueWrapping.cpp
[modify] https://crrev.com/7631256819385426bb224a42667129716ae1718b/third_party/WebKit/Source/modules/indexeddb/IDBValueWrappingTest.cpp
[modify] https://crrev.com/7631256819385426bb224a42667129716ae1718b/third_party/WebKit/Source/platform/SharedBuffer.cpp
[modify] https://crrev.com/7631256819385426bb224a42667129716ae1718b/third_party/WebKit/Source/platform/SharedBuffer.h
[modify] https://crrev.com/7631256819385426bb224a42667129716ae1718b/third_party/WebKit/Source/platform/SharedBufferTest.cpp

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/55ed55a0c0b8c15a956a0f3ea576dc3aa7de6835

commit 55ed55a0c0b8c15a956a0f3ea576dc3aa7de6835
Author: Florin Malita <fmalita@chromium.org>
Date: Fri Jun 02 13:23:51 2017

Refactor MHTMLParser::ParseNextPart() to avoid SharedBuffer::Data()

|content| is a temp buffer, and all its users just want plain/flat data.

No reason for a SharedBuffer - just use a Vector instead.

This also saves a copy from |part| -> |content| in some cases.

BUG=728627

Change-Id: I983987d26ae7f1559086ccb3f425f2b955642852
Reviewed-on: https://chromium-review.googlesource.com/521385
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#476627}
[modify] https://crrev.com/55ed55a0c0b8c15a956a0f3ea576dc3aa7de6835/third_party/WebKit/Source/platform/mhtml/MHTMLParser.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5db415b612948744f0500b9342cca739a7b13820

commit 5db415b612948744f0500b9342cca739a7b13820
Author: Florin Malita <fmalita@chromium.org>
Date: Fri Jun 02 13:38:31 2017

Refactor XMLHttpRequest::ResponseBlob() to avoid SharedBuffer::Data()

Instead of flattening the SharedBuffer data, iterate over its segments
(using a newly added helper: SharedBuffer::ForEachSegment) and append
them one-by-one.

BUG=728627

Change-Id: Iabe368b87d9b3e225a6ca9b9df97b9431b9b2dc7
Reviewed-on: https://chromium-review.googlesource.com/521962
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#476628}
[modify] https://crrev.com/5db415b612948744f0500b9342cca739a7b13820/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp
[modify] https://crrev.com/5db415b612948744f0500b9342cca739a7b13820/third_party/WebKit/Source/platform/SharedBuffer.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 5 2017

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab1ec45d07739f146fb90b8df2e42492cdc3b607

commit ab1ec45d07739f146fb90b8df2e42492cdc3b607
Author: Florin Malita <fmalita@chromium.org>
Date: Thu Jun 08 18:59:50 2017

Avoid SharedBuffer flattening in DocumentThreadableLoader::LoadRequestSync

Use a segment iterator, and consume one segment at a time.

BUG=728627

Change-Id: I8b9831d9709dbb8e9538255180e1146083d58ae1
Reviewed-on: https://chromium-review.googlesource.com/527377
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478043}
[modify] https://crrev.com/ab1ec45d07739f146fb90b8df2e42492cdc3b607/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3c894b4a9151dc2050f53e34a1fc88101ed0f302

commit 3c894b4a9151dc2050f53e34a1fc88101ed0f302
Author: Florin Malita <fmalita@chromium.org>
Date: Fri Jun 09 14:39:08 2017

Avoid SharedBuffer::Data() use in unit tests

This CL introduces an explicit SharedBuffer copy-to-flattened-buffer
method, for use in clients which need contiguous access to the data
(and don't care about the copy overhead).

Refactor SharedBuffer::Copy() to return a Vector<char>.  The few clients
which do require a SharedBuffer copy can always create one using
SharedBuffer::AdoptVector().

Update unit tests to use the explicit flattening API instead of Data().

BUG=728627

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I4f82eb7fea0b233b3d0f63b009fb1d90b778b559
Reviewed-on: https://chromium-review.googlesource.com/527558
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478273}
[modify] https://crrev.com/3c894b4a9151dc2050f53e34a1fc88101ed0f302/third_party/WebKit/Source/core/page/PagePopupClientTest.cpp
[modify] https://crrev.com/3c894b4a9151dc2050f53e34a1fc88101ed0f302/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
[modify] https://crrev.com/3c894b4a9151dc2050f53e34a1fc88101ed0f302/third_party/WebKit/Source/platform/SharedBuffer.cpp
[modify] https://crrev.com/3c894b4a9151dc2050f53e34a1fc88101ed0f302/third_party/WebKit/Source/platform/SharedBuffer.h
[modify] https://crrev.com/3c894b4a9151dc2050f53e34a1fc88101ed0f302/third_party/WebKit/Source/platform/SharedBufferTest.cpp
[modify] https://crrev.com/3c894b4a9151dc2050f53e34a1fc88101ed0f302/third_party/WebKit/Source/platform/graphics/DeferredImageDecoderTest.cpp
[modify] https://crrev.com/3c894b4a9151dc2050f53e34a1fc88101ed0f302/third_party/WebKit/Source/platform/graphics/DeferredImageDecoderTestWoPlatform.cpp
[modify] https://crrev.com/3c894b4a9151dc2050f53e34a1fc88101ed0f302/third_party/WebKit/Source/platform/image-decoders/ImageDecoderTestHelpers.cpp
[modify] https://crrev.com/3c894b4a9151dc2050f53e34a1fc88101ed0f302/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoderTest.cpp
[modify] https://crrev.com/3c894b4a9151dc2050f53e34a1fc88101ed0f302/third_party/WebKit/Source/platform/image-decoders/ico/ICOImageDecoderTest.cpp
[modify] https://crrev.com/3c894b4a9151dc2050f53e34a1fc88101ed0f302/third_party/WebKit/Source/platform/image-decoders/webp/WEBPImageDecoderTest.cpp
[modify] https://crrev.com/3c894b4a9151dc2050f53e34a1fc88101ed0f302/third_party/WebKit/Source/web/tests/FrameSerializerTest.cpp

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5b822a3a8b57bfe9ab6addd7a4af898fb499a9e3

commit 5b822a3a8b57bfe9ab6addd7a4af898fb499a9e3
Author: Florin Malita <fmalita@chromium.org>
Date: Thu Jun 15 11:56:29 2017

Avoid SharedBuffer::Data() use in RawResource

Refactor RawResource::DidAddClient() to use a segment iterator and push
data to the client one segment at a time.

Also update SharedBuffer::ForEachSegment() to allow clients to break early.

BUG=728627

Change-Id: I01597ad179a98cb63b66779224f1007ff78ea789
Reviewed-on: https://chromium-review.googlesource.com/529425
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479668}
[modify] https://crrev.com/5b822a3a8b57bfe9ab6addd7a4af898fb499a9e3/third_party/WebKit/Source/core/dom/DOMArrayBuffer.cpp
[modify] https://crrev.com/5b822a3a8b57bfe9ab6addd7a4af898fb499a9e3/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
[modify] https://crrev.com/5b822a3a8b57bfe9ab6addd7a4af898fb499a9e3/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp
[modify] https://crrev.com/5b822a3a8b57bfe9ab6addd7a4af898fb499a9e3/third_party/WebKit/Source/platform/SharedBuffer.cpp
[modify] https://crrev.com/5b822a3a8b57bfe9ab6addd7a4af898fb499a9e3/third_party/WebKit/Source/platform/SharedBuffer.h
[modify] https://crrev.com/5b822a3a8b57bfe9ab6addd7a4af898fb499a9e3/third_party/WebKit/Source/platform/loader/fetch/RawResource.cpp

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c3d21f501b7259e80f600dfed9e7eb42074c63e5

commit c3d21f501b7259e80f600dfed9e7eb42074c63e5
Author: Florin Malita <fmalita@chromium.org>
Date: Fri Jun 23 13:33:42 2017

Refactor OpenType::ValidateTable() to avoid SharedBuffer data flattening

There's no need to go through an intermediate SharedBuffer, because we
start with a flat Vector<char> in FontPlatformData::OpenTypeTable().

Refactor the validation code to use Vector<char> instead of
SharedBuffer.

BUG=728627

Change-Id: I81892c0c5c42b4f2e2abdeeabaca20979cd60eeb
Reviewed-on: https://chromium-review.googlesource.com/544317
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481867}
[modify] https://crrev.com/c3d21f501b7259e80f600dfed9e7eb42074c63e5/third_party/WebKit/Source/platform/fonts/FontPlatformData.cpp
[modify] https://crrev.com/c3d21f501b7259e80f600dfed9e7eb42074c63e5/third_party/WebKit/Source/platform/fonts/FontPlatformData.h
[modify] https://crrev.com/c3d21f501b7259e80f600dfed9e7eb42074c63e5/third_party/WebKit/Source/platform/fonts/opentype/OpenTypeTypes.h
[modify] https://crrev.com/c3d21f501b7259e80f600dfed9e7eb42074c63e5/third_party/WebKit/Source/platform/fonts/opentype/OpenTypeVerticalData.cpp
[modify] https://crrev.com/c3d21f501b7259e80f600dfed9e7eb42074c63e5/third_party/WebKit/Source/platform/fonts/opentype/OpenTypeVerticalDataTest.cpp
[modify] https://crrev.com/c3d21f501b7259e80f600dfed9e7eb42074c63e5/third_party/WebKit/Source/platform/testing/FontTestHelpers.cpp
[modify] https://crrev.com/c3d21f501b7259e80f600dfed9e7eb42074c63e5/third_party/WebKit/Source/platform/wtf/ByteOrder.h

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/70f64e7ea890adca0e3737fbcfd27bb623618619

commit 70f64e7ea890adca0e3737fbcfd27bb623618619
Author: Florin Malita <fmalita@chromium.org>
Date: Mon Jun 26 16:07:13 2017

Refactor XSLTProcessor to avoid SharedBuffer data flattening APIs

Use a segment iterator and a chunked/push libxml parser instead.

BUG=728627

Change-Id: I5d73cf68ff8f11bfd058b2ad27a62f8084c577ca
Reviewed-on: https://chromium-review.googlesource.com/544237
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482285}
[modify] https://crrev.com/70f64e7ea890adca0e3737fbcfd27bb623618619/third_party/WebKit/Source/core/xml/XSLTProcessorLibxslt.cpp

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/85a03d810c005596c8b24472514cf31d210d4fd4

commit 85a03d810c005596c8b24472514cf31d210d4fd4
Author: Florin Malita <fmalita@chromium.org>
Date: Tue Jun 27 13:47:01 2017

Follow up to SharedBuffer::Data() refactor

Post-landing review suggestions for
https://chromium-review.googlesource.com/c/529425/.

  1) break the DocumentThreadableLoader::LoadRequestSync() segment
     iterator early if the client removes itself

  2) as a precaution, retain the SharedBuffer while iterating in
     RawResource::DidAddClient()

BUG=728627

Change-Id: If9a9fb768b9d8127002e97b87ecdc99aa2d115cc
Reviewed-on: https://chromium-review.googlesource.com/547137
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482614}
[modify] https://crrev.com/85a03d810c005596c8b24472514cf31d210d4fd4/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
[modify] https://crrev.com/85a03d810c005596c8b24472514cf31d210d4fd4/third_party/WebKit/Source/platform/loader/fetch/RawResource.cpp

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b58d8d3f70e3cd57eee74f22225371b203a47ae

commit 6b58d8d3f70e3cd57eee74f22225371b203a47ae
Author: Florin Malita <fmalita@chromium.org>
Date: Wed Jul 12 01:02:27 2017

Refactor GenerateMHTMLPart() to avoid SharedBuffer::Data()

Introduce a helper class (SharedBuffer::DeprecatedFlatData) which produces
a flat data view for a given SharedBuffer.  The helper only makes a copy
when strictly necessary (the buffer is segmented).

Refactor GenerateMHTMLPart() to use DeprecatedFlatData instead of
SharedBuffer::Data().

Note: this means we may now create an extra temp copy in some cases, but
since this code path is only used for MHTML serialization, the overhead
is likely not critical.

BUG=728627

Change-Id: I8c5695cb87776482951c6b87b5a4105444dc7693
Reviewed-on: https://chromium-review.googlesource.com/546616
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485755}
[modify] https://crrev.com/6b58d8d3f70e3cd57eee74f22225371b203a47ae/third_party/WebKit/Source/platform/SharedBuffer.cpp
[modify] https://crrev.com/6b58d8d3f70e3cd57eee74f22225371b203a47ae/third_party/WebKit/Source/platform/SharedBuffer.h
[modify] https://crrev.com/6b58d8d3f70e3cd57eee74f22225371b203a47ae/third_party/WebKit/Source/platform/SharedBufferTest.cpp
[modify] https://crrev.com/6b58d8d3f70e3cd57eee74f22225371b203a47ae/third_party/WebKit/Source/platform/mhtml/MHTMLArchive.cpp

Cc: chrishtr@chromium.org
Blockedon: 741946
Project Member

Comment 15 by bugdroid1@chromium.org, Jul 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0fe3e77bdfb51ca9ce531f5c64687dd8332c1116

commit 0fe3e77bdfb51ca9ce531f5c64687dd8332c1116
Author: Florin Malita <fmalita@chromium.org>
Date: Tue Jul 18 18:30:53 2017

Refactor InspectorPageAgent to avoid SharedBuffer::Data()

Use the flat data accessor instead.

BUG=728627

Change-Id: I0ff48a9e4d1ce60c0fcc822ac6b839f3fb1ad6e0
Reviewed-on: https://chromium-review.googlesource.com/574576
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487535}
[modify] https://crrev.com/0fe3e77bdfb51ca9ce531f5c64687dd8332c1116/third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp

Project Member

Comment 16 by bugdroid1@chromium.org, Jul 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5c8a922c6b85d88a866cce65f96d5de91d9eb627

commit 5c8a922c6b85d88a866cce65f96d5de91d9eb627
Author: Florin Malita <fmalita@chromium.org>
Date: Wed Jul 19 14:50:23 2017

Refactor IDBValue to avoid SharedBuffer::Data()

Introduce a SharedBuffer-based SerializedScriptValue factory,
implemented with segment iterators.

BUG=728627

Change-Id: I9465b4c6126dbeaab8794931cc4f9d19f295aedb
Reviewed-on: https://chromium-review.googlesource.com/574867
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487858}
[modify] https://crrev.com/5c8a922c6b85d88a866cce65f96d5de91d9eb627/third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValue.cpp
[modify] https://crrev.com/5c8a922c6b85d88a866cce65f96d5de91d9eb627/third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValue.h
[modify] https://crrev.com/5c8a922c6b85d88a866cce65f96d5de91d9eb627/third_party/WebKit/Source/modules/indexeddb/IDBValue.cpp

Project Member

Comment 17 by bugdroid1@chromium.org, Jul 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a936854095a080b14226f5499328ff7a12b06f30

commit a936854095a080b14226f5499328ff7a12b06f30
Author: Florin Malita <fmalita@chromium.org>
Date: Thu Jul 20 11:41:22 2017

Refactor DocumentLoader to avoid SharedBuffer::Data()

Use a segment iterator insted.

BUG=728627

Change-Id: I6bb88a0696e920f2751181d4c8acc9d790787e26
Reviewed-on: https://chromium-review.googlesource.com/577672
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488200}
[modify] https://crrev.com/a936854095a080b14226f5499328ff7a12b06f30/third_party/WebKit/Source/core/loader/DocumentLoader.cpp

Project Member

Comment 18 by bugdroid1@chromium.org, Jul 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4378a01c688596d226d5d3bd2aabe67d8a538bab

commit 4378a01c688596d226d5d3bd2aabe67d8a538bab
Author: Florin Malita <fmalita@chromium.org>
Date: Thu Jul 20 11:41:39 2017

Refactor FontResource to avoid SharedBuffer::Data()

Since PackageFormatOf() only needs the first 4 bytes, just copy them to
a local buffer.

BUG=728627

Change-Id: I19e65c95f4737968967fb63196b8ed6d07fa7fae
Reviewed-on: https://chromium-review.googlesource.com/577673
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488201}
[modify] https://crrev.com/4378a01c688596d226d5d3bd2aabe67d8a538bab/third_party/WebKit/Source/core/loader/resource/FontResource.cpp

Project Member

Comment 19 by bugdroid1@chromium.org, Jul 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/672c226631964e9de99dd5f207d0ea10ec16f7f8

commit 672c226631964e9de99dd5f207d0ea10ec16f7f8
Author: Florin Malita <fmalita@chromium.org>
Date: Mon Jul 24 14:58:02 2017

Refactor WebFontDecoder to avoid SharedBuffer::Data()

Sice Decode() must pass the data to an external library (OTS), we have
no choice but to use the flattening API.  We only do this once per
webfont though (when instantiating the FontCustomPlatformData/typeface),
so I don't expect any regressions.

BUG=728627

Change-Id: If239d42d58e9ec1cf137cee5451cedd07366e058
Reviewed-on: https://chromium-review.googlesource.com/581430
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488967}
[modify] https://crrev.com/672c226631964e9de99dd5f207d0ea10ec16f7f8/third_party/WebKit/Source/platform/fonts/WebFontDecoder.cpp

Project Member

Comment 20 by bugdroid1@chromium.org, Jul 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a7ecc49b7b45c8988d280ea74cf0c4c9d8f0d16c

commit a7ecc49b7b45c8988d280ea74cf0c4c9d8f0d16c
Author: Florin Malita <fmalita@chromium.org>
Date: Mon Jul 24 17:21:05 2017

Remove WebData::Data()

WebData is a thin wrapper around SharedBuffer, and currently exposes
the latter's flat data API.

Add segment-based accessors similar to SharedBuffer::{GetSomeData,
ForEachSegment}, refactor all clients based on these, and finally remove
WebData::Data().

BUG=728627

Change-Id: I1f6d72a6f18da752d003e23376d349c2d4356d46
Reviewed-on: https://chromium-review.googlesource.com/581748
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489000}
[modify] https://crrev.com/a7ecc49b7b45c8988d280ea74cf0c4c9d8f0d16c/content/child/indexed_db/indexed_db_key_builders.cc
[modify] https://crrev.com/a7ecc49b7b45c8988d280ea74cf0c4c9d8f0d16c/content/child/indexed_db/webidbdatabase_impl.cc
[modify] https://crrev.com/a7ecc49b7b45c8988d280ea74cf0c4c9d8f0d16c/content/child/web_url_request_util.cc
[modify] https://crrev.com/a7ecc49b7b45c8988d280ea74cf0c4c9d8f0d16c/content/renderer/drop_data_builder.cc
[modify] https://crrev.com/a7ecc49b7b45c8988d280ea74cf0c4c9d8f0d16c/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/a7ecc49b7b45c8988d280ea74cf0c4c9d8f0d16c/third_party/WebKit/Source/modules/encryptedmedia/MediaKeyStatusMap.cpp
[modify] https://crrev.com/a7ecc49b7b45c8988d280ea74cf0c4c9d8f0d16c/third_party/WebKit/Source/platform/DataResourceHelper.cpp
[modify] https://crrev.com/a7ecc49b7b45c8988d280ea74cf0c4c9d8f0d16c/third_party/WebKit/Source/platform/audio/AudioBus.cpp
[modify] https://crrev.com/a7ecc49b7b45c8988d280ea74cf0c4c9d8f0d16c/third_party/WebKit/Source/platform/exported/WebData.cpp
[modify] https://crrev.com/a7ecc49b7b45c8988d280ea74cf0c4c9d8f0d16c/third_party/WebKit/Source/platform/exported/WebHTTPBody.cpp
[modify] https://crrev.com/a7ecc49b7b45c8988d280ea74cf0c4c9d8f0d16c/third_party/WebKit/Source/platform/loader/fetch/ResourceLoader.cpp
[modify] https://crrev.com/a7ecc49b7b45c8988d280ea74cf0c4c9d8f0d16c/third_party/WebKit/Source/platform/testing/weburl_loader_mock.cc
[modify] https://crrev.com/a7ecc49b7b45c8988d280ea74cf0c4c9d8f0d16c/third_party/WebKit/public/platform/WebData.h

Project Member

Comment 21 by bugdroid1@chromium.org, Jul 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7ce52e55c486ac3129cea139f771e43ee1024845

commit 7ce52e55c486ac3129cea139f771e43ee1024845
Author: Florin Malita <fmalita@chromium.org>
Date: Wed Jul 26 12:28:07 2017

Refactor NetworkResourcesData to avoid SharedBuffer::Data()

Factor out the common code as PrepareToAddResourceData(), and introduce
a MaybeAddResourceData() variant which takes a SharedBuffer and uses
segment iteration.

BUG=728627

Change-Id: I05f2665a813ac5b6940b0c171e3ee1ef91ab49ca
Reviewed-on: https://chromium-review.googlesource.com/584929
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489614}
[modify] https://crrev.com/7ce52e55c486ac3129cea139f771e43ee1024845/third_party/WebKit/Source/core/inspector/NetworkResourcesData.cpp
[modify] https://crrev.com/7ce52e55c486ac3129cea139f771e43ee1024845/third_party/WebKit/Source/core/inspector/NetworkResourcesData.h

Project Member

Comment 22 by sheriffbot@chromium.org, Jul 26

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment